From 904caa7da582b5b7fd058a6cb9db7fee31f0d3df Mon Sep 17 00:00:00 2001 From: Greg Bowler Date: Fri, 20 Mar 2026 14:29:10 +0000 Subject: [PATCH 1/2] test: isolate bug #72 --- test/phpunit/Header/HeadersTest.php | 59 +++++++++++++++++++++++++++-- test/phpunit/Header/ParserTest.php | 14 +++++++ test/phpunit/RequestTest.php | 20 ++++++++++ 3 files changed, 89 insertions(+), 4 deletions(-) diff --git a/test/phpunit/Header/HeadersTest.php b/test/phpunit/Header/HeadersTest.php index 17e7b28..5ebf8f8 100644 --- a/test/phpunit/Header/HeadersTest.php +++ b/test/phpunit/Header/HeadersTest.php @@ -62,12 +62,12 @@ public function testAddMultiple() { public function testAddMultipleCommaHeader() { $headers = new Headers(self::HEADER_ARRAY); $headers->add( - "Cookie-set", + "Set-Cookie", "language=en; expires=Thu, 1-Jan-1970 00:00:00 UTC; path=/; domain=example.com", "id=123; expires=Thu, 1-Jan-1970 00:00:00 UTC; path=/; domain=example.com httponly" ); $headerArray = $headers->asArray(); - $cookie = explode("\n", $headerArray["Cookie-set"]); + $cookie = explode("\n", $headerArray["Set-Cookie"]); self::assertContains("language=en; expires=Thu, 1-Jan-1970 00:00:00 UTC; path=/; domain=example.com", $cookie); self::assertContains("id=123; expires=Thu, 1-Jan-1970 00:00:00 UTC; path=/; domain=example.com httponly", $cookie); } @@ -115,7 +115,7 @@ public function testGetMultiple() { public function testGetMultipleCommas() { $headers = new Headers(self::HEADER_ARRAY); $headers->add( - "Cookie-set", + "Set-Cookie", "language=en; expires=Thu, 1-Jan-1970 00:00:00 UTC; path=/; domain=example.com", "id=123; expires=Thu, 1-Jan-1970 00:00:00 UTC; path=/; domain=example.com httponly" ); @@ -123,7 +123,58 @@ public function testGetMultipleCommas() { "language=en; expires=Thu, 1-Jan-1970 00:00:00 UTC; path=/; domain=example.com" . "\n" . "id=123; expires=Thu, 1-Jan-1970 00:00:00 UTC; path=/; domain=example.com httponly", - $headers->get("Cookie-set") + $headers->get("Set-Cookie") + ); + } + + public function testAddMultipleSetCookieWithoutCommaPreservesSeparateLines():void { + $headers = new Headers(self::HEADER_ARRAY); + $headers->add("Set-Cookie", "language=en; Path=/"); + $headers->add("Set-Cookie", "id=123; HttpOnly"); + + self::assertCount(5, $headers); + self::assertSame( + "language=en; Path=/\nid=123; HttpOnly", + $headers->asArray()["Set-Cookie"] + ); + } + + public function testGetAllAggregatesValuesAcrossSeparateHeaderLines():void { + $firstValue = "Digest realm=\"example.com\", qop=\"auth\""; + $secondValue = "Digest realm=\"api.example.com\", qop=\"auth-int\""; + $headers = new Headers(self::HEADER_ARRAY); + $headers->add("WWW-Authenticate", $firstValue); + $headers->add("WWW-Authenticate", $secondValue); + + self::assertSame( + [$firstValue, $secondValue], + $headers->getAll("WWW-Authenticate") + ); + } + + public function testAsArrayPreservesAllDuplicateSpecialCaseHeaderLines():void { + $firstValue = "Digest realm=\"example.com\", qop=\"auth\""; + $secondValue = "Digest realm=\"api.example.com\", qop=\"auth-int\""; + $headers = new Headers(self::HEADER_ARRAY); + $headers->add("WWW-Authenticate", $firstValue); + $headers->add("WWW-Authenticate", $secondValue); + + self::assertSame( + $firstValue . "\n" . $secondValue, + $headers->asArray()["WWW-Authenticate"] + ); + } + + public function testAsArrayNestedPreservesAllDuplicateHeaderLineValues():void { + $firstValue = "Digest realm=\"example.com\", qop=\"auth\""; + $secondValue = "Digest realm=\"api.example.com\", qop=\"auth-int\""; + $headers = new Headers(self::HEADER_ARRAY); + $headers->add("WWW-Authenticate", $firstValue); + $headers->add("WWW-Authenticate", $secondValue); + + self::assertSame( + [$firstValue, $secondValue], + $headers->asArray(true)["WWW-Authenticate"] ); } diff --git a/test/phpunit/Header/ParserTest.php b/test/phpunit/Header/ParserTest.php index 705e337..faa9bc6 100644 --- a/test/phpunit/Header/ParserTest.php +++ b/test/phpunit/Header/ParserTest.php @@ -73,4 +73,18 @@ public function testGetKeyValuesResponse() { self::assertArrayHasKey("X-Test-For", $keyValues); self::assertEquals("PHP.Gt", $keyValues["X-Test-For"]); } + + public function testGetKeyValuesCombinesRepeatedListValuedHeaders():void { + $headers = <<getKeyValues()["Accept"] + ); + } } diff --git a/test/phpunit/RequestTest.php b/test/phpunit/RequestTest.php index b833791..d1e11f5 100644 --- a/test/phpunit/RequestTest.php +++ b/test/phpunit/RequestTest.php @@ -115,6 +115,26 @@ public function testWithFormDataBodyAutomaticallySetsContentTypeHeader():void { ); } + public function testGetHeaderReturnsAllValuesAcrossSeparateHeaderLines():void { + $headers = new RequestHeaders(); + $headers->add("WWW-Authenticate", "Digest realm=\"example.com\", qop=\"auth\""); + $headers->add("WWW-Authenticate", "Digest realm=\"api.example.com\", qop=\"auth-int\""); + + $request = new Request( + "GET", + self::getUriMock("/"), + $headers + ); + + self::assertSame( + [ + "Digest realm=\"example.com\", qop=\"auth\"", + "Digest realm=\"api.example.com\", qop=\"auth-int\"", + ], + $request->getHeader("WWW-Authenticate") + ); + } + /** @return MockObject|Uri */ protected function getUriMock(string $uriPath = ""):MockObject { $partPath = parse_url($uriPath, PHP_URL_PATH); From 8d8b40992131dcf57115bc6d254960aec4f87a37 Mon Sep 17 00:00:00 2001 From: Greg Bowler Date: Fri, 20 Mar 2026 14:45:04 +0000 Subject: [PATCH 2/2] fix: implement fix for multi-line headers closes #72 --- .gitignore | 1 + src/Header/HeaderLine.php | 2 +- src/Header/Headers.php | 69 ++++++++++++++++++++--------- src/Header/Parser.php | 16 ++++++- test/phpunit/Header/HeadersTest.php | 36 +++++++-------- test/phpunit/RequestTest.php | 10 ++--- 6 files changed, 87 insertions(+), 47 deletions(-) diff --git a/.gitignore b/.gitignore index 8f318fb..9d55128 100644 --- a/.gitignore +++ b/.gitignore @@ -3,3 +3,4 @@ /test/phpunit/_coverage /*.phar .phpunit.*.cache +/test/phpunit/.phpunit.cache/ diff --git a/src/Header/HeaderLine.php b/src/Header/HeaderLine.php index f9afc69..078cfb0 100644 --- a/src/Header/HeaderLine.php +++ b/src/Header/HeaderLine.php @@ -17,7 +17,7 @@ public function __construct(string $name, string...$values) { } public function __toString():string { - if(in_array($this->name, Headers::COMMA_HEADERS)) { + if(in_array($this->name, Headers::NON_COMBINABLE_HEADERS)) { return $this->getValuesNewlineSeparated(); } else { diff --git a/src/Header/Headers.php b/src/Header/Headers.php index 6d8ee3c..f7f2835 100644 --- a/src/Header/Headers.php +++ b/src/Header/Headers.php @@ -13,11 +13,8 @@ class Headers implements Iterator, Countable, TypeSafeGetter { use NullableTypeSafeGetter; - const COMMA_HEADERS = [ -// These cookies use commas within the value, so can't be comma separated. - "cookie-set", - "www-authenticate", - "proxy-authenticate" + const NON_COMBINABLE_HEADERS = [ + "set-cookie", ]; /** @var HeaderLine[] */ @@ -43,17 +40,29 @@ public function asArray(bool $nested = false):array { foreach($this->headerLines as $header) { $name = $header->getName(); + $nameLower = strtolower($name); if($nested) { - $array[$name] = $header->getValues(); + $array[$name] ??= []; + $array[$name] = array_merge($array[$name], $header->getValues()); continue; } - if(in_array(strtolower($name), self::COMMA_HEADERS)) { - $array[$name] = $header->getValuesNewlineSeparated(); + if(!array_key_exists($name, $array)) { + $array[$name] = ""; + } + + if(in_array($nameLower, self::NON_COMBINABLE_HEADERS)) { + if($array[$name] !== "") { + $array[$name] .= "\n"; + } + $array[$name] .= $header->getValuesNewlineSeparated(); } else { - $array[$name] = $header->getValuesCommaSeparated(); + if($array[$name] !== "") { + $array[$name] .= ","; + } + $array[$name] .= $header->getValuesCommaSeparated(); } } @@ -67,7 +76,14 @@ public function fromArray(array $headerArray):void { $value = [$value]; } - $this->headerLines []= new HeaderLine($key, ...$value); + if(in_array(strtolower($key), self::NON_COMBINABLE_HEADERS)) { + foreach($value as $singleValue) { + array_push($this->headerLines, new HeaderLine($key, $singleValue)); + } + } + else { + array_push($this->headerLines, new HeaderLine($key, ...$value)); + } } } @@ -82,10 +98,11 @@ public function contains(string $name):bool { } public function add(string $name, string...$values):void { - $isCommaHeader = false; - if(strstr($values[0], ",") - && in_array(strtolower($name), self::COMMA_HEADERS)) { - $isCommaHeader = true; + if(in_array(strtolower($name), self::NON_COMBINABLE_HEADERS)) { + foreach($values as $value) { + array_push($this->headerLines, new HeaderLine($name, $value)); + } + return; } $headerLineToAdd = null; @@ -97,7 +114,7 @@ public function add(string $name, string...$values):void { $headerLineToAdd = $headerLine; } - if(is_null($headerLineToAdd) || $isCommaHeader) { + if(is_null($headerLineToAdd)) { array_push( $this->headerLines, new HeaderLine($name, ...$values) @@ -123,24 +140,34 @@ public function remove(string $name):void { } public function get(string $name):?HeaderLine { + $matchingValues = []; + $headerName = null; + foreach($this->headerLines as $line) { if($line->isNamed($name)) { - return $line; + $headerName ??= $line->getName(); + $matchingValues = array_merge($matchingValues, $line->getValues()); } } - return null; + if(!$headerName) { + return null; + } + + return new HeaderLine($headerName, ...$matchingValues); } - /** @return null|array */ - public function getAll(string $name):?array { + /** @return array */ + public function getAll(string $name):array { + $allValues = []; + foreach($this->headerLines as $line) { if($line->isNamed($name)) { - return $line->getValues(); + $allValues = array_merge($allValues, $line->getValues()); } } - return null; + return $allValues; } public function getFirst():string { diff --git a/src/Header/Parser.php b/src/Header/Parser.php index 7a259e5..8a2b971 100644 --- a/src/Header/Parser.php +++ b/src/Header/Parser.php @@ -31,8 +31,20 @@ public function getKeyValues():array { $kvp = explode(":", $h, 2); $key = $kvp[0]; $value = $kvp[1] ?? ""; + $value = trim($value); - $keyValues[$key] = trim($value); + if(array_key_exists($key, $keyValues)) { + if(strtolower($key) === "set-cookie") { + $keyValues[$key] .= "\n" . $value; + } + else { + $keyValues[$key] .= ", " . $value; + } + + continue; + } + + $keyValues[$key] = $value; } return $keyValues; @@ -50,6 +62,6 @@ protected function pregMatchProtocol(string $matchName):string { return ""; } - return (string)($matches[$matchName] ?? ""); + return $matches[$matchName] ?? ""; } } diff --git a/test/phpunit/Header/HeadersTest.php b/test/phpunit/Header/HeadersTest.php index 5ebf8f8..356f822 100644 --- a/test/phpunit/Header/HeadersTest.php +++ b/test/phpunit/Header/HeadersTest.php @@ -139,42 +139,42 @@ public function testAddMultipleSetCookieWithoutCommaPreservesSeparateLines():voi ); } - public function testGetAllAggregatesValuesAcrossSeparateHeaderLines():void { - $firstValue = "Digest realm=\"example.com\", qop=\"auth\""; - $secondValue = "Digest realm=\"api.example.com\", qop=\"auth-int\""; + public function testGetAllAggregatesValuesAcrossSeparateSetCookieHeaderLines():void { + $firstValue = "language=en; Path=/"; + $secondValue = "id=123; HttpOnly"; $headers = new Headers(self::HEADER_ARRAY); - $headers->add("WWW-Authenticate", $firstValue); - $headers->add("WWW-Authenticate", $secondValue); + $headers->add("Set-Cookie", $firstValue); + $headers->add("Set-Cookie", $secondValue); self::assertSame( [$firstValue, $secondValue], - $headers->getAll("WWW-Authenticate") + $headers->getAll("Set-Cookie") ); } - public function testAsArrayPreservesAllDuplicateSpecialCaseHeaderLines():void { - $firstValue = "Digest realm=\"example.com\", qop=\"auth\""; - $secondValue = "Digest realm=\"api.example.com\", qop=\"auth-int\""; + public function testAsArrayPreservesAllDuplicateSetCookieHeaderLines():void { + $firstValue = "language=en; Path=/"; + $secondValue = "id=123; HttpOnly"; $headers = new Headers(self::HEADER_ARRAY); - $headers->add("WWW-Authenticate", $firstValue); - $headers->add("WWW-Authenticate", $secondValue); + $headers->add("Set-Cookie", $firstValue); + $headers->add("Set-Cookie", $secondValue); self::assertSame( $firstValue . "\n" . $secondValue, - $headers->asArray()["WWW-Authenticate"] + $headers->asArray()["Set-Cookie"] ); } - public function testAsArrayNestedPreservesAllDuplicateHeaderLineValues():void { - $firstValue = "Digest realm=\"example.com\", qop=\"auth\""; - $secondValue = "Digest realm=\"api.example.com\", qop=\"auth-int\""; + public function testAsArrayNestedPreservesAllDuplicateSetCookieHeaderLineValues():void { + $firstValue = "language=en; Path=/"; + $secondValue = "id=123; HttpOnly"; $headers = new Headers(self::HEADER_ARRAY); - $headers->add("WWW-Authenticate", $firstValue); - $headers->add("WWW-Authenticate", $secondValue); + $headers->add("Set-Cookie", $firstValue); + $headers->add("Set-Cookie", $secondValue); self::assertSame( [$firstValue, $secondValue], - $headers->asArray(true)["WWW-Authenticate"] + $headers->asArray(true)["Set-Cookie"] ); } diff --git a/test/phpunit/RequestTest.php b/test/phpunit/RequestTest.php index d1e11f5..8f0bddb 100644 --- a/test/phpunit/RequestTest.php +++ b/test/phpunit/RequestTest.php @@ -117,8 +117,8 @@ public function testWithFormDataBodyAutomaticallySetsContentTypeHeader():void { public function testGetHeaderReturnsAllValuesAcrossSeparateHeaderLines():void { $headers = new RequestHeaders(); - $headers->add("WWW-Authenticate", "Digest realm=\"example.com\", qop=\"auth\""); - $headers->add("WWW-Authenticate", "Digest realm=\"api.example.com\", qop=\"auth-int\""); + $headers->add("Set-Cookie", "language=en; Path=/"); + $headers->add("Set-Cookie", "id=123; HttpOnly"); $request = new Request( "GET", @@ -128,10 +128,10 @@ public function testGetHeaderReturnsAllValuesAcrossSeparateHeaderLines():void { self::assertSame( [ - "Digest realm=\"example.com\", qop=\"auth\"", - "Digest realm=\"api.example.com\", qop=\"auth-int\"", + "language=en; Path=/", + "id=123; HttpOnly", ], - $request->getHeader("WWW-Authenticate") + $request->getHeader("Set-Cookie") ); }