Skip to content

Commit

Permalink
Merge pull request #214 from pfk84/main
Browse files Browse the repository at this point in the history 
Consistent handling for HTTP responses with multiple header values (PHP SAPI)
  • Loading branch information
@clue
clue authored Feb 22, 2023
2 parents 9e97ad3 + ca6d381 commit 2ce0b5d
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 23 deletions.
6 changes: 6 additions & 0 deletions examples/index.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,12 @@
);
});

$app->get('/set-cookie', function (ServerRequestInterface $request) {
return React\Http\Message\Response::plaintext(
"Cookies have been set.\n"
)->withHeader('Set-Cookie', '1=1')->withAddedHeader('Set-Cookie', '2=2');
});

$app->get('/error', function () {
throw new RuntimeException('Unable to load error');
});
Expand Down
2 changes: 1 addition & 1 deletion src/Io/SapiHandler.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ public function sendResponse(ResponseInterface $response): void
ini_set('default_charset', '');
foreach ($response->getHeaders() as $name => $values) {
foreach ($values as $value) {
header($name . ': ' . $value);
header($name . ': ' . $value, false);
}
}
ini_set('default_charset', $old);
Expand Down
63 changes: 41 additions & 22 deletions tests/Io/SapiHandlerTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,12 +144,14 @@ public function testSendResponseSendsEmptyResponseWithNoHeadersAndEmptyBodyAndAs
$this->markTestSkipped('Test requires running phpunit with --stderr and Xdebug enabled');
}

header_remove();
$_SERVER['SERVER_PROTOCOL'] = 'http/1.1';
$sapi = new SapiHandler();
$response = new Response(200, [], '');

$this->expectOutputString('');
$sapi->sendResponse($response);

$this->assertEquals(['Content-Type:', 'Content-Length: 0'], xdebug_get_headers());
}

Expand All @@ -159,15 +161,15 @@ public function testSendResponseSendsJsonResponseWithGivenHeadersAndBodyAndAssig
$this->markTestSkipped('Test requires running phpunit with --stderr and Xdebug enabled');
}

header_remove();
$_SERVER['SERVER_PROTOCOL'] = 'http/1.1';
$sapi = new SapiHandler();
$response = new Response(200, ['Content-Type' => 'application/json'], '{}');

$this->expectOutputString('{}');
$sapi->sendResponse($response);

$previous = ['Content-Type:'];
$this->assertEquals(array_merge($previous, ['Content-Type: application/json', 'Content-Length: 2']), xdebug_get_headers());
$this->assertEquals(['Content-Type: application/json', 'Content-Length: 2'], xdebug_get_headers());
}

/**
Expand All @@ -179,6 +181,7 @@ public function testSendResponseSendsJsonResponseWithGivenHeadersAndMatchingCont
$this->markTestSkipped('Test requires running phpunit with --stderr and Xdebug enabled');
}

header_remove();
$_SERVER['REQUEST_METHOD'] = 'HEAD';
$_SERVER['SERVER_PROTOCOL'] = 'http/1.1';
$sapi = new SapiHandler();
Expand All @@ -187,8 +190,7 @@ public function testSendResponseSendsJsonResponseWithGivenHeadersAndMatchingCont
$this->expectOutputString('');
$sapi->sendResponse($response);

$previous = ['Content-Type:'];
$this->assertEquals(array_merge($previous, ['Content-Type: application/json', 'Content-Length: 2']), xdebug_get_headers());
$this->assertEquals(['Content-Type: application/json', 'Content-Length: 2'], xdebug_get_headers());
}

public function testSendResponseSendsEmptyBodyWithGivenHeadersAndAssignsNoContentLengthForNoContentResponse(): void
Expand All @@ -197,15 +199,15 @@ public function testSendResponseSendsEmptyBodyWithGivenHeadersAndAssignsNoConten
$this->markTestSkipped('Test requires running phpunit with --stderr and Xdebug enabled');
}

header_remove();
$_SERVER['SERVER_PROTOCOL'] = 'http/1.1';
$sapi = new SapiHandler();
$response = new Response(204, ['Content-Type' => 'application/json'], '{}');

$this->expectOutputString('');
$sapi->sendResponse($response);

$previous = ['Content-Type:', 'Content-Length: 2'];
$this->assertEquals(array_merge($previous, ['Content-Type: application/json']), xdebug_get_headers());
$this->assertEquals(['Content-Type: application/json'], xdebug_get_headers());
}

public function testSendResponseSendsEmptyBodyWithGivenHeadersButWithoutExplicitContentLengthForNoContentResponse(): void
Expand All @@ -214,15 +216,15 @@ public function testSendResponseSendsEmptyBodyWithGivenHeadersButWithoutExplicit
$this->markTestSkipped('Test requires running phpunit with --stderr and Xdebug enabled');
}

header_remove();
$_SERVER['SERVER_PROTOCOL'] = 'http/1.1';
$sapi = new SapiHandler();
$response = new Response(204, ['Content-Type' => 'application/json', 'Content-Length' => '2'], '{}');

$this->expectOutputString('');
$sapi->sendResponse($response);

$previous = ['Content-Type:', 'Content-Length: 2'];
$this->assertEquals(array_merge($previous, ['Content-Type: application/json']), xdebug_get_headers());
$this->assertEquals(['Content-Type: application/json'], xdebug_get_headers());
}

public function testSendResponseSendsEmptyBodyWithGivenHeadersAndAssignsContentLengthForNotModifiedResponse(): void
Expand All @@ -231,15 +233,15 @@ public function testSendResponseSendsEmptyBodyWithGivenHeadersAndAssignsContentL
$this->markTestSkipped('Test requires running phpunit with --stderr and Xdebug enabled');
}

header_remove();
$_SERVER['SERVER_PROTOCOL'] = 'http/1.1';
$sapi = new SapiHandler();
$response = new Response(304, ['Content-Type' => 'application/json'], 'null');

$this->expectOutputString('');
$sapi->sendResponse($response);

$previous = ['Content-Type:'];
$this->assertEquals(array_merge($previous, ['Content-Type: application/json', 'Content-Length: 4']), xdebug_get_headers());
$this->assertEquals(['Content-Type: application/json', 'Content-Length: 4'], xdebug_get_headers());
}

public function testSendResponseSendsEmptyBodyWithGivenHeadersAndExplicitContentLengthForNotModifiedResponse(): void
Expand All @@ -248,15 +250,15 @@ public function testSendResponseSendsEmptyBodyWithGivenHeadersAndExplicitContent
$this->markTestSkipped('Test requires running phpunit with --stderr and Xdebug enabled');
}

header_remove();
$_SERVER['SERVER_PROTOCOL'] = 'http/1.1';
$sapi = new SapiHandler();
$response = new Response(304, ['Content-Type' => 'application/json', 'Content-Length' => '2'], '');

$this->expectOutputString('');
$sapi->sendResponse($response);

$previous = ['Content-Type:'];
$this->assertEquals(array_merge($previous, ['Content-Type: application/json', 'Content-Length: 2']), xdebug_get_headers());
$this->assertEquals(['Content-Type: application/json', 'Content-Length: 2'], xdebug_get_headers());
}

public function testSendResponseSendsStreamingResponseWithNoHeadersAndBodyFromStreamData(): void
Expand All @@ -265,6 +267,7 @@ public function testSendResponseSendsStreamingResponseWithNoHeadersAndBodyFromSt
$this->markTestSkipped('Test requires running phpunit with --stderr and Xdebug enabled');
}

header_remove();
$_SERVER['SERVER_PROTOCOL'] = 'http/1.1';
$sapi = new SapiHandler();
$body = new ThroughStream();
Expand All @@ -273,8 +276,7 @@ public function testSendResponseSendsStreamingResponseWithNoHeadersAndBodyFromSt
$this->expectOutputString('test');
$sapi->sendResponse($response);

$previous = ['Content-Type:', 'Content-Length: 2'];
$this->assertEquals(array_merge($previous, ['Content-Type:']), xdebug_get_headers());
$this->assertEquals(['Content-Type:'], xdebug_get_headers());

$body->end('test');
}
Expand All @@ -288,6 +290,7 @@ public function testSendResponseClosesStreamingResponseAndSendsResponseWithNoHea
$this->markTestSkipped('Test requires running phpunit with --stderr and Xdebug enabled');
}

header_remove();
$_SERVER['REQUEST_METHOD'] = 'HEAD';
$_SERVER['SERVER_PROTOCOL'] = 'http/1.1';
$sapi = new SapiHandler();
Expand All @@ -297,8 +300,7 @@ public function testSendResponseClosesStreamingResponseAndSendsResponseWithNoHea
$this->expectOutputString('');
$sapi->sendResponse($response);

$previous = ['Content-Type:', 'Content-Length: 2', 'Content-Type:'];
$this->assertEquals(array_merge($previous, ['Content-Type:']), xdebug_get_headers());
$this->assertEquals(['Content-Type:'], xdebug_get_headers());
$this->assertFalse($body->isReadable());
}

Expand All @@ -308,6 +310,7 @@ public function testSendResponseClosesStreamingResponseAndSendsResponseWithNoHea
$this->markTestSkipped('Test requires running phpunit with --stderr and Xdebug enabled');
}

header_remove();
$_SERVER['SERVER_PROTOCOL'] = 'http/1.1';
$sapi = new SapiHandler();
$body = new ThroughStream();
Expand All @@ -316,8 +319,7 @@ public function testSendResponseClosesStreamingResponseAndSendsResponseWithNoHea
$this->expectOutputString('');
$sapi->sendResponse($response);

$previous = ['Content-Type:', 'Content-Length: 2', 'Content-Type:', 'Content-Type:'];
$this->assertEquals(array_merge($previous, ['Content-Type:']), xdebug_get_headers());
$this->assertEquals(['Content-Type:'], xdebug_get_headers());
$this->assertFalse($body->isReadable());
}

Expand All @@ -327,6 +329,7 @@ public function testSendResponseClosesStreamingResponseAndSendsResponseWithNoHea
$this->markTestSkipped('Test requires running phpunit with --stderr and Xdebug enabled');
}

header_remove();
$_SERVER['SERVER_PROTOCOL'] = 'http/1.1';
$sapi = new SapiHandler();
$body = new ThroughStream();
Expand All @@ -335,8 +338,7 @@ public function testSendResponseClosesStreamingResponseAndSendsResponseWithNoHea
$this->expectOutputString('');
$sapi->sendResponse($response);

$previous = ['Content-Type:', 'Content-Length: 2', 'Content-Type:', 'Content-Type:', 'Content-Type:'];
$this->assertEquals(array_merge($previous, ['Content-Type:']), xdebug_get_headers());
$this->assertEquals(['Content-Type:'], xdebug_get_headers());
$this->assertFalse($body->isReadable());
}

Expand All @@ -346,6 +348,7 @@ public function testSendResponseSendsStreamingResponseWithNoHeadersAndBodyFromSt
$this->markTestSkipped('Test requires running phpunit with --stderr and Xdebug enabled');
}

header_remove();
$_SERVER['SERVER_PROTOCOL'] = 'http/1.1';
$_SERVER['SERVER_SOFTWARE'] = 'nginx/1';
$sapi = new SapiHandler();
Expand All @@ -355,12 +358,28 @@ public function testSendResponseSendsStreamingResponseWithNoHeadersAndBodyFromSt
$this->expectOutputString('test');
$sapi->sendResponse($response);

$previous = ['Content-Type:', 'Content-Length: 2', 'Content-Type:', 'Content-Type:', 'Content-Type:', 'Content-Type:'];
$this->assertEquals(array_merge($previous, ['Content-Type:', 'X-Accel-Buffering: no']), xdebug_get_headers());
$this->assertEquals(['Content-Type:', 'X-Accel-Buffering: no'], xdebug_get_headers());

$body->end('test');
}

public function testSendResponseSetsMultipleCookieHeaders(): void
{
if (headers_sent() || !function_exists('xdebug_get_headers')) {
$this->markTestSkipped('Test requires running phpunit with --stderr and Xdebug enabled');
}

header_remove();
$_SERVER['SERVER_PROTOCOL'] = 'http/1.1';
$sapi = new SapiHandler();
$response = new Response(204, ['Set-Cookie' => ['1=1', '2=2']], '');

$this->expectOutputString('');
$sapi->sendResponse($response);

$this->assertEquals(['Content-Type:', 'Set-Cookie: 1=1', 'Set-Cookie: 2=2'], xdebug_get_headers());
}

public function testLogPrintsMessageWithCurrentDateAndTime(): void
{
// 2021-01-29 12:22:01.717 Hello\n
Expand Down
2 changes: 2 additions & 0 deletions tests/acceptance.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,8 @@ out=$(curl -v $base/headers -H 'Content-Type;' 2>&1); skipif "Server: Apac
out=$(curl -v $base/headers -H 'DNT: 1' 2>&1); skipif "Server: nginx" && match "HTTP/.* 200" && match "\"DNT\"" && notmatch "\"Dnt\"" # skip nginx which doesn't report original case (DNT->Dnt)
out=$(curl -v $base/headers -H 'V: a' -H 'V: b' 2>&1); skipif "Server: nginx" && skipif -v "Server:" && match "HTTP/.* 200" && match "\"V\": \"a, b\"" # skip nginx (last only) and PHP webserver (first only)

out=$(curl -v $base/set-cookie 2>&1); match "HTTP/.* 200" && match "Set-Cookie: 1=1" && match "Set-Cookie: 2=2"

out=$(curl -v --proxy $baseWithPort $base/debug 2>&1); skipif "Server: nginx" && match "HTTP/.* 400" # skip nginx (continues like direct request)
out=$(curl -v --proxy $baseWithPort -p $base/debug 2>&1); skipif "CONNECT aborted" && match "HTTP/.* 400" # skip PHP development server (rejects as "Malformed HTTP request")

Expand Down

0 comments on commit 2ce0b5d

Please sign in to comment.