[php-src] master: json: Stop tracking columns in the success path (#22487)
Marc via GitHub <[email protected]>
| Newsgroups | gmane.comp.php.cvs.general |
|---|---|
| Message-ID | <[email protected]> |
Author: Marc (henderkes) Committer: GitHub (web-flow) Pusher: TimWolla Date: 2026-07-13T12:59:42+02:00 Commit: https://github.com/php/php-src/commit/13169cd2fec73f9ea0ecac3c555618c3c32fb8aa Raw diff: https://github.com/php/php-src/commit/13169cd2fec73f9ea0ecac3c555618c3c32fb8aa.diff json: Stop tracking columns in the success path (#22487) Instead of tracking columns during lexing, which costs performance for something that will only be required in the unexpected failure path, we now only track the line numbers and calculate the column on demand when an error occurs. As part of this we also fix GH-22487 by adjusting the logic to report the column based on the (UTF-8) column in the source, instead of treating `\uXXXX` escapes as a single column, which users will need to count manually instead of being able to rely on their editor or `mb_substr()` or similar. [tim: (re)written the commit message] Co-authored-by: Tim Düsterhus <[email protected]> Changed paths: A ext/json/tests/gh22514.phpt M NEWS M ext/json/json.c M ext/json/json_parser.y M ext/json/json_scanner.re M ext/json/php_json.h M ext/json/php_json_parser.h M ext/json/php_json_scanner.h M ext/json/tests/bug68546.phpt M ext/json/tests/json_last_error_msg_error_location_005.phpt Diff: diff --git a/NEWS b/NEWS index 2fa31ab34fde..35eb80d675ca 100644 --- a/NEWS +++ b/NEWS @@ -65,6 +65,10 @@ PHP NEWS - JSON: . Report unterminated JSON strings as syntax errors. (timwolla) + . Improve performance error position tracking during JSON decoding. + (henderkes) + . Fixed bug GH-22514 (Incorrect error column in PHP 8.6 JSON parser). + (henderkes, timwolla) - Opcache: . Fixed bug GH-21770 (Infinite recursion in property hook getter in opcache diff --git a/ext/json/json.c b/ext/json/json.c index ac033c057ac4..04a62f52152f 100644 --- a/ext/json/json.c +++ b/ext/json/json.c @@ -184,7 +184,7 @@ static zend_string *php_json_get_error_msg_with_location(const php_json_error_de const char *base_msg = php_json_get_error_msg(details->code); if (details->line > 0 && details->column > 0) { - return zend_strpprintf(0, "%s near location %zu:%zu", base_msg, details->line, details->column); + return zend_strpprintf(0, "%s near location %" PRIu64 ":%" PRIu64, base_msg, details->line, details->column); } return zend_string_init(base_msg, strlen(base_msg), 0); diff --git a/ext/json/json_parser.y b/ext/json/json_parser.y index 0d3b90b29e1e..0fd3e2c4e364 100644 --- a/ext/json/json_parser.y +++ b/ext/json/json_parser.y @@ -39,7 +39,6 @@ int json_yydebug = 1; } -%locations %define api.prefix {php_json_yy} %define api.pure full %param { php_json_parser *parser } @@ -64,8 +63,8 @@ int json_yydebug = 1; %destructor { zval_ptr_dtor_nogc(&$$); } <value> %code { -static int php_json_yylex(union YYSTYPE *value, YYLTYPE *location, php_json_parser *parser); -static void php_json_yyerror(YYLTYPE *location, php_json_parser *parser, char const *msg); +static int php_json_yylex(union YYSTYPE *value, php_json_parser *parser); +static void php_json_yyerror(php_json_parser *parser, char const *msg); static int php_json_parser_array_create(php_json_parser *parser, zval *array); static int php_json_parser_object_create(php_json_parser *parser, zval *array); @@ -275,7 +274,7 @@ static int php_json_parser_object_update_validate(php_json_parser *parser, zval return SUCCESS; } -static int php_json_yylex(union YYSTYPE *value, YYLTYPE *location, php_json_parser *parser) +static int php_json_yylex(union YYSTYPE *value, php_json_parser *parser) { int token = php_json_scan(&parser->scanner); @@ -291,15 +290,10 @@ static int php_json_yylex(union YYSTYPE *value, YYLTYPE *location, php_json_pars value->value = parser->scanner.value; } - location->first_column = PHP_JSON_SCANNER_LOCATION(parser->scanner, first_column); - location->first_line = PHP_JSON_SCANNER_LOCATION(parser->scanner, first_line); - location->last_column = PHP_JSON_SCANNER_LOCATION(parser->scanner, last_column); - location->last_line = PHP_JSON_SCANNER_LOCATION(parser->scanner, last_line); - return token; } -static void php_json_yyerror(YYLTYPE *location, php_json_parser *parser, char const *msg) +static void php_json_yyerror(php_json_parser *parser, char const *msg) { if (!parser->scanner.errcode) { parser->scanner.errcode = PHP_JSON_ERROR_SYNTAX; @@ -311,11 +305,28 @@ PHP_JSON_API php_json_error_code php_json_parser_error_code(const php_json_parse return parser->scanner.errcode; } +static uint64_t php_json_compute_error_column(const php_json_scanner *s) +{ + const php_json_ctype *p = s->line_start; + const php_json_ctype *end = s->token; + /* Count characters from the start of the line to the failing token, + * folding UTF-8 continuation bytes into their leading byte. */ + uint64_t column = 1; + + while (p < end) { + if ((*p & 0b11000000) != 0b10000000) { + column++; + } + p++; + } + return column; +} + PHP_JSON_API void php_json_parser_error_details(const php_json_parser *parser, php_json_error_details *out) { out->code = parser->scanner.errcode; - out->line = parser->scanner.errloc.first_line; - out->column = parser->scanner.errloc.first_column; + out->line = parser->scanner.line; + out->column = php_json_compute_error_column(&parser->scanner); } static const php_json_parser_methods default_parser_methods = diff --git a/ext/json/json_scanner.re b/ext/json/json_scanner.re index 0c64a6423baf..be62875a00e0 100644 --- a/ext/json/json_scanner.re +++ b/ext/json/json_scanner.re @@ -51,7 +51,6 @@ #define PHP_JSON_INT_MAX_LENGTH (MAX_LENGTH_OF_LONG - 1) #define PHP_JSON_TOKEN_LENGTH() ((size_t) (s->cursor - s->token)) -#define PHP_JSON_TOKEN_LOCATION(location) (s)->errloc.location static void php_json_scanner_copy_string(php_json_scanner *s, size_t esc_size) { @@ -96,10 +95,8 @@ void php_json_scanner_init(php_json_scanner *s, const char *str, size_t str_len, s->cursor = (php_json_ctype *) str; s->limit = (php_json_ctype *) str + str_len; s->options = options; - PHP_JSON_TOKEN_LOCATION(first_column) = 1; - PHP_JSON_TOKEN_LOCATION(first_line) = 1; - PHP_JSON_TOKEN_LOCATION(last_column) = 1; - PHP_JSON_TOKEN_LOCATION(last_line) = 1; + s->line = 1; + s->line_start = (php_json_ctype *) str; PHP_JSON_CONDITION_SET(JS); } @@ -108,8 +105,6 @@ int php_json_scan(php_json_scanner *s) ZVAL_NULL(&s->value); std: - PHP_JSON_TOKEN_LOCATION(first_column) = s->errloc.last_column; - PHP_JSON_TOKEN_LOCATION(first_line) = s->errloc.last_line; s->token = s->cursor; /*!re2c @@ -155,49 +150,27 @@ std: UTF16_3 = UTFPREF ( ( ( HEXC | [efEF] ) HEX ) | ( [dD] HEX7 ) ) HEX{2} ; UTF16_4 = UTFPREF [dD] [89abAB] HEX{2} UTFPREF [dD] [c-fC-F] HEX{2} ; - <JS>"{" { - PHP_JSON_TOKEN_LOCATION(last_column)++; - return '{'; - } - <JS>"}" { - PHP_JSON_TOKEN_LOCATION(last_column)++; - return '}'; - } - <JS>"[" { - PHP_JSON_TOKEN_LOCATION(last_column)++; - return '['; - } - <JS>"]" { - PHP_JSON_TOKEN_LOCATION(last_column)++; - return ']'; - } - <JS>":" { - PHP_JSON_TOKEN_LOCATION(last_column)++; - return ':'; - } - <JS>"," { - PHP_JSON_TOKEN_LOCATION(last_column)++; - return ','; - } + <JS>"{" { return '{'; } + <JS>"}" { return '}'; } + <JS>"[" { return '['; } + <JS>"]" { return ']'; } + <JS>":" { return ':'; } + <JS>"," { return ','; } <JS>"null" { - PHP_JSON_TOKEN_LOCATION(last_column) += 4; ZVAL_NULL(&s->value); return PHP_JSON_T_NUL; } <JS>"true" { - PHP_JSON_TOKEN_LOCATION(last_column) += 4; ZVAL_TRUE(&s->value); return PHP_JSON_T_TRUE; } <JS>"false" { - PHP_JSON_TOKEN_LOCATION(last_column) += 5; ZVAL_FALSE(&s->value); return PHP_JSON_T_FALSE; } <JS>INT { bool bigint = 0, negative = s->token[0] == '-'; size_t digits = PHP_JSON_TOKEN_LENGTH(); - PHP_JSON_TOKEN_LOCATION(last_column) += digits; digits -= negative; if (digits >= PHP_JSON_INT_MAX_LENGTH) { if (digits == PHP_JSON_INT_MAX_LENGTH) { @@ -221,19 +194,15 @@ std: } } <JS>FLOAT|EXP { - PHP_JSON_TOKEN_LOCATION(last_column) += PHP_JSON_TOKEN_LENGTH(); ZVAL_DOUBLE(&s->value, zend_strtod((char *) s->token, NULL)); return PHP_JSON_T_DOUBLE; } <JS>NL { - PHP_JSON_TOKEN_LOCATION(last_line)++; - PHP_JSON_TOKEN_LOCATION(last_column) = 1; - goto std; - } - <JS>WS { - PHP_JSON_TOKEN_LOCATION(last_column) += PHP_JSON_TOKEN_LENGTH(); + s->line++; + s->line_start = s->cursor; goto std; } + <JS>WS { goto std; } <JS>EOI { if (s->limit < s->cursor) { return PHP_JSON_T_EOI; @@ -243,7 +212,6 @@ std: } } <JS>["] { - PHP_JSON_TOKEN_LOCATION(last_column)++; s->str_start = s->cursor; s->str_esc = 0; s->utf8_invalid = 0; @@ -275,22 +243,18 @@ std: return PHP_JSON_T_ERROR; } <STR_P1>UTF16_1 { - PHP_JSON_TOKEN_LOCATION(last_column) += 1; s->str_esc += 5; PHP_JSON_CONDITION_GOTO(STR_P1); } <STR_P1>UTF16_2 { - PHP_JSON_TOKEN_LOCATION(last_column) += 1; s->str_esc += 4; PHP_JSON_CONDITION_GOTO(STR_P1); } <STR_P1>UTF16_3 { - PHP_JSON_TOKEN_LOCATION(last_column) += 1; s->str_esc += 3; PHP_JSON_CONDITION_GOTO(STR_P1); } <STR_P1>UTF16_4 { - PHP_JSON_TOKEN_LOCATION(last_column) += 1; s->str_esc += 8; PHP_JSON_CONDITION_GOTO(STR_P1); } @@ -299,7 +263,6 @@ std: return PHP_JSON_T_ERROR; } <STR_P1>ESC { - PHP_JSON_TOKEN_LOCATION(last_column) += 2; s->str_esc++; PHP_JSON_CONDITION_GOTO(STR_P1); } @@ -308,7 +271,6 @@ std: return PHP_JSON_T_ERROR; } <STR_P1>["] { - PHP_JSON_TOKEN_LOCATION(last_column)++; zend_string *str; size_t len = (size_t)(s->cursor - s->str_start - s->str_esc - 1 + s->utf8_invalid_count); if (len == 0) { @@ -329,22 +291,7 @@ std: return PHP_JSON_T_STRING; } } - <STR_P1>UTF8_1 { - PHP_JSON_TOKEN_LOCATION(last_column)++; - PHP_JSON_CONDITION_GOTO(STR_P1); - } - <STR_P1>UTF8_2 { - PHP_JSON_TOKEN_LOCATION(last_column) += 1; - PHP_JSON_CONDITION_GOTO(STR_P1); - } - <STR_P1>UTF8_3 { - PHP_JSON_TOKEN_LOCATION(last_column) += 1; - PHP_JSON_CONDITION_GOTO(STR_P1); - } - <STR_P1>UTF8_4 { - PHP_JSON_TOKEN_LOCATION(last_column) += 1; - PHP_JSON_CONDITION_GOTO(STR_P1); - } + <STR_P1>UTF8 { PHP_JSON_CONDITION_GOTO(STR_P1); } <STR_P1>ANY { if (s->options & (PHP_JSON_INVALID_UTF8_IGNORE | PHP_JSON_INVALID_UTF8_SUBSTITUTE)) { if (s->options & PHP_JSON_INVALID_UTF8_SUBSTITUTE) { diff --git a/ext/json/php_json.h b/ext/json/php_json.h index f34684e149d8..f20b20964a71 100644 --- a/ext/json/php_json.h +++ b/ext/json/php_json.h @@ -54,8 +54,8 @@ typedef enum { typedef struct php_json_error_details { php_json_error_code code; - size_t line; - size_t column; + uint64_t line; + uint64_t column; } php_json_error_details; static inline void php_json_error_details_clear(php_json_error_details *out) { diff --git a/ext/json/php_json_parser.h b/ext/json/php_json_parser.h index 888a0d317fe0..ae927d880aeb 100644 --- a/ext/json/php_json_parser.h +++ b/ext/json/php_json_parser.h @@ -48,20 +48,12 @@ typedef struct _php_json_parser_methods { php_json_parser_func_object_end_t object_end; } php_json_parser_methods; - typedef struct _php_json_parser_location { - size_t first_line; - size_t first_column; - size_t last_line; - size_t last_column; -} php_json_parser_location; - struct _php_json_parser { php_json_scanner scanner; zval *return_value; int depth; int max_depth; php_json_parser_methods methods; - php_json_parser_location *location; }; PHP_JSON_API void php_json_parser_init_ex( diff --git a/ext/json/php_json_scanner.h b/ext/json/php_json_scanner.h index 90460cf9952a..f432f66b6d78 100644 --- a/ext/json/php_json_scanner.h +++ b/ext/json/php_json_scanner.h @@ -20,17 +20,6 @@ typedef unsigned char php_json_ctype; -typedef struct _php_json_error_location { - /** first column of the error */ - size_t first_column; - /** first line of the error */ - size_t first_line; - /** last column of the error */ - size_t last_column; - /** last line of the error */ - size_t last_line; -} php_json_error_location; - typedef struct _php_json_scanner { php_json_ctype *cursor; /* cursor position */ php_json_ctype *token; /* token position */ @@ -39,18 +28,17 @@ typedef struct _php_json_scanner { php_json_ctype *ctxmarker; /* marker position for context backtracking */ php_json_ctype *str_start; /* start position of the string */ php_json_ctype *pstr; /* string pointer for escapes conversion */ + php_json_ctype *line_start; /* start position of the current line */ + uint64_t line; /* current line number (1-based) */ zval value; /* value */ int str_esc; /* number of extra characters for escaping */ int state; /* condition state */ int options; /* options */ php_json_error_code errcode; /* error type if there is an error */ - php_json_error_location errloc; /* error location */ int utf8_invalid; /* whether utf8 is invalid */ int utf8_invalid_count; /* number of extra character for invalid utf8 */ } php_json_scanner; -#define PHP_JSON_SCANNER_LOCATION(scanner, slocation) (scanner).errloc.slocation - void php_json_scanner_init(php_json_scanner *scanner, const char *str, size_t str_len, int options); int php_json_scan(php_json_scanner *s); diff --git a/ext/json/tests/bug68546.phpt b/ext/json/tests/bug68546.phpt index 1847eabf3a8e..40ea085ba23b 100644 --- a/ext/json/tests/bug68546.phpt +++ b/ext/json/tests/bug68546.phpt @@ -16,5 +16,5 @@ NULL bool(true) NULL bool(true) -string(55) "The decoded property name is invalid near location 1:27" +string(55) "The decoded property name is invalid near location 1:37" Done diff --git a/ext/json/tests/gh22514.phpt b/ext/json/tests/gh22514.phpt new file mode 100644 index 000000000000..9e7d2adfff8a --- /dev/null +++ b/ext/json/tests/gh22514.phpt @@ -0,0 +1,21 @@ +--TEST-- +GH-22514: Incorrect error column in JSON parser after escape sequences +--FILE-- +<?php + +// The error is on the '_' on line 2. The ÿ escape occupies six source +// characters, so the column must count each of them: '_' is at column 10. +var_dump(json_decode("\n{\"\\u00FF\"_")); +var_dump(json_last_error_msg()); + +// A real multibyte UTF-8 character counts as a single column, so the same +// logical value reached via raw UTF-8 yields a smaller column than the escape. +var_dump(json_decode("\n{\"\xC3\xBF\"_")); +var_dump(json_last_error_msg()); + +?> +--EXPECT-- +NULL +string(31) "Syntax error near location 2:10" +NULL +string(30) "Syntax error near location 2:5" diff --git a/ext/json/tests/json_last_error_msg_error_location_005.phpt b/ext/json/tests/json_last_error_msg_error_location_005.phpt index ee1800eb8273..ca798fcb01fe 100644 --- a/ext/json/tests/json_last_error_msg_error_location_005.phpt +++ b/ext/json/tests/json_last_error_msg_error_location_005.phpt @@ -19,8 +19,8 @@ json_validate_trycatchdump('{"test": "\uD83D\uDE00\uD83C\uDF89}'); echo "\nError with mixed UTF-8 and UTF-16:\n"; json_validate_trycatchdump('{"mixed": "Hello \u4E16\u754C world}'); -// UTF-16 in key and value -echo "\nError with UTF-16 in key:\n"; +// Missing closing quote after a UTF-16 key +echo "\nError on unterminated value after UTF-16 key:\n"; json_validate_trycatchdump('{"\u30D7\u30EC\u30B9": "value}'); // Multiple keys with UTF-16 @@ -66,15 +66,15 @@ bool(false) int(4) string(31) "Syntax error near location 1:11" -Error with UTF-16 in key: +Error on unterminated value after UTF-16 key: bool(false) int(4) -string(30) "Syntax error near location 1:9" +string(31) "Syntax error near location 1:24" Error with multiple UTF-16 keys: bool(false) int(4) -string(31) "Syntax error near location 1:22" +string(31) "Syntax error near location 1:42" Error with BMP characters: bool(false) @@ -89,12 +89,12 @@ string(31) "Syntax error near location 1:11" Error in array with UTF-16: bool(false) int(4) -string(31) "Syntax error near location 1:12" +string(31) "Syntax error near location 1:22" Error in nested structure with UTF-16: bool(false) int(4) -string(31) "Syntax error near location 1:18" +string(31) "Syntax error near location 1:28" Error with UTF-16 and control chars: bool(false)