Skip to content

Commit a054ef2

Browse files
cmb69cmb69
committed
Fix #80849: HTTP Status header truncation
While truncating the contents of a header is okay, we must never omit the trailing CRLF. Closes GH-7238.
1 parent a942cf5 commit a054ef2

File tree

5 files changed

+63
-8
lines changed

5 files changed

+63
-8
lines changed

‎NEWS

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@ PHP NEWS
55
- Core:
66
. Fixed bug #72595 (php_output_handler_append illegal write access). (cmb)
77

8+
- CGI:
9+
. Fixed bug #80849 (HTTP Status header truncation). (cmb)
10+
811
- Standard:
912
. Fixed bug #72146 (Integer overflow on substr_replace). (cmb)
1013

‎sapi/cgi/cgi_main.c

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -387,7 +387,7 @@ static int sapi_cgi_send_headers(sapi_headers_struct *sapi_headers)
387387

388388
if (CGIG(rfc2616_headers) && SG(sapi_headers).http_status_line) {
389389
char *s;
390-
len = slprintf(buf, SAPI_CGI_MAX_HEADER_LENGTH, "%s\r\n", SG(sapi_headers).http_status_line);
390+
len = slprintf(buf, SAPI_CGI_MAX_HEADER_LENGTH, "%s", SG(sapi_headers).http_status_line);
391391
if ((s = strchr(SG(sapi_headers).http_status_line, ' '))) {
392392
response_status = atoi((s + 1));
393393
}
@@ -404,7 +404,7 @@ static int sapi_cgi_send_headers(sapi_headers_struct *sapi_headers)
404404
(s - SG(sapi_headers).http_status_line) >= 5 &&
405405
strncasecmp(SG(sapi_headers).http_status_line, "HTTP/", 5) == 0
406406
) {
407-
len = slprintf(buf, sizeof(buf), "Status:%s\r\n", s);
407+
len = slprintf(buf, sizeof(buf), "Status:%s", s);
408408
response_status = atoi((s + 1));
409409
} else {
410410
h = (sapi_header_struct*)zend_llist_get_first_ex(&sapi_headers->headers, &pos);
@@ -427,16 +427,17 @@ static int sapi_cgi_send_headers(sapi_headers_struct *sapi_headers)
427427
err++;
428428
}
429429
if (err->str) {
430-
len = slprintf(buf, sizeof(buf), "Status: %d %s\r\n", SG(sapi_headers).http_response_code, err->str);
430+
len = slprintf(buf, sizeof(buf), "Status: %d %s", SG(sapi_headers).http_response_code, err->str);
431431
} else {
432-
len = slprintf(buf, sizeof(buf), "Status: %d\r\n", SG(sapi_headers).http_response_code);
432+
len = slprintf(buf, sizeof(buf), "Status: %d", SG(sapi_headers).http_response_code);
433433
}
434434
}
435435
}
436436
}
437437

438438
if (!has_status) {
439439
PHPWRITE_H(buf, len);
440+
PHPWRITE_H("\r\n", 2);
440441
ignore_status = 1;
441442
}
442443
}

‎sapi/cgi/tests/bug80849-cgi.phpt

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
--TEST--
2+
Bug #80849 (HTTP Status header truncation)
3+
--CGI--
4+
--FILE--
5+
<?php
6+
header('HTTP/1.1 201 ' . str_repeat('A', 1014), true);
7+
?>
8+
--EXPECTHEADERS--
9+
Status: 201 AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
10+
--EXPECT--

‎sapi/fpm/fpm/fpm_main.c

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,7 @@ static int sapi_cgi_send_headers(sapi_headers_struct *sapi_headers) /* {{{ */
328328

329329
if (CGIG(rfc2616_headers) && SG(sapi_headers).http_status_line) {
330330
char *s;
331-
len = slprintf(buf, SAPI_CGI_MAX_HEADER_LENGTH, "%s\r\n", SG(sapi_headers).http_status_line);
331+
len = slprintf(buf, SAPI_CGI_MAX_HEADER_LENGTH, "%s", SG(sapi_headers).http_status_line);
332332
if ((s = strchr(SG(sapi_headers).http_status_line, ' '))) {
333333
response_status = atoi((s + 1));
334334
}
@@ -345,7 +345,7 @@ static int sapi_cgi_send_headers(sapi_headers_struct *sapi_headers) /* {{{ */
345345
(s - SG(sapi_headers).http_status_line) >= 5 &&
346346
strncasecmp(SG(sapi_headers).http_status_line, "HTTP/", 5) == 0
347347
) {
348-
len = slprintf(buf, sizeof(buf), "Status:%s\r\n", s);
348+
len = slprintf(buf, sizeof(buf), "Status:%s", s);
349349
response_status = atoi((s + 1));
350350
} else {
351351
h = (sapi_header_struct*)zend_llist_get_first_ex(&sapi_headers->headers, &pos);
@@ -368,16 +368,17 @@ static int sapi_cgi_send_headers(sapi_headers_struct *sapi_headers) /* {{{ */
368368
err++;
369369
}
370370
if (err->str) {
371-
len = slprintf(buf, sizeof(buf), "Status: %d %s\r\n", SG(sapi_headers).http_response_code, err->str);
371+
len = slprintf(buf, sizeof(buf), "Status: %d %s", SG(sapi_headers).http_response_code, err->str);
372372
} else {
373-
len = slprintf(buf, sizeof(buf), "Status: %d\r\n", SG(sapi_headers).http_response_code);
373+
len = slprintf(buf, sizeof(buf), "Status: %d", SG(sapi_headers).http_response_code);
374374
}
375375
}
376376
}
377377
}
378378

379379
if (!has_status) {
380380
PHPWRITE_H(buf, len);
381+
PHPWRITE_H("\r\n", 2);
381382
ignore_status = 1;
382383
}
383384
}

‎sapi/fpm/tests/bug80849-fpm.phpt

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
--TEST--
2+
Bug #80849 (HTTP Status header truncation)
3+
--SKIPIF--
4+
<?php include "skipif.inc"; ?>
5+
--FILE--
6+
<?php
7+
require_once "tester.inc";
8+
9+
$cfg = <<<EOT
10+
[global]
11+
error_log = {{FILE:LOG}}
12+
[unconfined]
13+
listen = {{ADDR}}
14+
pm = dynamic
15+
pm.max_children = 5
16+
pm.start_servers = 1
17+
pm.min_spare_servers = 1
18+
pm.max_spare_servers = 3
19+
EOT;
20+
21+
$code = <<<EOT
22+
<?php
23+
header('HTTP/1.1 201 ' . str_repeat('A', 1014), true);
24+
EOT;
25+
26+
$tester = new FPM\Tester($cfg, $code);
27+
$tester->start();
28+
$tester->expectLogStartNotices();
29+
$tester
30+
->request()
31+
->expectHeader('Status', '201 ' . str_repeat('A', 1011));
32+
$tester->terminate();
33+
$tester->close();
34+
?>
35+
--CLEAN--
36+
<?php
37+
require_once "tester.inc";
38+
FPM\Tester::clean();
39+
?>
40+
--EXPECT--

0 commit comments

Comments
 (0)