Skip to content

RFC - json_validate() #9399

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

Merged
merged 22 commits into from
Oct 8, 2022
Merged

RFC - json_validate() #9399

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
0405a86
json_validate()
Aug 22, 2022
8f86a67
retrigger pipelines
Aug 22, 2022
344687f
005 performance test
Aug 22, 2022
cfbee85
Add new line test 001
juan-morales Aug 22, 2022
85dcc6a
Add new line at the end of tests
Aug 26, 2022
6688e20
Add PHP_JSON_VALIDATE_ONLY constant instead of validate_only
Aug 26, 2022
47756e9
Add test comparing results with json_decode() on different types of i…
Aug 28, 2022
ae041be
group together function hooks for the json parser used by validation …
Aug 28, 2022
8e97349
Remove usage of flag PHP_JSON_THROW_ON_ERROR
Aug 28, 2022
52ab14e
Add test cases for invalid types on test 006
Aug 28, 2022
1b1b4f3
Stop propagation of parser option PHP_JSON_VALIDATE_ONLY to the scann…
Aug 28, 2022
baf8ec6
get_validate_methods() usage instead of options constant PHP_JSON_VAL…
Aug 28, 2022
d038d1f
fix qualifier error
Aug 28, 2022
722303b
remove get_validate_methods() from PHP API
Aug 28, 2022
f02c89c
remove test 005 and rename 006
Aug 28, 2022
70467bc
fix typeO in comment
Aug 28, 2022
8b528bd
use RETURN_BOOL macro
juan-morales Sep 25, 2022
7331b47
Fix return type to bool
juan-morales Sep 26, 2022
2e92c03
Optimizations usage of ZVAL_NULL instead of empty array. Return valid…
juan-morales Sep 26, 2022
d71b16f
dependencies
juan-morales Sep 26, 2022
9223115
adjust indentation in test from 2 to 4 spaces
juan-morales Sep 26, 2022
a08d066
Remove test of passing NULL as parameter
juan-morales Sep 28, 2022
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
84 changes: 83 additions & 1 deletion ext/json/json.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ PHP_JSON_API zend_result php_json_decode_ex(zval *return_value, const char *str,
{
php_json_parser parser;

php_json_parser_init(&parser, return_value, str, str_len, (int)options, (int)depth);
php_json_parser_init(&parser, return_value, str, str_len, (int)options, (int)depth, false);

if (php_json_yyparse(&parser)) {
php_json_error_code error_code = php_json_parser_error_code(&parser);
Expand All @@ -178,6 +178,28 @@ PHP_JSON_API zend_result php_json_decode_ex(zval *return_value, const char *str,
}
/* }}} */

/* {{{ */
PHP_JSON_API zend_result php_json_validate_ex(zval *return_value, const char *str, size_t str_len, zend_long options, zend_long depth)
{
php_json_parser parser;

php_json_parser_init(&parser, return_value, str, str_len, (int)options, (int)depth, true);

if (php_json_yyparse(&parser)) {
php_json_error_code error_code = php_json_parser_error_code(&parser);
if (!(options & PHP_JSON_THROW_ON_ERROR)) {
JSON_G(error_code) = error_code;
Copy link
Member

Choose a reason for hiding this comment

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

A validation routine should not have side effects, such as setting a global error_code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will check this and will be back to you

Copy link
Contributor Author

@juan-morales juan-morales Aug 26, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

@derickr
The reason I kept that code, was because of the usage of json_last_error() function.

I , and other developers in the mailing list find it useful to have such information, to know why a validation failed so we can have messages like:

  • Malformed UTF-8 characters, possibly incorrectly encoded
  • Syntax error
  • Maximum stack depth exceeded

etc.

I think is good thing to have, also some other devs from mailing list.

What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

I think this is a legit solution; an alternative could be to return the error code as int (instead of a bool).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wrote this down in the RFC as an open question

} else {
zend_throw_exception(php_json_exception_ce, php_json_get_error_msg(error_code), error_code);
}
RETVAL_FALSE;
return FAILURE;
}

return SUCCESS;
}
/* }}} */

/* {{{ Returns the JSON representation of a value */
PHP_FUNCTION(json_encode)
{
Expand Down Expand Up @@ -270,6 +292,66 @@ PHP_FUNCTION(json_decode)
}
/* }}} */

/* {{{ Validates if an string contains a valid json */
PHP_FUNCTION(json_validate)
{
char *str;
size_t str_len;
zend_long depth = PHP_JSON_PARSER_DEFAULT_DEPTH;
zend_long options = 0;

ZEND_PARSE_PARAMETERS_START(1, 3)
Z_PARAM_STRING(str, str_len)
Z_PARAM_OPTIONAL
Z_PARAM_LONG(depth)
Z_PARAM_LONG(options)
ZEND_PARSE_PARAMETERS_END();


if (options != 0) {
zend_long tmp_options = options;
tmp_options ^= (PHP_JSON_INVALID_UTF8_IGNORE | PHP_JSON_THROW_ON_ERROR);

if (tmp_options != 0) {
if (!((tmp_options == PHP_JSON_INVALID_UTF8_IGNORE) || (tmp_options == PHP_JSON_THROW_ON_ERROR))) {
zend_argument_value_error(3, "must be a valid flag (JSON_THROW_ON_ERROR, JSON_INVALID_UTF8_IGNORE)");
RETURN_THROWS();
}
}
}

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

RETURN_FALSE;
}

if (!(options & PHP_JSON_THROW_ON_ERROR)) {
JSON_G(error_code) = PHP_JSON_ERROR_NONE;
Copy link
Member

Choose a reason for hiding this comment

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

You're setting the error_code here, but wouldn't it make more sense to return this value from the function? That would mean that instead of returning a boolean, it would have to be an integer, with 0 likely having to mean no error, but that is not something that is unheard of.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have to say that returning a boolean was my personal preferred approach. If you think we should do it with the int way, let me know.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have being thinking, and I really prefer returning bool, and being able to call json_last_error() to find out what happened

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wrote this down in the RFC as an open question

}

if (depth <= 0) {
zend_argument_value_error(2, "must be greater than 0");
RETURN_THROWS();
}

if (depth > INT_MAX) {
zend_argument_value_error(2, "must be less than %d", INT_MAX);
RETURN_THROWS();
}

if (php_json_validate_ex(return_value, str, str_len, options, depth) == SUCCESS) {
RETURN_TRUE;
}

RETURN_FALSE;
}
/* }}} */

/* {{{ Returns the error code of the last json_encode() or json_decode() call. */
PHP_FUNCTION(json_last_error)
{
Expand Down
2 changes: 2 additions & 0 deletions ext/json/json.stub.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,8 @@ function json_encode(mixed $value, int $flags = 0, int $depth = 512): string|fal

function json_decode(string $json, ?bool $associative = null, int $depth = 512, int $flags = 0): mixed {}

function json_validate(string $json, int $depth = 512, int $flags = 0): bool {}

function json_last_error(): int {}

/** @refcount 1 */
Expand Down
10 changes: 9 additions & 1 deletion ext/json/json_arginfo.h
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

47 changes: 45 additions & 2 deletions ext/json/json_parser.y
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,18 +212,35 @@ value:

%% /* Functions */

static int php_json_parser_array_create_validate(php_json_parser *parser, zval *array)
{
ZVAL_EMPTY_ARRAY(array);
return SUCCESS;
}

static int php_json_parser_array_create(php_json_parser *parser, zval *array)
{
array_init(array);
return SUCCESS;
}

static int php_json_parser_array_append_validate(php_json_parser *parser, zval *array, zval *zvalue)
{
return SUCCESS;
}

static int php_json_parser_array_append(php_json_parser *parser, zval *array, zval *zvalue)
{
zend_hash_next_index_insert(Z_ARRVAL_P(array), zvalue);
return SUCCESS;
}

static int php_json_parser_object_create_validate(php_json_parser *parser, zval *object)
{
ZVAL_EMPTY_ARRAY(object);
return SUCCESS;
}

static int php_json_parser_object_create(php_json_parser *parser, zval *object)
{
if (parser->scanner.options & PHP_JSON_OBJECT_AS_ARRAY) {
Expand All @@ -234,6 +251,11 @@ static int php_json_parser_object_create(php_json_parser *parser, zval *object)
return SUCCESS;
}

static int php_json_parser_object_update_validate(php_json_parser *parser, zval *object, zend_string *key, zval *zvalue)
{
return SUCCESS;
}

static int php_json_parser_object_update(php_json_parser *parser, zval *object, zend_string *key, zval *zvalue)
{
/* if JSON_OBJECT_AS_ARRAY is set */
Expand All @@ -259,6 +281,14 @@ static int php_json_yylex(union YYSTYPE *value, php_json_parser *parser)
{
int token = php_json_scan(&parser->scanner);
value->value = parser->scanner.value;

if (parser->methods.array_create == php_json_parser_array_create_validate
&& parser->methods.array_append == php_json_parser_array_append_validate
&& parser->methods.object_create == php_json_parser_object_create_validate
&& parser->methods.object_update == php_json_parser_object_update_validate) {
zval_ptr_dtor_str(&(parser->scanner.value));
}

return token;
}

Expand Down Expand Up @@ -286,6 +316,18 @@ static const php_json_parser_methods default_parser_methods =
NULL,
};

static const php_json_parser_methods validate_parser_methods =
{
php_json_parser_array_create_validate,
php_json_parser_array_append_validate,
NULL,
NULL,
php_json_parser_object_create_validate,
php_json_parser_object_update_validate,
NULL,
NULL,
};

PHP_JSON_API void php_json_parser_init_ex(php_json_parser *parser,
zval *return_value,
const char *str,
Expand All @@ -307,7 +349,8 @@ PHP_JSON_API void php_json_parser_init(php_json_parser *parser,
const char *str,
size_t str_len,
int options,
int max_depth)
int max_depth,
bool validate_only)
{
php_json_parser_init_ex(
parser,
Expand All @@ -316,7 +359,7 @@ PHP_JSON_API void php_json_parser_init(php_json_parser *parser,
str_len,
options,
max_depth,
&default_parser_methods);
(validate_only ? &validate_parser_methods : &default_parser_methods));
}

PHP_JSON_API int php_json_parse(php_json_parser *parser)
Expand Down
6 changes: 4 additions & 2 deletions ext/json/php_json.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,11 +72,13 @@ typedef enum {
#define PHP_JSON_PRESERVE_ZERO_FRACTION (1<<10)
#define PHP_JSON_UNESCAPED_LINE_TERMINATORS (1<<11)

/* json_decode() and json_encode() common options */
/* json_validate(), json_decode() and json_encode() common options */
#define PHP_JSON_INVALID_UTF8_IGNORE (1<<20)
#define PHP_JSON_INVALID_UTF8_SUBSTITUTE (1<<21)
#define PHP_JSON_THROW_ON_ERROR (1<<22)

/* json_decode() and json_encode() common options */
#define PHP_JSON_INVALID_UTF8_SUBSTITUTE (1<<21)

/* Internal flags */
#define PHP_JSON_OUTPUT_ARRAY 0
#define PHP_JSON_OUTPUT_OBJECT 1
Expand Down
3 changes: 2 additions & 1 deletion ext/json/php_json_parser.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,8 @@ PHP_JSON_API void php_json_parser_init(
const char *str,
size_t str_len,
int options,
int max_depth);
int max_depth,
bool validate_only);

PHP_JSON_API php_json_error_code php_json_parser_error_code(const php_json_parser *parser);

Expand Down
41 changes: 41 additions & 0 deletions ext/json/tests/json_validate_001.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
--TEST--
json_validate() - General usage
--FILE--
<?php

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

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

?>
--EXPECTF--
bool(false)
bool(false)
bool(false)
bool(false)
bool(false)
bool(false)
bool(false)
bool(false)
bool(false)
bool(true)
bool(true)
bool(true)
bool(true)
bool(true)
bool(true)
Loading