Fix bug #64539: FPM status - query_string not properly JSON encoded

Closes GH-11050
php · May 13, 2023 · 5e64ead · 5e64ead
1 parent e8a836e
commit 5e64ead
Show file tree

Hide file tree

Showing 5 changed files with 90 additions and 12 deletions.
diff --git a/NEWS b/NEWS
@@ -17,6 +17,8 @@ PHP                                                                        NEWS
 - FPM:
   . Fixed bug GH-10461 (PHP-FPM segfault due to after free usage of
     child->ev_std(out|err)). (Jakub Zelenka)
+  . Fixed bug #64539 (FPM status page: query_string not properly JSON encoded).
+    (Jakub Zelenka)
 
 - Hash:
   . Fixed bug GH-11180 (hash_file() appears to be restricted to 3 arguments).

diff --git a/sapi/fpm/fpm/fpm_status.c b/sapi/fpm/fpm/fpm_status.c
@@ -13,7 +13,8 @@
 #include "fpm_atomic.h"
 #include "fpm_conf.h"
 #include "fpm_php.h"
-#include <ext/standard/html.h>
+#include "ext/standard/html.h"
+#include "ext/json/php_json.h"
 
 static char *fpm_status_uri = NULL;
 static char *fpm_status_ping_uri = NULL;
@@ -140,7 +141,8 @@ int fpm_status_handle_request(void) /* {{{ */
 	struct fpm_scoreboard_proc_s *proc;
 	char *buffer, *time_format, time_buffer[64];
 	time_t now_epoch;
-	int full, encode, has_start_time;
+	int full, has_start_time;
+	bool encode_html, encode_json;
 	char *short_syntax, *short_post;
 	char *full_pre, *full_syntax, *full_post, *full_separator;
 	zend_string *_GET_str;
@@ -175,7 +177,8 @@ int fpm_status_handle_request(void) /* {{{ */
 		full = (fpm_php_get_string_from_table(_GET_str, "full") != NULL);
 		short_syntax = short_post = NULL;
 		full_separator = full_pre = full_syntax = full_post = NULL;
-		encode = 0;
+		encode_html = false;
+		encode_json = false;
 		has_start_time = 1;
 
 		scoreboard_p = fpm_scoreboard_get();
@@ -218,7 +221,7 @@ int fpm_status_handle_request(void) /* {{{ */
 		if (fpm_php_get_string_from_table(_GET_str, "html")) {
 			sapi_add_header_ex(ZEND_STRL("Content-Type: text/html"), 1, 1);
 			time_format = "%d/%b/%Y:%H:%M:%S %z";
-			encode = 1;
+			encode_html = true;
 
 			short_syntax =
 				"<!DOCTYPE html PUBLIC \"-//W3C//DTD XHTML 1.0 Strict//EN\" \"http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd\">\n"
@@ -287,7 +290,7 @@ int fpm_status_handle_request(void) /* {{{ */
 		} else if (fpm_php_get_string_from_table(_GET_str, "xml")) {
 			sapi_add_header_ex(ZEND_STRL("Content-Type: text/xml"), 1, 1);
 			time_format = "%s";
-			encode = 1;
+			encode_html = true;
 
 			short_syntax =
 				"<?xml version=\"1.0\" ?>\n"
@@ -336,6 +339,8 @@ int fpm_status_handle_request(void) /* {{{ */
 			sapi_add_header_ex(ZEND_STRL("Content-Type: application/json"), 1, 1);
 			time_format = "%s";
 
+			encode_json = true;
+
 			short_syntax =
 				"{"
 				"\"pool\":\"%s\","
@@ -549,11 +554,24 @@ int fpm_status_handle_request(void) /* {{{ */
 				query_string = NULL;
 				tmp_query_string = NULL;
 				if (proc->query_string[0] != '\0') {
-					if (!encode) {
-						query_string = proc->query_string;
+					if (encode_html) {
+						tmp_query_string = php_escape_html_entities_ex(
+								(const unsigned char *) proc->query_string,
+								strlen(proc->query_string), 1, ENT_HTML_IGNORE_ERRORS & ENT_COMPAT,
+								NULL, /* double_encode */ 1, /* quiet */ 0);
+					} else if (encode_json) {
+						tmp_query_string = php_json_encode_string(proc->query_string,
+								strlen(proc->query_string), PHP_JSON_INVALID_UTF8_IGNORE);
 					} else {
-						tmp_query_string = php_escape_html_entities_ex((const unsigned char *) proc->query_string, strlen(proc->query_string), 1, ENT_HTML_IGNORE_ERRORS & ENT_COMPAT, NULL, /* double_encode */ 1, /* quiet */ 0);
+						query_string = proc->query_string;
+					}
+					if (tmp_query_string) {
 						query_string = ZSTR_VAL(tmp_query_string);
+						/* remove quotes around the string */
+						if (encode_json && ZSTR_LEN(tmp_query_string) >= 2) {
+							query_string[ZSTR_LEN(tmp_query_string) - 1] = '\0';
+							++query_string;
+						}
 					}
 				}
 

diff --git a/sapi/fpm/tests/bug64539-status-json-encoding.phpt b/sapi/fpm/tests/bug64539-status-json-encoding.phpt
@@ -0,0 +1,50 @@
+--TEST--
+FPM: bug64539 - status json format escaping
+--SKIPIF--
+<?php
+include "skipif.inc"; ?>
+--FILE--
+<?php
+
+require_once "tester.inc";
+
+$cfg = <<<EOT
+[global]
+error_log = {{FILE:LOG}}
+[unconfined]
+listen = {{ADDR}}
+pm = static
+pm.max_children = 2
+pm.status_path = /status
+catch_workers_output = yes
+EOT;
+
+$code = <<<EOT
+<?php
+usleep(200000);
+EOT;
+
+$tester = new FPM\Tester($cfg, $code);
+$tester->start();
+$tester->expectLogStartNotices();
+$responses = $tester
+    ->multiRequest([
+        ['query' => 'a=b"c'],
+        ['uri' => '/status', 'query' => 'full&json', 'delay' => 100000],
+    ]);
+$data = json_decode($responses[1]->getBody('application/json'), true);
+var_dump(explode('?', $data['processes'][0]['request uri'])[1]);
+$tester->terminate();
+$tester->expectLogTerminatingNotices();
+$tester->close();
+
+?>
+Done
+--EXPECT--
+string(5) "a=b"c"
+Done
+--CLEAN--
+<?php
+require_once "tester.inc";
+FPM\Tester::clean();
+?>
diff --git a/sapi/fpm/tests/response.inc b/sapi/fpm/tests/response.inc
@@ -192,18 +192,22 @@ class Response
 
     /**
      * Print raw body.
+     *
+     * @param string $contentType Expect body to have specified content type.
      */
-    public function dumpBody()
+    public function dumpBody(string $contentType = 'text/html')
     {
-        var_dump($this->getBody());
+        var_dump($this->getBody($contentType));
     }
 
     /**
      * Print raw body.
+     *
+     * @param string $contentType Expect body to have specified content type.
      */
-    public function printBody()
+    public function printBody(string $contentType = 'text/html')
     {
-        echo $this->getBody() . "\n";
+        echo $this->getBody($contentType) . "\n";
     }
 
     /**

diff --git a/sapi/fpm/tests/tester.inc b/sapi/fpm/tests/tester.inc
@@ -795,6 +795,10 @@ class Tester
                     $requestData['uri'] ?? null
                 );
 
+                if (isset($requestData['delay'])) {
+                    usleep($requestData['delay']);
+                }
+
                 return [
                     'client'    => $client,
                     'requestId' => $client->async_request($params, false),