From 6dfaa566996905d1fdb0e2a07521a1f694945b7e Mon Sep 17 00:00:00 2001 From: Tom Kay Date: Fri, 4 Sep 2026 09:56:08 +0100 Subject: [PATCH 1/2] Compare ports as integers when suppressing the default port #3 tightened the standard-port comparisons from == to ===, in both Request::isStandardPort() and LinkBuilder::_isStandardPort(). Neither receives a reliably-typed port, so the strict comparison silently stopped matching: - LinkBuilder::setPort() is documented "mixed" and callers pass strings. setPort('443') on an https URL emitted https://host:443/path instead of https://host/path. setPort(443) worked, which is why only one assertion in LinkBuilderTest caught it. - Symfony's Request::getPort() returns int|string|null. With no HOST header it returns SERVER_PORT verbatim, which the SAPI provides as a string, so isStandardPort() returned false for a genuinely standard port and urlSprintf("%o") appended ":80" / ":443" to generated URLs. Cast to int before comparing, in both places. The strict comparison stays, so a non-numeric port still does not count as standard. Adds coverage for the string-typed port on both paths -- LinkBuilder via setPort('80') and Request via SERVER_PORT with the HOST header removed. Both new assertions fail without this change. Co-Authored-By: Claude Opus 5 (1M context) --- src/LinkBuilder/LinkBuilder.php | 1 + src/Request.php | 2 +- tests/LinkBuilderTest.php | 2 ++ tests/RequestTest.php | 16 ++++++++++++++++ 4 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/LinkBuilder/LinkBuilder.php b/src/LinkBuilder/LinkBuilder.php index 97c0ea1..1793e50 100644 --- a/src/LinkBuilder/LinkBuilder.php +++ b/src/LinkBuilder/LinkBuilder.php @@ -56,6 +56,7 @@ public function asUrl(): string protected function _isStandardPort($scheme, $port) { + $port = (int)$port; return ('http' === $scheme && $port === 80) || ('https' === $scheme && $port === 443); } diff --git a/src/Request.php b/src/Request.php index 71dcd5b..98bb698 100644 --- a/src/Request.php +++ b/src/Request.php @@ -195,7 +195,7 @@ public function url() public function isStandardPort() { $scheme = $this->getScheme(); - $port = $this->getPort(); + $port = (int)$this->getPort(); return ('http' === $scheme && $port === 80) || ('https' === $scheme && $port === 443); } diff --git a/tests/LinkBuilderTest.php b/tests/LinkBuilderTest.php index 4bbae8d..4932140 100644 --- a/tests/LinkBuilderTest.php +++ b/tests/LinkBuilderTest.php @@ -20,6 +20,8 @@ public function testAsUrl() self::assertEquals('http://www.packaged.local:81', LinkBuilder::fromRequest($request)->asUrl()); self::assertEquals('http://www.packaged.local:81/ab', LinkBuilder::fromRequest($request, '/ab')->asUrl()); self::assertEquals('http://www.packaged.local', LinkBuilder::fromRequest($request)->setPort(80)->asUrl()); + self::assertEquals('http://www.packaged.local', LinkBuilder::fromRequest($request)->setPort('80')->asUrl()); + self::assertEquals('http://www.packaged.local:81', LinkBuilder::fromRequest($request)->setPort('81')->asUrl()); $lb = LinkBuilder::fromRequest($request); self::assertEquals('http://www.packaged.local:81', $lb->asUrl()); diff --git a/tests/RequestTest.php b/tests/RequestTest.php index 8341cdc..5111fa7 100644 --- a/tests/RequestTest.php +++ b/tests/RequestTest.php @@ -82,6 +82,22 @@ public function testStandardPort() $this->assertTrue($request->isStandardPort()); } + /** + * With no HOST header, getPort() returns SERVER_PORT verbatim, which the + * SAPI provides as a string + */ + public function testStandardPortFromServerPortString() + { + $request = Request::createFromGlobals(); + $request->headers->remove('HOST'); + + $request->server->set('SERVER_PORT', '80'); + $this->assertTrue($request->isStandardPort()); + + $request->server->set('SERVER_PORT', '81'); + $this->assertFalse($request->isStandardPort()); + } + public function testMatchDomain() { $request = Request::createFromGlobals(); From 543cd70030c44a09bf96d7324aaabc845bdddaa3 Mon Sep 17 00:00:00 2001 From: Tom Kay Date: Thu, 3 Sep 2026 12:55:12 +0100 Subject: [PATCH 2/2] Add GitHub Actions CI with a PHP version matrix Adds a setup-php matrix over 8.2, 8.3, 8.4 and 8.5, with fail-fast disabled so every leg reports independently. This repo had no CI of any kind. The matrix starts at 8.2 rather than dal's 8.0. composer.json claims "php": "^8.0", but symfony/http-foundation ^v7.2.0 resolves to 7.4.x, which requires PHP >= 8.2, so composer cannot install on 8.0 or 8.1 -- verified by those legs failing on an earlier push of this branch. Bumping the declared floor to ^8.2 would make composer.json honest, but that is a packaging decision and is left alone. Note that the suite is red on master: LinkBuilderTest::testAsUrl expects setPort('443') on an https URL to drop the default port, and asUrl() now emits it. That predates this change and reproduces on every PHP version in the matrix, so it is left for a separate fix. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/ci.yml | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 .github/workflows/ci.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..155adc7 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,38 @@ +name: CI + +on: + push: + branches: + - master + pull_request: + branches: + - master + +jobs: + test: + runs-on: ubuntu-latest + + strategy: + fail-fast: false + matrix: + php-version: + - "8.2" + - "8.3" + - "8.4" + - "8.5" + + steps: + - name: Checkout repository + uses: actions/checkout@v7 + + - name: Setup PHP + uses: shivammathur/setup-php@v2 + with: + php-version: ${{ matrix.php-version }} + coverage: none + + - name: Composer install + run: composer install + + - name: PHPUnit + run: vendor/bin/phpunit