Skip to content

Commit 13169cd

Browse files
henderkesTimWolla
henderkes
and
TimWolla
authored
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 <tim@bastelstu.be>
1 parent cf3c803 commit 13169cd

10 files changed

Lines changed: 73 additions & 110 deletions

‎NEWS‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,10 @@ PHP NEWS
6565

6666
- JSON:
6767
. Report unterminated JSON strings as syntax errors. (timwolla)
68+
. Improve performance error position tracking during JSON decoding.
69+
(henderkes)
70+
. Fixed bug GH-22514 (Incorrect error column in PHP 8.6 JSON parser).
71+
(henderkes, timwolla)
6872

6973
- Opcache:
7074
. Fixed bug GH-21770 (Infinite recursion in property hook getter in opcache

‎ext/json/json.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ static zend_string *php_json_get_error_msg_with_location(const php_json_error_de
184184
const char *base_msg = php_json_get_error_msg(details->code);
185185

186186
if (details->line > 0 && details->column > 0) {
187-
return zend_strpprintf(0, "%s near location %zu:%zu", base_msg, details->line, details->column);
187+
return zend_strpprintf(0, "%s near location %" PRIu64 ":%" PRIu64, base_msg, details->line, details->column);
188188
}
189189

190190
return zend_string_init(base_msg, strlen(base_msg), 0);

‎ext/json/json_parser.y‎

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@ int json_yydebug = 1;
3939

4040
}
4141

42-
%locations
4342
%define api.prefix {php_json_yy}
4443
%define api.pure full
4544
%param { php_json_parser *parser }
@@ -64,8 +63,8 @@ int json_yydebug = 1;
6463
%destructor { zval_ptr_dtor_nogc(&$$); } <value>
6564

6665
%code {
67-
static int php_json_yylex(union YYSTYPE *value, YYLTYPE *location, php_json_parser *parser);
68-
static void php_json_yyerror(YYLTYPE *location, php_json_parser *parser, char const *msg);
66+
static int php_json_yylex(union YYSTYPE *value, php_json_parser *parser);
67+
static void php_json_yyerror(php_json_parser *parser, char const *msg);
6968
static int php_json_parser_array_create(php_json_parser *parser, zval *array);
7069
static int php_json_parser_object_create(php_json_parser *parser, zval *array);
7170

@@ -275,7 +274,7 @@ static int php_json_parser_object_update_validate(php_json_parser *parser, zval
275274
return SUCCESS;
276275
}
277276

278-
static int php_json_yylex(union YYSTYPE *value, YYLTYPE *location, php_json_parser *parser)
277+
static int php_json_yylex(union YYSTYPE *value, php_json_parser *parser)
279278
{
280279
int token = php_json_scan(&parser->scanner);
281280

@@ -291,15 +290,10 @@ static int php_json_yylex(union YYSTYPE *value, YYLTYPE *location, php_json_pars
291290
value->value = parser->scanner.value;
292291
}
293292

294-
location->first_column = PHP_JSON_SCANNER_LOCATION(parser->scanner, first_column);
295-
location->first_line = PHP_JSON_SCANNER_LOCATION(parser->scanner, first_line);
296-
location->last_column = PHP_JSON_SCANNER_LOCATION(parser->scanner, last_column);
297-
location->last_line = PHP_JSON_SCANNER_LOCATION(parser->scanner, last_line);
298-
299293
return token;
300294
}
301295

302-
static void php_json_yyerror(YYLTYPE *location, php_json_parser *parser, char const *msg)
296+
static void php_json_yyerror(php_json_parser *parser, char const *msg)
303297
{
304298
if (!parser->scanner.errcode) {
305299
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
311305
return parser->scanner.errcode;
312306
}
313307

308+
static uint64_t php_json_compute_error_column(const php_json_scanner *s)
309+
{
310+
const php_json_ctype *p = s->line_start;
311+
const php_json_ctype *end = s->token;
312+
/* Count characters from the start of the line to the failing token,
313+
* folding UTF-8 continuation bytes into their leading byte. */
314+
uint64_t column = 1;
315+
316+
while (p < end) {
317+
if ((*p & 0b11000000) != 0b10000000) {
318+
column++;
319+
}
320+
p++;
321+
}
322+
return column;
323+
}
324+
314325
PHP_JSON_API void php_json_parser_error_details(const php_json_parser *parser, php_json_error_details *out)
315326
{
316327
out->code = parser->scanner.errcode;
317-
out->line = parser->scanner.errloc.first_line;
318-
out->column = parser->scanner.errloc.first_column;
328+
out->line = parser->scanner.line;
329+
out->column = php_json_compute_error_column(&parser->scanner);
319330
}
320331

321332
static const php_json_parser_methods default_parser_methods =

‎ext/json/json_scanner.re‎

Lines changed: 12 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,6 @@
5151
#define PHP_JSON_INT_MAX_LENGTH (MAX_LENGTH_OF_LONG - 1)
5252

5353
#define PHP_JSON_TOKEN_LENGTH() ((size_t) (s->cursor - s->token))
54-
#define PHP_JSON_TOKEN_LOCATION(location) (s)->errloc.location
5554

5655
static void php_json_scanner_copy_string(php_json_scanner *s, size_t esc_size)
5756
{
@@ -96,10 +95,8 @@ void php_json_scanner_init(php_json_scanner *s, const char *str, size_t str_len,
9695
s->cursor = (php_json_ctype *) str;
9796
s->limit = (php_json_ctype *) str + str_len;
9897
s->options = options;
99-
PHP_JSON_TOKEN_LOCATION(first_column) = 1;
100-
PHP_JSON_TOKEN_LOCATION(first_line) = 1;
101-
PHP_JSON_TOKEN_LOCATION(last_column) = 1;
102-
PHP_JSON_TOKEN_LOCATION(last_line) = 1;
98+
s->line = 1;
99+
s->line_start = (php_json_ctype *) str;
103100
PHP_JSON_CONDITION_SET(JS);
104101
}
105102

@@ -108,8 +105,6 @@ int php_json_scan(php_json_scanner *s)
108105
ZVAL_NULL(&s->value);
109106

110107
std:
111-
PHP_JSON_TOKEN_LOCATION(first_column) = s->errloc.last_column;
112-
PHP_JSON_TOKEN_LOCATION(first_line) = s->errloc.last_line;
113108
s->token = s->cursor;
114109

115110
/*!re2c
@@ -155,49 +150,27 @@ std:
155150
UTF16_3 = UTFPREF ( ( ( HEXC | [efEF] ) HEX ) | ( [dD] HEX7 ) ) HEX{2} ;
156151
UTF16_4 = UTFPREF [dD] [89abAB] HEX{2} UTFPREF [dD] [c-fC-F] HEX{2} ;
157152
158-
<JS>"{" {
159-
PHP_JSON_TOKEN_LOCATION(last_column)++;
160-
return '{';
161-
}
162-
<JS>"}" {
163-
PHP_JSON_TOKEN_LOCATION(last_column)++;
164-
return '}';
165-
}
166-
<JS>"[" {
167-
PHP_JSON_TOKEN_LOCATION(last_column)++;
168-
return '[';
169-
}
170-
<JS>"]" {
171-
PHP_JSON_TOKEN_LOCATION(last_column)++;
172-
return ']';
173-
}
174-
<JS>":" {
175-
PHP_JSON_TOKEN_LOCATION(last_column)++;
176-
return ':';
177-
}
178-
<JS>"," {
179-
PHP_JSON_TOKEN_LOCATION(last_column)++;
180-
return ',';
181-
}
153+
<JS>"{" { return '{'; }
154+
<JS>"}" { return '}'; }
155+
<JS>"[" { return '['; }
156+
<JS>"]" { return ']'; }
157+
<JS>":" { return ':'; }
158+
<JS>"," { return ','; }
182159
<JS>"null" {
183-
PHP_JSON_TOKEN_LOCATION(last_column) += 4;
184160
ZVAL_NULL(&s->value);
185161
return PHP_JSON_T_NUL;
186162
}
187163
<JS>"true" {
188-
PHP_JSON_TOKEN_LOCATION(last_column) += 4;
189164
ZVAL_TRUE(&s->value);
190165
return PHP_JSON_T_TRUE;
191166
}
192167
<JS>"false" {
193-
PHP_JSON_TOKEN_LOCATION(last_column) += 5;
194168
ZVAL_FALSE(&s->value);
195169
return PHP_JSON_T_FALSE;
196170
}
197171
<JS>INT {
198172
bool bigint = 0, negative = s->token[0] == '-';
199173
size_t digits = PHP_JSON_TOKEN_LENGTH();
200-
PHP_JSON_TOKEN_LOCATION(last_column) += digits;
201174
digits -= negative;
202175
if (digits >= PHP_JSON_INT_MAX_LENGTH) {
203176
if (digits == PHP_JSON_INT_MAX_LENGTH) {
@@ -221,19 +194,15 @@ std:
221194
}
222195
}
223196
<JS>FLOAT|EXP {
224-
PHP_JSON_TOKEN_LOCATION(last_column) += PHP_JSON_TOKEN_LENGTH();
225197
ZVAL_DOUBLE(&s->value, zend_strtod((char *) s->token, NULL));
226198
return PHP_JSON_T_DOUBLE;
227199
}
228200
<JS>NL {
229-
PHP_JSON_TOKEN_LOCATION(last_line)++;
230-
PHP_JSON_TOKEN_LOCATION(last_column) = 1;
231-
goto std;
232-
}
233-
<JS>WS {
234-
PHP_JSON_TOKEN_LOCATION(last_column) += PHP_JSON_TOKEN_LENGTH();
201+
s->line++;
202+
s->line_start = s->cursor;
235203
goto std;
236204
}
205+
<JS>WS { goto std; }
237206
<JS>EOI {
238207
if (s->limit < s->cursor) {
239208
return PHP_JSON_T_EOI;
@@ -243,7 +212,6 @@ std:
243212
}
244213
}
245214
<JS>["] {
246-
PHP_JSON_TOKEN_LOCATION(last_column)++;
247215
s->str_start = s->cursor;
248216
s->str_esc = 0;
249217
s->utf8_invalid = 0;
@@ -275,22 +243,18 @@ std:
275243
return PHP_JSON_T_ERROR;
276244
}
277245
<STR_P1>UTF16_1 {
278-
PHP_JSON_TOKEN_LOCATION(last_column) += 1;
279246
s->str_esc += 5;
280247
PHP_JSON_CONDITION_GOTO(STR_P1);
281248
}
282249
<STR_P1>UTF16_2 {
283-
PHP_JSON_TOKEN_LOCATION(last_column) += 1;
284250
s->str_esc += 4;
285251
PHP_JSON_CONDITION_GOTO(STR_P1);
286252
}
287253
<STR_P1>UTF16_3 {
288-
PHP_JSON_TOKEN_LOCATION(last_column) += 1;
289254
s->str_esc += 3;
290255
PHP_JSON_CONDITION_GOTO(STR_P1);
291256
}
292257
<STR_P1>UTF16_4 {
293-
PHP_JSON_TOKEN_LOCATION(last_column) += 1;
294258
s->str_esc += 8;
295259
PHP_JSON_CONDITION_GOTO(STR_P1);
296260
}
@@ -299,7 +263,6 @@ std:
299263
return PHP_JSON_T_ERROR;
300264
}
301265
<STR_P1>ESC {
302-
PHP_JSON_TOKEN_LOCATION(last_column) += 2;
303266
s->str_esc++;
304267
PHP_JSON_CONDITION_GOTO(STR_P1);
305268
}
@@ -308,7 +271,6 @@ std:
308271
return PHP_JSON_T_ERROR;
309272
}
310273
<STR_P1>["] {
311-
PHP_JSON_TOKEN_LOCATION(last_column)++;
312274
zend_string *str;
313275
size_t len = (size_t)(s->cursor - s->str_start - s->str_esc - 1 + s->utf8_invalid_count);
314276
if (len == 0) {
@@ -329,22 +291,7 @@ std:
329291
return PHP_JSON_T_STRING;
330292
}
331293
}
332-
<STR_P1>UTF8_1 {
333-
PHP_JSON_TOKEN_LOCATION(last_column)++;
334-
PHP_JSON_CONDITION_GOTO(STR_P1);
335-
}
336-
<STR_P1>UTF8_2 {
337-
PHP_JSON_TOKEN_LOCATION(last_column) += 1;
338-
PHP_JSON_CONDITION_GOTO(STR_P1);
339-
}
340-
<STR_P1>UTF8_3 {
341-
PHP_JSON_TOKEN_LOCATION(last_column) += 1;
342-
PHP_JSON_CONDITION_GOTO(STR_P1);
343-
}
344-
<STR_P1>UTF8_4 {
345-
PHP_JSON_TOKEN_LOCATION(last_column) += 1;
346-
PHP_JSON_CONDITION_GOTO(STR_P1);
347-
}
294+
<STR_P1>UTF8 { PHP_JSON_CONDITION_GOTO(STR_P1); }
348295
<STR_P1>ANY {
349296
if (s->options & (PHP_JSON_INVALID_UTF8_IGNORE | PHP_JSON_INVALID_UTF8_SUBSTITUTE)) {
350297
if (s->options & PHP_JSON_INVALID_UTF8_SUBSTITUTE) {

‎ext/json/php_json.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,8 @@ typedef enum {
5454

5555
typedef struct php_json_error_details {
5656
php_json_error_code code;
57-
size_t line;
58-
size_t column;
57+
uint64_t line;
58+
uint64_t column;
5959
} php_json_error_details;
6060

6161
static inline void php_json_error_details_clear(php_json_error_details *out) {

‎ext/json/php_json_parser.h‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -48,20 +48,12 @@ typedef struct _php_json_parser_methods {
4848
php_json_parser_func_object_end_t object_end;
4949
} php_json_parser_methods;
5050

51-
typedef struct _php_json_parser_location {
52-
size_t first_line;
53-
size_t first_column;
54-
size_t last_line;
55-
size_t last_column;
56-
} php_json_parser_location;
57-
5851
struct _php_json_parser {
5952
php_json_scanner scanner;
6053
zval *return_value;
6154
int depth;
6255
int max_depth;
6356
php_json_parser_methods methods;
64-
php_json_parser_location *location;
6557
};
6658

6759
PHP_JSON_API void php_json_parser_init_ex(

‎ext/json/php_json_scanner.h‎

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -20,17 +20,6 @@
2020

2121
typedef unsigned char php_json_ctype;
2222

23-
typedef struct _php_json_error_location {
24-
/** first column of the error */
25-
size_t first_column;
26-
/** first line of the error */
27-
size_t first_line;
28-
/** last column of the error */
29-
size_t last_column;
30-
/** last line of the error */
31-
size_t last_line;
32-
} php_json_error_location;
33-
3423
typedef struct _php_json_scanner {
3524
php_json_ctype *cursor; /* cursor position */
3625
php_json_ctype *token; /* token position */
@@ -39,18 +28,17 @@ typedef struct _php_json_scanner {
3928
php_json_ctype *ctxmarker; /* marker position for context backtracking */
4029
php_json_ctype *str_start; /* start position of the string */
4130
php_json_ctype *pstr; /* string pointer for escapes conversion */
31+
php_json_ctype *line_start; /* start position of the current line */
32+
uint64_t line; /* current line number (1-based) */
4233
zval value; /* value */
4334
int str_esc; /* number of extra characters for escaping */
4435
int state; /* condition state */
4536
int options; /* options */
4637
php_json_error_code errcode; /* error type if there is an error */
47-
php_json_error_location errloc; /* error location */
4838
int utf8_invalid; /* whether utf8 is invalid */
4939
int utf8_invalid_count; /* number of extra character for invalid utf8 */
5040
} php_json_scanner;
5141

52-
#define PHP_JSON_SCANNER_LOCATION(scanner, slocation) (scanner).errloc.slocation
53-
5442
void php_json_scanner_init(php_json_scanner *scanner, const char *str, size_t str_len, int options);
5543
int php_json_scan(php_json_scanner *s);
5644

‎ext/json/tests/bug68546.phpt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,5 +16,5 @@ NULL
1616
bool(true)
1717
NULL
1818
bool(true)
19-
string(55) "The decoded property name is invalid near location 1:27"
19+
string(55) "The decoded property name is invalid near location 1:37"
2020
Done

‎ext/json/tests/gh22514.phpt‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
--TEST--
2+
GH-22514: Incorrect error column in JSON parser after escape sequences
3+
--FILE--
4+
<?php
5+
6+
// The error is on the '_' on line 2. The ÿ escape occupies six source
7+
// characters, so the column must count each of them: '_' is at column 10.
8+
var_dump(json_decode("\n{\"\\u00FF\"_"));
9+
var_dump(json_last_error_msg());
10+
11+
// A real multibyte UTF-8 character counts as a single column, so the same
12+
// logical value reached via raw UTF-8 yields a smaller column than the escape.
13+
var_dump(json_decode("\n{\"\xC3\xBF\"_"));
14+
var_dump(json_last_error_msg());
15+
16+
?>
17+
--EXPECT--
18+
NULL
19+
string(31) "Syntax error near location 2:10"
20+
NULL
21+
string(30) "Syntax error near location 2:5"

0 commit comments

Comments
 (0)