Skip to content

Fix json_encode regression with JSON_PRETTY_PRINT #6811

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

Closed
wants to merge 1 commit into from
Closed

Fix json_encode regression with JSON_PRETTY_PRINT #6811

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
13 changes: 12 additions & 1 deletion ext/json/json_encoder.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ static int php_json_encode_array(smart_str *buf, zval *val, int options, php_jso
} else if (Z_OBJ_P(val)->properties == NULL
&& Z_OBJ_HT_P(val)->get_properties_for == NULL
&& Z_OBJ_HT_P(val)->get_properties == zend_std_get_properties) {
/* Optimized version without rebulding properties HashTable */
/* Optimized version without rebuilding properties HashTable */
zend_object *obj = Z_OBJ_P(val);
zend_class_entry *ce = obj->ce;
zend_property_info *prop_info;
Expand All @@ -133,7 +133,11 @@ static int php_json_encode_array(smart_str *buf, zval *val, int options, php_jso
}

PHP_JSON_HASH_PROTECT_RECURSION(obj);

smart_str_appendc(buf, '{');

++encoder->depth;

for (i = 0; i < ce->default_properties_count; i++) {
prop_info = ce->properties_info_table[i];
if (!prop_info) {
Expand Down Expand Up @@ -174,6 +178,13 @@ static int php_json_encode_array(smart_str *buf, zval *val, int options, php_jso
return FAILURE;
}
}

--encoder->depth;

if (need_comma) {
php_json_pretty_print_char(buf, options, '\n');
php_json_pretty_print_indent(buf, options, encoder);
}
smart_str_appendc(buf, '}');
PHP_JSON_HASH_UNPROTECT_RECURSION(obj);
return SUCCESS;
Expand Down
55 changes: 55 additions & 0 deletions ext/json/tests/json_encode_pretty_print2.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
--TEST--
json_encode() with JSON_PRETTY_PRINT on declared properties
--FILE--
<?php
class MyClass {
public $x;
public $y;
public function __construct($x = 123, $y = []) {
$this->x = $x;
$this->y = $y;
}
}

class HasNoProperties {}

echo json_encode(new HasNoProperties()), "\n";
echo json_encode(new HasNoProperties(), JSON_PRETTY_PRINT), "\n";

echo json_encode(new MyClass()), "\n";
echo json_encode(new MyClass(), JSON_PRETTY_PRINT), "\n";
$obj = new MyClass();
$obj->dynamic = new MyClass(null, []);
echo json_encode($obj), "\n";
echo json_encode($obj, JSON_PRETTY_PRINT), "\n";
$obj = new MyClass();
unset($obj->y);
echo json_encode($obj), "\n";
echo json_encode($obj, JSON_PRETTY_PRINT), "\n";
unset($obj->x);
echo json_encode($obj), "\n";
echo json_encode($obj, JSON_PRETTY_PRINT), "\n";
?>
--EXPECT--
{}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The output in php 8.1.0-dev after this patch matches what I'm seeing in php 7.4

I think the original patch was worth it to avoid an increase in memory usage caused by json_encode(), and shouldn't cause any invalid memory accesses - there may be a tiny change in output for edge cases involving JsonSerializable and regular objects, but I don't expect code to rely on that behavior

{}
{"x":123,"y":[]}
{
"x": 123,
"y": []
}
{"x":123,"y":[],"dynamic":{"x":null,"y":[]}}
{
"x": 123,
"y": [],
"dynamic": {
"x": null,
"y": []
}
}
{"x":123}
{
"x": 123
}
{}
{}