RFC - json_validate() #9399

juan-morales · 2022-08-22T12:53:39Z

https://wiki.php.net/rfc/json_validate

juan-morales · 2022-08-22T22:44:43Z

Here is my new PR, I changed the name of the function to json_validate as you were the 3rd person suggesting that name actually 👍

Honestly, I can go further with the code, but I would have to touch more the parser, and you mentioned that I should stay away from json_decode the more that I could. What I had in mind (but I did not do it) was to move out all hooks that the parser could trigger, out of the parser itself, into a different place; giving the developer the chance to re-use the parser with custom function hooks, but doing so would involve move things to different places, and I though that doing something like that would affect negatively my RFC; at the end, is the functionality what I car more about.

Anyway, hopefully, this is in the direction you suggested me.

I don't mind changing the code as many times as needed, seriously.

Thanks in advance, and I will wait for your reply.

PD: I will keep the PR as Draft, and keep the commits also, until the moment to squash comes.

juan-morales · 2022-08-25T09:14:19Z

@cmb69 @dstogov @bukka @nikic

Whenever you can, can you give me a feedback about this?

Thanks in advance 😄

PD:

Sorry for specifically tagging you, but you are the ones I have being in contact for this RFC so far
The RFC document still needs some completion, but I would lik to know if this implementation is fine enough

bukka · 2022-08-25T11:41:03Z

It looks good to me for the RFC purpose. There are some minor things that we can iron out if the RFC gets accepted but otherwise it's fine.

cmb69

Yeah, certainly good enough to proceed with the RFC process. Thank you!

ext/json/tests/json_validate_001.phpt

ext/json/json_parser.y

ext/json/json.c

derickr · 2022-08-26T09:34:47Z

ext/json/json.c

+	}
+
+	if (!(options & PHP_JSON_THROW_ON_ERROR)) {
+		JSON_G(error_code) = PHP_JSON_ERROR_NONE;


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.

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.

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

I wrote this down in the RFC as an open question

derickr · 2022-08-26T09:35:15Z

ext/json/json.c

+	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;


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

I will check this and will be back to you

@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?

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

I wrote this down in the RFC as an open question

ext/json/json_parser.y

ext/json/tests/json_validate_002.phpt

ext/json/json.c

juan-morales · 2022-08-27T16:30:39Z

I Will update tomorrow with all reviews

…nput strings

…only

…er object

ext/json/json_parser.y

…IDATE_ONLY

juan-morales · 2022-08-28T12:10:13Z

@bukka I already pushed a change your review.

Can you take a look at the change again?

ext/json/json.c

ext/json/json_parser.y

ext/json/tests/json_validate_005.phpt

ext/json/json.c

ext/json/tests/json_validate_requires.inc

ext/json/json_parser.y

ext/json/php_json_parser.h

ext/json/json.c

ext/json/json_parser.y

juan-morales · 2022-09-26T00:46:21Z

@TysonAndre @bukka wow!!!!! Thanks for such a feedback,I will make the changes later today, traveling at the moment

…ate methods as pointer

juan-morales · 2022-09-27T15:14:44Z

Thanks in advance, hopefully I got the comments in the right way. .... at least the automatic tests are green

ext/json/tests/json_validate_005.phpt

juan-morales · 2022-10-07T13:52:31Z

@bukka @TysonAndre @cmb69 , RFC Accepted! 🥳

Should I squeeze the commits on this PR to one?

Is there anything else I should do besides updating the RFC as stated in step 7.1 of the RFC process? RFC How-To

TimWolla · 2022-10-07T18:17:32Z

Is there anything else I should do besides updating the RFC as stated in step 7.1 of the RFC

You should likely send another email to the list, mentioning that the vote finished and summarizing the voting results.

TysonAndre

I don't see any issues, the implementation looks correct. One optional style nit on the bit flag check.

TysonAndre · 2022-10-07T22:15:50Z

ext/json/json.c

+	ZEND_PARSE_PARAMETERS_END();
+
+
+	if ((options != 0) && (options != PHP_JSON_INVALID_UTF8_IGNORE)) {


Suggested change

if ((options != 0) && (options != PHP_JSON_INVALID_UTF8_IGNORE)) {

if (options & ~PHP_JSON_INVALID_UTF8_IGNORE) {

this works, but checking for any bits found in bitwise not in allowed flags is shorter and conventional - it also works and is used for combinations of multiple flags

ext/json/json.c

TimWolla · 2022-10-08T13:28:25Z

@TysonAndre Will you also make the NEWS/UPGRADING changes?

TysonAndre · 2022-10-08T13:53:49Z

Will you also make the NEWS/UPGRADING changes?

Done, I added documentation of those changes in 4ed8d52 and 68301b1

staabm · 2023-06-27T15:21:49Z

@juan-morales do you plan to add a new php.net manual page for this new function?

I would like to add support for it in PHPStan and was wondering which JSON flags are supported

TimWolla · 2023-06-27T15:24:10Z

do you plan to add a new php.net manual page for this new function?

Based off my experience with PHP 8.2, the manual pages are written “in bulk” based off the UPGRADING notes once the release comes nearer. See: php/doc-en#1803

was wondering which JSON flags are supported

Only PHP_JSON_INVALID_UTF8_IGNORE.

marksgerasimovs · 2023-10-16T07:37:33Z

Greetings, I have a small corner case I don't think gets addressed here, what if we have json object where same key is used two times, php json_decode would just overwrite it and leave the last in the order, but JAVA for example would throw an error as the json is not valid. I do think it is okey that PHP still can parse it, and json_decode remains silent on an issue in a way, but possibly a function that is validating my json should be giving me information about it. What do you think guys?

juan-morales · 2023-10-16T12:08:17Z

Hello @marksgerasimovs , thanks for the message.
This is a very good question. I Will address this in the INTERNALS email list

a1vin1au · 2025-01-06T04:05:02Z

Update: Thanks to Ayesh for pointing out that it is a valid JSON.

~~Hi I just found a bug in both 8.3.15 and 8.4.2~~

~~when string "" it will returns true, but it should be false as it is not a valid JSON, right?~~

json_validate('""') // return true

Below screenshots, I tested function in Laravel thinker with PHP 8.3.15 and 8.4.2

Ayesh · 2025-01-06T04:11:39Z

@a1vin1au

when string "" it will returns true, but it should be false as it is not a valid JSON, right?

"" is a valid JSON string. You can confirm this on JavaScript by running this in a browser console:

JSON.parse('""');

It returns an empty string, which is consistent with PHP's json_decode.

a1vin1au · 2025-01-06T04:19:32Z

@Ayesh
Oh, I never knew that "" was a valid JSON string, I'm coding with something like {"a":"b"} the whole time.
Thank you for letting me learn new thing

juan-morales · 2025-01-06T11:30:41Z

Remember, in this context a "valid JSON" is a JSON valid for json_decode 😄

cmb69 · 2025-01-06T11:36:27Z

An empty string ("") is valid JSON according to https://datatracker.ietf.org/doc/html/rfc8259 (as well as https://datatracker.ietf.org/doc/html/rfc7159); it is only invalid for the informational https://datatracker.ietf.org/doc/html/rfc4627.html.

json_validate()

0405a86

github-actions bot added Extension: json SAPI: fuzzer labels Aug 22, 2022

jcm and others added 3 commits August 22, 2022 15:42

retrigger pipelines

8f86a67

005 performance test

344687f

Add new line test 001

cfbee85

TimWolla added the RFC label Aug 23, 2022

cmb69 reviewed Aug 25, 2022

View reviewed changes

cmb69 marked this pull request as ready for review August 25, 2022 11:52

juan-morales changed the title ~~RFC - json_validate() - WIP~~ RFC - json_validate() Aug 25, 2022

drealecs reviewed Aug 26, 2022

View reviewed changes

ext/json/tests/json_validate_001.phpt Show resolved Hide resolved

derickr reviewed Aug 26, 2022

View reviewed changes

jcm added 2 commits August 26, 2022 13:28

Add new line at the end of tests

85dcc6a

Add PHP_JSON_VALIDATE_ONLY constant instead of validate_only

6688e20

TysonAndre reviewed Aug 27, 2022

View reviewed changes

ext/json/json.c Outdated Show resolved Hide resolved

jcm added 4 commits August 28, 2022 11:57

Add test comparing results with json_decode() on different types of i…

47756e9

…nput strings

group together function hooks for the json parser used by validation …

ae041be

…only

Remove usage of flag PHP_JSON_THROW_ON_ERROR

8e97349

Add test cases for invalid types on test 006

52ab14e

github-actions bot removed the SAPI: fuzzer label Aug 28, 2022

Stop propagation of parser option PHP_JSON_VALIDATE_ONLY to the scann…

1b1b4f3

…er object

bukka requested changes Aug 28, 2022

View reviewed changes

ext/json/json_parser.y Outdated Show resolved Hide resolved

get_validate_methods() usage instead of options constant PHP_JSON_VAL…

baf8ec6

…IDATE_ONLY

fix qualifier error

d038d1f

TysonAndre reviewed Aug 28, 2022

View reviewed changes

TysonAndre reviewed Sep 25, 2022

View reviewed changes

ext/json/json.c Outdated Show resolved Hide resolved

bukka reviewed Sep 25, 2022

View reviewed changes

ext/json/tests/json_validate_requires.inc Outdated Show resolved Hide resolved

bukka requested changes Sep 25, 2022

View reviewed changes

juan-morales added 4 commits September 26, 2022 12:27

Fix return type to bool

7331b47

Optimizations usage of ZVAL_NULL instead of empty array. Return valid…

2e92c03

…ate methods as pointer

dependencies

d71b16f

adjust indentation in test from 2 to 4 spaces

9223115

TysonAndre reviewed Sep 28, 2022

View reviewed changes

ext/json/tests/json_validate_005.phpt Outdated Show resolved Hide resolved

Remove test of passing NULL as parameter

a08d066

TysonAndre approved these changes Oct 7, 2022

View reviewed changes

TysonAndre requested changes Oct 7, 2022

View reviewed changes

ext/json/json.c Show resolved Hide resolved

TysonAndre approved these changes Oct 7, 2022

View reviewed changes

TysonAndre merged commit 2e8699f into php:master Oct 8, 2022

staabm mentioned this pull request Jun 28, 2023

Implement JsonValidateTypeSpecifyingExtension phpstan/phpstan-src#2491

Closed

jrfnl mentioned this pull request Aug 10, 2023

PHP 8.3 | NewFunctions: detect use of json_validate() (RFC) PHPCompatibility/PHPCompatibility#1622

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RFC - json_validate() #9399

RFC - json_validate() #9399

juan-morales commented Aug 22, 2022

juan-morales commented Aug 22, 2022

juan-morales commented Aug 25, 2022 •

edited

Loading

bukka commented Aug 25, 2022

cmb69 left a comment

derickr Aug 26, 2022

juan-morales Aug 26, 2022

juan-morales Aug 26, 2022

juan-morales Aug 28, 2022

derickr Aug 26, 2022

juan-morales Aug 26, 2022

juan-morales Aug 26, 2022 •

edited

Loading

cmb69 Aug 26, 2022

juan-morales Aug 28, 2022

juan-morales commented Aug 27, 2022

juan-morales commented Aug 28, 2022 •

edited

Loading

juan-morales commented Sep 26, 2022

juan-morales commented Sep 27, 2022

juan-morales commented Oct 7, 2022 •

edited

Loading

TimWolla commented Oct 7, 2022

TysonAndre left a comment •

edited

Loading

TysonAndre Oct 7, 2022

TimWolla commented Oct 8, 2022

TysonAndre commented Oct 8, 2022

staabm commented Jun 27, 2023

TimWolla commented Jun 27, 2023

marksgerasimovs commented Oct 16, 2023 •

edited

Loading

juan-morales commented Oct 16, 2023

a1vin1au commented Jan 6, 2025 •

edited

Loading

Ayesh commented Jan 6, 2025

a1vin1au commented Jan 6, 2025

juan-morales commented Jan 6, 2025

cmb69 commented Jan 6, 2025

		ZEND_PARSE_PARAMETERS_END();


		if ((options != 0) && (options != PHP_JSON_INVALID_UTF8_IGNORE)) {

	if ((options != 0) && (options != PHP_JSON_INVALID_UTF8_IGNORE)) {
	if (options & ~PHP_JSON_INVALID_UTF8_IGNORE) {

RFC - json_validate() #9399

RFC - json_validate() #9399

Conversation

juan-morales commented Aug 22, 2022

juan-morales commented Aug 22, 2022

juan-morales commented Aug 25, 2022 • edited Loading

bukka commented Aug 25, 2022

cmb69 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

juan-morales Aug 26, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

juan-morales commented Aug 27, 2022

juan-morales commented Aug 28, 2022 • edited Loading

juan-morales commented Sep 26, 2022

juan-morales commented Sep 27, 2022

juan-morales commented Oct 7, 2022 • edited Loading

TimWolla commented Oct 7, 2022

TysonAndre left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TimWolla commented Oct 8, 2022

TysonAndre commented Oct 8, 2022

staabm commented Jun 27, 2023

TimWolla commented Jun 27, 2023

marksgerasimovs commented Oct 16, 2023 • edited Loading

juan-morales commented Oct 16, 2023

a1vin1au commented Jan 6, 2025 • edited Loading

Ayesh commented Jan 6, 2025

a1vin1au commented Jan 6, 2025

juan-morales commented Jan 6, 2025

cmb69 commented Jan 6, 2025

juan-morales commented Aug 25, 2022 •

edited

Loading

juan-morales Aug 26, 2022 •

edited

Loading

juan-morales commented Aug 28, 2022 •

edited

Loading

juan-morales commented Oct 7, 2022 •

edited

Loading

TysonAndre left a comment •

edited

Loading

marksgerasimovs commented Oct 16, 2023 •

edited

Loading

a1vin1au commented Jan 6, 2025 •

edited

Loading