From 603950d7fd36543d82158930ea685458e9f29809 Mon Sep 17 00:00:00 2001 From: Calvin Buckley Date: Wed, 1 Mar 2023 13:30:15 -0400 Subject: [PATCH 1/7] `http_response_code` should warn if headers already sent This would fail silently otherwise. The warning should be similar to the one that header emits (the code is some copy and paste from main/SAPI.c, to match). It'll also return false in that case. Fixes GH-10742 --- ext/standard/head.c | 11 +++++++++++ .../tests/general_functions/http_response_code.phpt | 11 ++++++++++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/ext/standard/head.c b/ext/standard/head.c index 5bdae98dfce56..c440edc72e7e4 100644 --- a/ext/standard/head.c +++ b/ext/standard/head.c @@ -363,6 +363,17 @@ PHP_FUNCTION(http_response_code) if (response_code) { + if (SG(headers_sent) && !SG(request_info).no_headers) { + const char *output_start_filename = php_output_get_start_filename(); + int output_start_lineno = php_output_get_start_lineno(); + + if (output_start_filename) { + php_error_docref(NULL, E_WARNING, "Cannot set response code - headers already sent by (output started at %s:%d)", output_start_filename, output_start_lineno); + } else { + php_error_docref(NULL, E_WARNING, "Cannot set response code - headers already sent"); + } + RETURN_FALSE; + } zend_long old_response_code; old_response_code = SG(sapi_headers).http_response_code; diff --git a/ext/standard/tests/general_functions/http_response_code.phpt b/ext/standard/tests/general_functions/http_response_code.phpt index ab290c3cefe19..313e857e7f176 100644 --- a/ext/standard/tests/general_functions/http_response_code.phpt +++ b/ext/standard/tests/general_functions/http_response_code.phpt @@ -21,8 +21,17 @@ var_dump( // Get the new response code http_response_code() ); +echo "Now we've sent the headers\n"; +var_dump( + // This should fail + http_response_code(500) +); ?> ---EXPECT-- +--EXPECTF-- bool(false) bool(true) int(201) +Now we've sent the headers + +Warning: http_response_code(): Cannot set response code - headers already sent by (output started at %s:%d) in %s on line %d +bool(false) From 33d1110bb356082affe63e0388bc58aa74526166 Mon Sep 17 00:00:00 2001 From: Calvin Buckley Date: Wed, 1 Mar 2023 13:32:58 -0400 Subject: [PATCH 2/7] The line number is unsigned --- ext/standard/head.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ext/standard/head.c b/ext/standard/head.c index c440edc72e7e4..75c1cb81a8fd9 100644 --- a/ext/standard/head.c +++ b/ext/standard/head.c @@ -365,10 +365,10 @@ PHP_FUNCTION(http_response_code) { if (SG(headers_sent) && !SG(request_info).no_headers) { const char *output_start_filename = php_output_get_start_filename(); - int output_start_lineno = php_output_get_start_lineno(); + uint32_t output_start_lineno = php_output_get_start_lineno(); if (output_start_filename) { - php_error_docref(NULL, E_WARNING, "Cannot set response code - headers already sent by (output started at %s:%d)", output_start_filename, output_start_lineno); + php_error_docref(NULL, E_WARNING, "Cannot set response code - headers already sent by (output started at %s:%u)", output_start_filename, output_start_lineno); } else { php_error_docref(NULL, E_WARNING, "Cannot set response code - headers already sent"); } From 04e2b9cc8257f669afa9e17d1c60302a75fd5cb3 Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Wed, 22 Mar 2023 16:50:38 -0300 Subject: [PATCH 3/7] Fix SAPI test --- sapi/fpm/tests/log-suppress-output.phpt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sapi/fpm/tests/log-suppress-output.phpt b/sapi/fpm/tests/log-suppress-output.phpt index 5a5e7bb9544ba..a507180e99227 100644 --- a/sapi/fpm/tests/log-suppress-output.phpt +++ b/sapi/fpm/tests/log-suppress-output.phpt @@ -38,7 +38,7 @@ function doTestCalls(FPM\Tester &$tester, bool $expectSuppressableEntries) $tester->request(query: 'test=output', uri: '/ping')->expectBody('pong', 'text/plain'); $tester->expectAccessLog("'GET /ping?test=output' 200", suppressable: false); - $tester->request(headers: ['X_ERROR' => 1])->expectBody('Not OK'); + $tester->request(headers: ['X_ERROR' => 1])->expectStatus('500 Internal Server Error')->expectBody('Not OK'); $tester->expectAccessLog("'GET /log-suppress-output.src.php' 500", suppressable: false); $tester->request()->expectBody('OK'); @@ -54,8 +54,8 @@ function doTestCalls(FPM\Tester &$tester, bool $expectSuppressableEntries) $src = << Date: Tue, 28 Mar 2023 15:55:13 -0300 Subject: [PATCH 4/7] output line number is actually defined as an int --- ext/standard/head.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ext/standard/head.c b/ext/standard/head.c index 75c1cb81a8fd9..1efbf27df82cc 100644 --- a/ext/standard/head.c +++ b/ext/standard/head.c @@ -365,10 +365,10 @@ PHP_FUNCTION(http_response_code) { if (SG(headers_sent) && !SG(request_info).no_headers) { const char *output_start_filename = php_output_get_start_filename(); - uint32_t output_start_lineno = php_output_get_start_lineno(); + int output_start_lineno = php_output_get_start_lineno(); if (output_start_filename) { - php_error_docref(NULL, E_WARNING, "Cannot set response code - headers already sent by (output started at %s:%u)", output_start_filename, output_start_lineno); + php_error_docref(NULL, E_WARNING, "Cannot set response code - headers already sent by (output started at %s:%"PRIi32")", output_start_filename, output_start_lineno); } else { php_error_docref(NULL, E_WARNING, "Cannot set response code - headers already sent"); } From 1f17995129810bcbe59987d4c767836e3522d5c6 Mon Sep 17 00:00:00 2001 From: Calvin Buckley Date: Wed, 29 Mar 2023 11:08:19 -0300 Subject: [PATCH 5/7] Clean up message for already sent response code --- ext/standard/head.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/standard/head.c b/ext/standard/head.c index 1efbf27df82cc..a1a7198f39623 100644 --- a/ext/standard/head.c +++ b/ext/standard/head.c @@ -368,7 +368,7 @@ PHP_FUNCTION(http_response_code) int output_start_lineno = php_output_get_start_lineno(); if (output_start_filename) { - php_error_docref(NULL, E_WARNING, "Cannot set response code - headers already sent by (output started at %s:%"PRIi32")", output_start_filename, output_start_lineno); + php_error_docref(NULL, E_WARNING, "Cannot set response code - headers already sent (output started at %s:%"PRIi32")", output_start_filename, output_start_lineno); } else { php_error_docref(NULL, E_WARNING, "Cannot set response code - headers already sent"); } From 16300ddc18505442e9c6f2911d6c8e8f790f056f Mon Sep 17 00:00:00 2001 From: Calvin Buckley Date: Wed, 29 Mar 2023 12:01:55 -0300 Subject: [PATCH 6/7] Oops, forgot to adjust test to match --- ext/standard/tests/general_functions/http_response_code.phpt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/standard/tests/general_functions/http_response_code.phpt b/ext/standard/tests/general_functions/http_response_code.phpt index 313e857e7f176..8f8b87511a3b9 100644 --- a/ext/standard/tests/general_functions/http_response_code.phpt +++ b/ext/standard/tests/general_functions/http_response_code.phpt @@ -33,5 +33,5 @@ bool(true) int(201) Now we've sent the headers -Warning: http_response_code(): Cannot set response code - headers already sent by (output started at %s:%d) in %s on line %d +Warning: http_response_code(): Cannot set response code - headers already sent (output started at %s:%d) in %s on line %d bool(false) From 2fa29d9fc9fb0cfe4e007d467ccf8cfd297795d9 Mon Sep 17 00:00:00 2001 From: Calvin Buckley Date: Mon, 17 Apr 2023 10:43:11 -0300 Subject: [PATCH 7/7] Change format string back to %d and wrap --- ext/standard/head.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ext/standard/head.c b/ext/standard/head.c index a1a7198f39623..7d223c646f215 100644 --- a/ext/standard/head.c +++ b/ext/standard/head.c @@ -368,7 +368,8 @@ PHP_FUNCTION(http_response_code) int output_start_lineno = php_output_get_start_lineno(); if (output_start_filename) { - php_error_docref(NULL, E_WARNING, "Cannot set response code - headers already sent (output started at %s:%"PRIi32")", output_start_filename, output_start_lineno); + php_error_docref(NULL, E_WARNING, "Cannot set response code - headers already sent " + "(output started at %s:%d)", output_start_filename, output_start_lineno); } else { php_error_docref(NULL, E_WARNING, "Cannot set response code - headers already sent"); }