Skip to content

Commit c34c575

Browse files
TysonAndreTysonAndre
committed
Change handling of depth errors, key_exists exceptions
And throw exceptions in simdjson_key_count instead of returning null
1 parent cf4b6be commit c34c575

5 files changed

Lines changed: 73 additions & 40 deletions

File tree

‎php_simdjson.cpp‎

Lines changed: 35 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,11 @@ PHP_SIMDJSON_API zend_class_entry *simdjson_value_error_ce;
3838
/* Single header file from fork of simdjson C project (to imitate php's handling of infinity/overflowing integers in json_decode) */
3939
#include "src/simdjson.h"
4040

41+
/* Define RETURN_THROWS macro in older php versions */
42+
#ifndef RETURN_THROWS
43+
#define RETURN_THROWS() do { ZEND_ASSERT(EG(exception)); (void) return_value; return; } while (0)
44+
#endif
45+
4146
ZEND_DECLARE_MODULE_GLOBALS(simdjson);
4247

4348
#if PHP_VERSION_ID >= 70200
@@ -48,7 +53,7 @@ ZEND_DECLARE_MODULE_GLOBALS(simdjson);
4853
ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(name, return_reference, required_num_args, type, NULL, allow_null)
4954
#endif
5055

51-
SIMDJSON_ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(simdjson_is_valid_arginfo, 0, 1, _IS_BOOL, 1)
56+
SIMDJSON_ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(simdjson_is_valid_arginfo, 0, 1, _IS_BOOL, 0)
5257
ZEND_ARG_TYPE_INFO(0, json, IS_STRING, 0)
5358
ZEND_ARG_TYPE_INFO(0, depth, IS_LONG, 0)
5459
ZEND_END_ARG_INFO()
@@ -92,12 +97,19 @@ static simdjson_php_parser *simdjson_get_parser() {
9297
// The simdjson parser accepts strings with at most 32-bit lengths, for now.
9398
#define SIMDJSON_MAX_DEPTH ((zend_long)((SIZE_MAX / 8) < (UINT32_MAX / 2) ? (SIZE_MAX / 8) : (UINT32_MAX / 2)))
9499

95-
static bool simdjson_validate_depth(zend_long depth) {
100+
static ZEND_COLD void simdjson_throw_depth_must_be_positive(const char *function_name, const int arg_num) {
101+
zend_throw_error(simdjson_value_error_ce, "%s(): Argument #%d ($depth) must be greater than zero", function_name, arg_num);
102+
}
103+
static ZEND_COLD void simdjson_throw_depth_too_large(const char *function_name, const int arg_num) {
104+
zend_throw_error(simdjson_value_error_ce, "%s(): Argument #%d ($depth) exceeds maximum allowed value of " ZEND_LONG_FMT, function_name, arg_num, SIMDJSON_MAX_DEPTH);
105+
}
106+
107+
static zend_always_inline bool simdjson_validate_depth(zend_long depth, const char *function_name, const int arg_num) {
96108
if (UNEXPECTED(depth <= 0)) {
97-
php_error_docref(NULL, E_WARNING, "Depth must be greater than zero");
109+
simdjson_throw_depth_must_be_positive(function_name, arg_num);
98110
return false;
99111
} else if (UNEXPECTED(depth > SIMDJSON_MAX_DEPTH)) {
100-
php_error_docref(NULL, E_WARNING, "Depth exceeds maximum allowed value of " ZEND_LONG_FMT, SIMDJSON_MAX_DEPTH);
112+
simdjson_throw_depth_too_large(function_name, arg_num);
101113
return false;
102114
}
103115
return true;
@@ -107,10 +119,10 @@ PHP_FUNCTION (simdjson_is_valid) {
107119
zend_string *json = NULL;
108120
zend_long depth = SIMDJSON_PARSE_DEFAULT_DEPTH;
109121
if (zend_parse_parameters(ZEND_NUM_ARGS(), "S|l", &json, &depth) == FAILURE) {
110-
return;
122+
RETURN_THROWS();
111123
}
112-
if (!simdjson_validate_depth(depth)) {
113-
RETURN_NULL();
124+
if (!simdjson_validate_depth(depth, "simdjson_is_valid", 2)) {
125+
RETURN_THROWS();
114126
}
115127
bool is_json = php_simdjson_is_valid(simdjson_get_parser(), ZSTR_VAL(json), ZSTR_LEN(json), depth);
116128
ZVAL_BOOL(return_value, is_json);
@@ -121,32 +133,33 @@ PHP_FUNCTION (simdjson_decode) {
121133
zend_long depth = SIMDJSON_PARSE_DEFAULT_DEPTH;
122134
zend_string *json = NULL;
123135
if (zend_parse_parameters(ZEND_NUM_ARGS(), "S|bl", &json, &assoc, &depth) == FAILURE) {
124-
return;
136+
RETURN_THROWS();
125137
}
126-
if (!simdjson_validate_depth(depth)) {
127-
RETURN_NULL();
138+
if (!simdjson_validate_depth(depth, "simdjson_decode", 2)) {
139+
RETURN_THROWS();
128140
}
129141
simdjson_php_error_code error = php_simdjson_parse(simdjson_get_parser(), ZSTR_VAL(json), ZSTR_LEN(json), return_value, assoc, depth);
130142
if (error) {
131143
php_simdjson_throw_jsonexception(error);
144+
RETURN_THROWS();
132145
}
133146
}
134147

135148
PHP_FUNCTION (simdjson_key_value) {
136-
137149
zend_string *json = NULL;
138150
zend_string *key = NULL;
139151
zend_bool assoc = 0;
140152
zend_long depth = SIMDJSON_PARSE_DEFAULT_DEPTH;
141153
if (zend_parse_parameters(ZEND_NUM_ARGS(), "SS|bl", &json, &key, &assoc, &depth) == FAILURE) {
142-
return;
154+
RETURN_THROWS();
143155
}
144-
if (!simdjson_validate_depth(depth)) {
145-
RETURN_NULL();
156+
if (!simdjson_validate_depth(depth, "simdjson_key_value", 4)) {
157+
RETURN_THROWS();
146158
}
147159
simdjson_php_error_code error = php_simdjson_key_value(simdjson_get_parser(), ZSTR_VAL(json), ZSTR_LEN(json), ZSTR_VAL(key), return_value, assoc, depth);
148160
if (error) {
149161
php_simdjson_throw_jsonexception(error);
162+
RETURN_THROWS();
150163
}
151164
}
152165

@@ -156,17 +169,18 @@ PHP_FUNCTION (simdjson_key_count) {
156169
zend_long depth = SIMDJSON_PARSE_DEFAULT_DEPTH;
157170
bool throw_if_uncountable = false;
158171
if (zend_parse_parameters(ZEND_NUM_ARGS(), "SS|lb", &json, &key, &depth, &throw_if_uncountable) == FAILURE) {
159-
return;
172+
RETURN_THROWS();
160173
}
161-
if (!simdjson_validate_depth(depth)) {
162-
RETURN_NULL();
174+
if (!simdjson_validate_depth(depth, "simdjson_key_count", 4)) {
175+
RETURN_THROWS();
163176
}
164177
simdjson_php_error_code error = php_simdjson_key_count(simdjson_get_parser(), ZSTR_VAL(json), ZSTR_LEN(json), ZSTR_VAL(key), return_value, depth, throw_if_uncountable);
165178
if (error) {
166179
if (error == SIMDJSON_PHP_ERR_KEY_COUNT_NOT_COUNTABLE && !throw_if_uncountable) {
167180
RETURN_LONG(0);
168181
}
169182
php_simdjson_throw_jsonexception(error);
183+
RETURN_THROWS();
170184
}
171185
}
172186

@@ -175,10 +189,10 @@ PHP_FUNCTION (simdjson_key_exists) {
175189
zend_string *key = NULL;
176190
zend_long depth = SIMDJSON_PARSE_DEFAULT_DEPTH;
177191
if (zend_parse_parameters(ZEND_NUM_ARGS(), "SS|l", &json, &key, &depth) == FAILURE) {
178-
return;
192+
RETURN_THROWS();
179193
}
180-
if (!simdjson_validate_depth(depth)) {
181-
return;
194+
if (!simdjson_validate_depth(depth, "simdjson_key_exists", 3)) {
195+
RETURN_THROWS();
182196
}
183197
simdjson_php_error_code error = php_simdjson_key_exists(simdjson_get_parser(), ZSTR_VAL(json), ZSTR_LEN(json), ZSTR_VAL(key), depth);
184198
switch (error) {
@@ -190,6 +204,7 @@ PHP_FUNCTION (simdjson_key_exists) {
190204
RETURN_FALSE;
191205
default:
192206
php_simdjson_throw_jsonexception(error);
207+
RETURN_THROWS();
193208
}
194209
}
195210

‎php_simdjson.h‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,13 @@ BEGIN_EXTERN_C()
5151
extern zend_module_entry simdjson_module_entry;
5252
#define phpext_simdjson_ptr &simdjson_module_entry
5353

54-
#define PHP_SIMDJSON_VERSION "2.2.0dev"
54+
#define PHP_SIMDJSON_VERSION "3.0.0dev"
55+
/**
56+
* PHP_SIMDJSON_VERSION_ID has the same format as PHP_VERSION_ID: Major version * 10000 + Minor version * 100 + Patch version.
57+
* This is meant for use by PECL extensions that depend on simdjson.
58+
*/
59+
#define PHP_SIMDJSON_VERSION_ID 30000
60+
5561
#define SIMDJSON_SUPPORT_URL "https://github.com/crazyxman/simdjson_php"
5662

5763
#define SIMDJSON_PARSE_DEFAULT_DEPTH 512

‎simdjson.stub.php‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,14 @@ class SimdJsonException extends RuntimeException {
1111
}
1212

1313
/**
14-
* Thrown for invalid depths, with similar behavior to php 8.0.
14+
* Thrown for error conditions on fields such as $depth that are not expected to be
15+
* from user-provided JSON, with similar behavior to php 8.0.
1516
*
1617
* NOTE: https://www.php.net/valueerror was added in php 8.0.
1718
* In older php versions, this extends Error instead.
19+
*
20+
* When support for php 8.0 is dropped completely,
21+
* a major release of simdjson will likely switch to a standard ValueError.
1822
*/
1923
class SimdJsonValueError extends ValueError {
2024
}

‎simdjson_arginfo.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/* This is a generated file, edit the .stub.php file instead.
2-
* Stub hash: 7d4b37b2b4c79e2802efe01a53bb11cdfbc62d7c */
2+
* Stub hash: b8ea2fddf5afdd9d45d2541d63169e659208d20e */
33

44

55

‎tests/decode_max_depth.phpt‎

Lines changed: 25 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -6,29 +6,37 @@ declare(strict_types=1);
66
ini_set('error_reporting', (string)E_ALL);
77
ini_set('display_errors', 'stderr');
88

9-
foreach ([1024, PHP_INT_MAX >> 1] as $depth) {
10-
$value = \simdjson_decode('[]', true, $depth);
11-
var_dump($value);
12-
$value = \simdjson_key_count('{"a":"b"}', 'a', $depth);
13-
var_dump($value);
14-
$value = \simdjson_key_value('{"a":{}}', 'a', true, $depth);
15-
var_dump($value);
16-
$value = \simdjson_is_valid('{}', $depth);
17-
var_dump($value);
9+
function dump(Closure $c) {
10+
try {
11+
var_dump($c());
12+
} catch (SimdJsonValueError $e) {
13+
printf("Caught %s: %s\n", get_class($e), $e->getMessage());
14+
}
15+
}
16+
17+
foreach ([0, 1024, PHP_INT_MAX >> 1] as $depth) {
18+
dump(function () use ($depth) { return simdjson_decode('[]', true, $depth); });
19+
dump(function () use ($depth) { return simdjson_key_count('{"a":"b"}', 'a', $depth); });
20+
dump(function () use ($depth) { return simdjson_key_value('{"a":{}}', 'a', true, $depth); });
21+
dump(function () use ($depth) { return simdjson_key_exists('{"a":{}}', 'a', $depth); });
22+
dump(function () use ($depth) { return simdjson_is_valid('{}', $depth); });
1823
}
1924
?>
2025
--EXPECTF--
26+
Caught SimdJsonValueError: simdjson_decode(): Argument #2 ($depth) must be greater than zero
27+
Caught SimdJsonValueError: simdjson_key_count(): Argument #4 ($depth) must be greater than zero
28+
Caught SimdJsonValueError: simdjson_key_value(): Argument #4 ($depth) must be greater than zero
29+
Caught SimdJsonValueError: simdjson_key_exists(): Argument #3 ($depth) must be greater than zero
30+
Caught SimdJsonValueError: simdjson_is_valid(): Argument #2 ($depth) must be greater than zero
2131
array(0) {
2232
}
2333
int(0)
2434
array(0) {
2535
}
2636
bool(true)
27-
Warning: simdjson_decode(): Depth exceeds maximum allowed value of %d in %s on line 7
28-
NULL
29-
Warning: simdjson_key_count(): Depth exceeds maximum allowed value of %d in %s on line 9
30-
NULL
31-
Warning: simdjson_key_value(): Depth exceeds maximum allowed value of %d in %s on line 11
32-
NULL
33-
Warning: simdjson_is_valid(): Depth exceeds maximum allowed value of %d in %s on line 13
34-
NULL
37+
bool(true)
38+
Caught SimdJsonValueError: simdjson_decode(): Argument #2 ($depth) exceeds maximum allowed value of %d
39+
Caught SimdJsonValueError: simdjson_key_count(): Argument #4 ($depth) exceeds maximum allowed value of %d
40+
Caught SimdJsonValueError: simdjson_key_value(): Argument #4 ($depth) exceeds maximum allowed value of %d
41+
Caught SimdJsonValueError: simdjson_key_exists(): Argument #3 ($depth) exceeds maximum allowed value of %d
42+
Caught SimdJsonValueError: simdjson_is_valid(): Argument #2 ($depth) exceeds maximum allowed value of %d

0 commit comments

Comments
 (0)