Skip to content

better json error message #14672

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open

better json error message #14672

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
be91021
WIP - tests are working with GCC warnings
juan-morales Jun 26, 2024
c8590bf
no compiler warnings or memory leaks
juan-morales Jun 27, 2024
60aa905
switch to char[array] instead of char* for the message
juan-morales Jun 27, 2024
1967215
prevent bufferoverflow
juan-morales Jun 27, 2024
376c654
adjust json_last_error_msg php function to the new message type char[…
juan-morales Jun 27, 2024
9125d92
-
juan-morales Jun 27, 2024
5e7de7b
wip
juan-morales Jun 28, 2024
79aa999
-
juan-morales Jun 28, 2024
e832ae1
switch to a new function json_last_error_info() - first try
juan-morales Jun 28, 2024
14b18f6
-
juan-morales Jul 7, 2024
929baa3
-
juan-morales Jul 7, 2024
a79a7f3
Remove new function json_last_error_info(), comments on new data type…
juan-morales Jul 13, 2024
66423ec
json_arginfo.h out
juan-morales Jul 13, 2024
02378dc
json_arginfo.h out
juan-morales Jul 13, 2024
5848b08
-
juan-morales Jul 13, 2024
e198cea
fix comment typeO
juan-morales Jul 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 80 additions & 5 deletions ext/json/json.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include "php_json.h"
#include "php_json_encoder.h"
#include "php_json_parser.h"
#include "php_json_scanner.h"
#include "json_arginfo.h"
#include <zend_exceptions.h>

Expand All @@ -35,6 +36,46 @@ PHP_JSON_API zend_class_entry *php_json_exception_ce;

PHP_JSON_API ZEND_DECLARE_MODULE_GLOBALS(json)

json_error_info_type json_error_info_data = {
NULL, // Parse token
0, // Error ode
0, // Character count
0 // Total number of characters to parse
};

void update_json_error_info_data(php_json_parser *parser) {
json_error_info_data.errcode = parser->scanner.errcode;
json_error_info_data.character_count = parser->scanner.character_count;
json_error_info_data.character_max_count = parser->scanner.character_max_count;

if (json_error_info_data.token) {
efree(json_error_info_data.token);
}

char token[32];
snprintf(
token,
sizeof(token),
"%s",
parser->scanner.token
);
Comment on lines +56 to +61
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to print something in the function called to update data? This might very well be justified, I just don't understand it for now, and so I wanted to ask.


json_error_info_data.token = (php_json_ctype*) emalloc(strlen(token) + 1);
memcpy(json_error_info_data.token, token, strlen(token) + 1);
}

void reset_json_error_info(json_error_info_type* data)
{
if (data->token) {
efree(data->token);
}

data->token = NULL;
data->errcode = 0;
data->character_count = 0;
data->character_max_count = 0;
}

static int php_json_implement_json_serializable(zend_class_entry *interface, zend_class_entry *class_type)
{
class_type->ce_flags |= ZEND_ACC_USE_GUARDS;
Expand Down Expand Up @@ -73,6 +114,14 @@ static PHP_RINIT_FUNCTION(json)
return SUCCESS;
}

static PHP_RSHUTDOWN_FUNCTION(json)
{
if (json_error_info_data.token) {
efree(json_error_info_data.token);
}
return SUCCESS;
}

/* {{{ json_module_entry */
zend_module_entry json_module_entry = {
STANDARD_MODULE_HEADER,
Expand All @@ -81,7 +130,7 @@ zend_module_entry json_module_entry = {
PHP_MINIT(json),
NULL,
PHP_RINIT(json),
NULL,
PHP_RSHUTDOWN(json),
PHP_MINFO(json),
PHP_JSON_VERSION,
PHP_MODULE_GLOBALS(json),
Expand Down Expand Up @@ -185,12 +234,16 @@ PHP_JSON_API zend_result php_json_decode_ex(zval *return_value, const char *str,

if (php_json_yyparse(&parser)) {
php_json_error_code error_code = php_json_parser_error_code(&parser);
update_json_error_info_data(&parser);

if (!(options & PHP_JSON_THROW_ON_ERROR)) {
JSON_G(error_code) = error_code;
} else {
zend_throw_exception(php_json_exception_ce, php_json_get_error_msg(error_code), error_code);
}

RETVAL_NULL();

return FAILURE;
}

Expand All @@ -209,6 +262,7 @@ PHP_JSON_API bool php_json_validate_ex(const char *str, size_t str_len, zend_lon
if (php_json_yyparse(&parser)) {
php_json_error_code error_code = php_json_parser_error_code(&parser);
JSON_G(error_code) = error_code;
update_json_error_info_data(&parser);
return false;
}

Expand All @@ -232,6 +286,8 @@ PHP_FUNCTION(json_encode)
Z_PARAM_LONG(depth)
ZEND_PARSE_PARAMETERS_END();

reset_json_error_info(&json_error_info_data);

php_json_encode_init(&encoder);
encoder.max_depth = (int)depth;
php_json_encode_zval(&buf, parameter, (int)options, &encoder);
Expand Down Expand Up @@ -272,6 +328,8 @@ PHP_FUNCTION(json_decode)
Z_PARAM_LONG(options)
ZEND_PARSE_PARAMETERS_END();

reset_json_error_info(&json_error_info_data);

if (!(options & PHP_JSON_THROW_ON_ERROR)) {
JSON_G(error_code) = PHP_JSON_ERROR_NONE;
}
Expand Down Expand Up @@ -323,12 +381,13 @@ PHP_FUNCTION(json_validate)
Z_PARAM_LONG(options)
ZEND_PARSE_PARAMETERS_END();


if ((options != 0) && (options != PHP_JSON_INVALID_UTF8_IGNORE)) {
zend_argument_value_error(3, "must be a valid flag (allowed flags: JSON_INVALID_UTF8_IGNORE)");
RETURN_THROWS();
}

reset_json_error_info(&json_error_info_data);

if (!str_len) {
JSON_G(error_code) = PHP_JSON_ERROR_SYNTAX;
RETURN_FALSE;
Expand All @@ -350,7 +409,7 @@ PHP_FUNCTION(json_validate)
}
/* }}} */

/* {{{ Returns the error code of the last json_encode() or json_decode() call. */
/* {{{ Returns the error code of the last json_validate(), json_encode() or json_decode() call. */
PHP_FUNCTION(json_last_error)
{
ZEND_PARSE_PARAMETERS_NONE();
Expand All @@ -359,11 +418,27 @@ PHP_FUNCTION(json_last_error)
}
/* }}} */

/* {{{ Returns the error string of the last json_encode() or json_decode() call. */
/* {{{ Returns the error string of the last json_validate(), json_encode() or json_decode() call. */
PHP_FUNCTION(json_last_error_msg)
{
ZEND_PARSE_PARAMETERS_NONE();

RETURN_STRING(php_json_get_error_msg(JSON_G(error_code)));
const char* msg = php_json_get_error_msg(JSON_G(error_code));

if (JSON_G(error_code) == PHP_JSON_ERROR_NONE || !json_error_info_data.token || strlen((const char*)json_error_info_data.token) == 0) {
RETURN_STRING(msg);
}

char msg2[128];
snprintf(
msg2,
sizeof(msg2),
"%s, at character %zu near content: %s",
msg,
json_error_info_data.character_count,
json_error_info_data.token
);

RETURN_STRING(msg2);
}
/* }}} */
1 change: 1 addition & 0 deletions ext/json/json_parser.y
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "php.h"
#include "php_json.h"
#include "php_json_parser.h"
#include "php_json_scanner.h"

#define YYDEBUG 0

Expand Down
8 changes: 8 additions & 0 deletions ext/json/json_scanner.re
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,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;
s->character_count = 0;
s->character_max_count = 0;
PHP_JSON_CONDITION_SET(JS);
}

Expand All @@ -106,6 +108,12 @@ int php_json_scan(php_json_scanner *s)
std:
s->token = s->cursor;

if (s->character_max_count == 0) {
s->character_max_count = strlen((const char*) s->token);
} else {
s->character_count = s->character_max_count + (-1 * strlen((const char*) s->token));
}

/*!re2c
re2c:indent:top = 1;
re2c:yyfill:enable = 0;
Expand Down
8 changes: 8 additions & 0 deletions ext/json/php_json_parser.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "php_json_scanner.h"

typedef struct _php_json_parser php_json_parser;
typedef struct _json_error_info_type json_error_info_type;

typedef int (*php_json_parser_func_array_create_t)(
php_json_parser *parser, zval *array);
Expand Down Expand Up @@ -58,6 +59,13 @@ struct _php_json_parser {
php_json_parser_methods methods;
};

struct _json_error_info_type {
php_json_ctype* token; // Parse token
php_json_error_code errcode; // Error ode
size_t character_count; // Character count
size_t character_max_count; // Total number of characters to parse
};

PHP_JSON_API void php_json_parser_init_ex(
php_json_parser *parser,
zval *return_value,
Expand Down
2 changes: 2 additions & 0 deletions ext/json/php_json_scanner.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ typedef struct _php_json_scanner {
php_json_error_code errcode; /* error type if there is an error */
int utf8_invalid; /* whether utf8 is invalid */
int utf8_invalid_count; /* number of extra character for invalid utf8 */
size_t character_count; /* counts the number of characters scanned (not tokens) */
size_t character_max_count;
} php_json_scanner;


Expand Down
6 changes: 3 additions & 3 deletions ext/json/tests/007.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,13 @@ int(0)
string(8) "No error"
NULL
int(1)
string(28) "Maximum stack depth exceeded"
string(63) "Maximum stack depth exceeded, at character 1 near content: [1]]"
NULL
int(2)
string(42) "State mismatch (invalid or malformed JSON)"
string(74) "State mismatch (invalid or malformed JSON), at character 2 near content: }"
NULL
int(3)
string(53) "Control character error, possibly incorrectly encoded"
string(85) "Control character error, possibly incorrectly encoded, at character 1 near content: ""
NULL
int(4)
string(12) "Syntax error"
Expand Down
2 changes: 1 addition & 1 deletion ext/json/tests/bug62010.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,4 @@ var_dump(json_last_error_msg());
--EXPECT--
NULL
bool(true)
string(50) "Single unpaired UTF-16 surrogate in unicode escape"
string(89) "Single unpaired UTF-16 surrogate in unicode escape, at character 0 near content: "\ud834""
2 changes: 1 addition & 1 deletion ext/json/tests/bug68546.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,5 @@ NULL
bool(true)
NULL
bool(true)
string(36) "The decoded property name is invalid"
string(71) "The decoded property name is invalid, at character 23 near content: 1}]"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is exactly what I was talking about when I mentioned that the might not be exactly right. This is not where ther error is.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are likely many more examples where this would fail.

Done
35 changes: 34 additions & 1 deletion ext/json/tests/json_validate_001.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,38 +4,71 @@ json_validate() - General usage
<?php

var_dump(
json_validate('{"key":"'.str_repeat('x', 1000).'"foobar}'),
json_last_error_msg(),
json_validate(""),
json_last_error_msg(),
json_validate("."),
json_last_error_msg(),
json_validate("<?>"),
json_last_error_msg(),
json_validate(";"),
json_last_error_msg(),
json_validate("руссиш"),
json_last_error_msg(),
json_validate("blah"),
json_last_error_msg(),
json_validate('{ "": "": "" } }'),
json_last_error_msg(),
json_validate('{ "": { "": "" }'),
json_last_error_msg(),
json_validate('{ "test": {} "foo": "bar" }, "test2": {"foo" : "bar" }, "test2": {"foo" : "bar" } }'),

json_last_error_msg(),
json_validate('{ "test": { "foo": "bar" } }'),
json_last_error_msg(),
json_validate('{ "test": { "foo": "" } }'),
json_last_error_msg(),
json_validate('{ "": { "foo": "" } }'),
json_last_error_msg(),
json_validate('{ "": { "": "" } }'),
json_last_error_msg(),
json_validate('{ "test": {"foo": "bar"}, "test2": {"foo" : "bar" }, "test2": {"foo" : "bar" } }'),
json_last_error_msg(),
json_validate('{ "test": {"foo": "bar"}, "test2": {"foo" : "bar" }, "test3": {"foo" : "bar" } }'),
json_last_error_msg(),
);

?>
--EXPECT--
bool(false)
string(53) "Syntax error, at character 1009 near content: foobar}"
bool(false)
string(12) "Syntax error"
bool(false)
string(44) "Syntax error, at character 0 near content: ."
bool(false)
string(46) "Syntax error, at character 0 near content: <?>"
bool(false)
string(44) "Syntax error, at character 0 near content: ;"
bool(false)
string(55) "Syntax error, at character 0 near content: руссиш"
bool(false)
string(47) "Syntax error, at character 0 near content: blah"
bool(false)
string(51) "Syntax error, at character 8 near content: : "" } }"
bool(false)
string(12) "Syntax error"
bool(false)
string(75) "Syntax error, at character 13 near content: "foo": "bar" }, "test2": {"foo""
bool(true)
string(8) "No error"
bool(true)
string(8) "No error"
bool(true)
string(8) "No error"
bool(true)
string(8) "No error"
bool(true)
string(8) "No error"
bool(true)
string(8) "No error"
6 changes: 3 additions & 3 deletions ext/json/tests/json_validate_002.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,13 @@ int(4)
string(12) "Syntax error"
bool(false)
int(4)
string(12) "Syntax error"
string(44) "Syntax error, at character 0 near content: -"
bool(false)
int(4)
string(12) "Syntax error"
bool(false)
int(1)
string(28) "Maximum stack depth exceeded"
string(90) "Maximum stack depth exceeded, at character 0 near content: {"key1":"value1", "key2":"value"
bool(true)
int(0)
string(8) "No error"
Expand All @@ -44,7 +44,7 @@ int(0)
string(8) "No error"
bool(false)
int(4)
string(12) "Syntax error"
string(44) "Syntax error, at character 0 near content: -"
bool(true)
int(0)
string(8) "No error"
10 changes: 5 additions & 5 deletions ext/json/tests/json_validate_004.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,20 +19,20 @@ json_validate_trycatchdump("\"\x61\xf0\x80\x80\x41\"", 512, JSON_INVALID_UTF8_IG
json_validate_trycatchdump("[\"\xc1\xc1\",\"a\"]", 512, JSON_INVALID_UTF8_IGNORE);

?>
--EXPECT--
--EXPECTF--
Testing Invalid UTF-8
bool(false)
int(5)
string(56) "Malformed UTF-8 characters, possibly incorrectly encoded"
string(92) "Malformed UTF-8 characters, possibly incorrectly encoded, at character 0 near content: %s"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should check for the exact error message (i.e. the %s should not be there).

bool(false)
int(5)
string(56) "Malformed UTF-8 characters, possibly incorrectly encoded"
string(93) "Malformed UTF-8 characters, possibly incorrectly encoded, at character 0 near content: %s"
bool(false)
int(5)
string(56) "Malformed UTF-8 characters, possibly incorrectly encoded"
string(94) "Malformed UTF-8 characters, possibly incorrectly encoded, at character 0 near content: %s"
bool(false)
int(5)
string(56) "Malformed UTF-8 characters, possibly incorrectly encoded"
string(96) "Malformed UTF-8 characters, possibly incorrectly encoded, at character 1 near content: %s"
bool(true)
int(0)
string(8) "No error"
Expand Down
2 changes: 1 addition & 1 deletion ext/json/tests/json_validate_005.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
--TEST--
json_validate() - compare against json_decode() for different types of inputs
json_validate() - compare against json_decode() for different types of inputs
--FILE--
<?php

Expand Down
Loading