Skip to content

Commit 2ce0b5d

Browse files
clueclue
authored
Merge pull request #214 from pfk84/main
Consistent handling for HTTP responses with multiple header values (PHP SAPI)
2 parents 9e97ad3 + ca6d381 commit 2ce0b5d

File tree

4 files changed

+50
-23
lines changed

4 files changed

+50
-23
lines changed

‎examples/index.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,12 @@
188188
);
189189
});
190190

191+
$app->get('/set-cookie', function (ServerRequestInterface $request) {
192+
return React\Http\Message\Response::plaintext(
193+
"Cookies have been set.\n"
194+
)->withHeader('Set-Cookie', '1=1')->withAddedHeader('Set-Cookie', '2=2');
195+
});
196+
191197
$app->get('/error', function () {
192198
throw new RuntimeException('Unable to load error');
193199
});

‎src/Io/SapiHandler.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ public function sendResponse(ResponseInterface $response): void
108108
ini_set('default_charset', '');
109109
foreach ($response->getHeaders() as $name => $values) {
110110
foreach ($values as $value) {
111-
header($name . ': ' . $value);
111+
header($name . ': ' . $value, false);
112112
}
113113
}
114114
ini_set('default_charset', $old);

‎tests/Io/SapiHandlerTest.php

Lines changed: 41 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -144,12 +144,14 @@ public function testSendResponseSendsEmptyResponseWithNoHeadersAndEmptyBodyAndAs
144144
$this->markTestSkipped('Test requires running phpunit with --stderr and Xdebug enabled');
145145
}
146146

147+
header_remove();
147148
$_SERVER['SERVER_PROTOCOL'] = 'http/1.1';
148149
$sapi = new SapiHandler();
149150
$response = new Response(200, [], '');
150151

151152
$this->expectOutputString('');
152153
$sapi->sendResponse($response);
154+
153155
$this->assertEquals(['Content-Type:', 'Content-Length: 0'], xdebug_get_headers());
154156
}
155157

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

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

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

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

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

184+
header_remove();
182185
$_SERVER['REQUEST_METHOD'] = 'HEAD';
183186
$_SERVER['SERVER_PROTOCOL'] = 'http/1.1';
184187
$sapi = new SapiHandler();
@@ -187,8 +190,7 @@ public function testSendResponseSendsJsonResponseWithGivenHeadersAndMatchingCont
187190
$this->expectOutputString('');
188191
$sapi->sendResponse($response);
189192

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

293+
header_remove();
291294
$_SERVER['REQUEST_METHOD'] = 'HEAD';
292295
$_SERVER['SERVER_PROTOCOL'] = 'http/1.1';
293296
$sapi = new SapiHandler();
@@ -297,8 +300,7 @@ public function testSendResponseClosesStreamingResponseAndSendsResponseWithNoHea
297300
$this->expectOutputString('');
298301
$sapi->sendResponse($response);
299302

300-
$previous = ['Content-Type:', 'Content-Length: 2', 'Content-Type:'];
301-
$this->assertEquals(array_merge($previous, ['Content-Type:']), xdebug_get_headers());
303+
$this->assertEquals(['Content-Type:'], xdebug_get_headers());
302304
$this->assertFalse($body->isReadable());
303305
}
304306

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

313+
header_remove();
311314
$_SERVER['SERVER_PROTOCOL'] = 'http/1.1';
312315
$sapi = new SapiHandler();
313316
$body = new ThroughStream();
@@ -316,8 +319,7 @@ public function testSendResponseClosesStreamingResponseAndSendsResponseWithNoHea
316319
$this->expectOutputString('');
317320
$sapi->sendResponse($response);
318321

319-
$previous = ['Content-Type:', 'Content-Length: 2', 'Content-Type:', 'Content-Type:'];
320-
$this->assertEquals(array_merge($previous, ['Content-Type:']), xdebug_get_headers());
322+
$this->assertEquals(['Content-Type:'], xdebug_get_headers());
321323
$this->assertFalse($body->isReadable());
322324
}
323325

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

332+
header_remove();
330333
$_SERVER['SERVER_PROTOCOL'] = 'http/1.1';
331334
$sapi = new SapiHandler();
332335
$body = new ThroughStream();
@@ -335,8 +338,7 @@ public function testSendResponseClosesStreamingResponseAndSendsResponseWithNoHea
335338
$this->expectOutputString('');
336339
$sapi->sendResponse($response);
337340

338-
$previous = ['Content-Type:', 'Content-Length: 2', 'Content-Type:', 'Content-Type:', 'Content-Type:'];
339-
$this->assertEquals(array_merge($previous, ['Content-Type:']), xdebug_get_headers());
341+
$this->assertEquals(['Content-Type:'], xdebug_get_headers());
340342
$this->assertFalse($body->isReadable());
341343
}
342344

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

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

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

361363
$body->end('test');
362364
}
363365

366+
public function testSendResponseSetsMultipleCookieHeaders(): void
367+
{
368+
if (headers_sent() || !function_exists('xdebug_get_headers')) {
369+
$this->markTestSkipped('Test requires running phpunit with --stderr and Xdebug enabled');
370+
}
371+
372+
header_remove();
373+
$_SERVER['SERVER_PROTOCOL'] = 'http/1.1';
374+
$sapi = new SapiHandler();
375+
$response = new Response(204, ['Set-Cookie' => ['1=1', '2=2']], '');
376+
377+
$this->expectOutputString('');
378+
$sapi->sendResponse($response);
379+
380+
$this->assertEquals(['Content-Type:', 'Set-Cookie: 1=1', 'Set-Cookie: 2=2'], xdebug_get_headers());
381+
}
382+
364383
public function testLogPrintsMessageWithCurrentDateAndTime(): void
365384
{
366385
// 2021-01-29 12:22:01.717 Hello\n

‎tests/acceptance.sh

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,8 @@ out=$(curl -v $base/headers -H 'Content-Type;' 2>&1); skipif "Server: Apac
123123
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)
124124
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)
125125

126+
out=$(curl -v $base/set-cookie 2>&1); match "HTTP/.* 200" && match "Set-Cookie: 1=1" && match "Set-Cookie: 2=2"
127+
126128
out=$(curl -v --proxy $baseWithPort $base/debug 2>&1); skipif "Server: nginx" && match "HTTP/.* 400" # skip nginx (continues like direct request)
127129
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")
128130

0 commit comments

Comments
 (0)