RFC json_validate() - status: Under Discussion

David Gebler

4 years ago

I'm not a voter on RFCs so my input may be largely irrelevant here but for
discussion purposes:

I remain unconvinced regarding the justification for this proposal. I'm not
saying there's a strong reason to NOT implement it, but I'm not convinced
it's really going to be a significant benefit to many people at all.

I agree that the number of userland implementations for a "is_valid_json"
type function including in some widely used frameworks and systems
indicates there's some degree of demand in the ecosystem for validating a
JSON string.

But the more salient question is whether there is a significant demand for
whatever memory and speed benefit the implementation of a new core ext_json
function delivers; that is, has it been established that the use of
json_decode or common userland solutions are in practice not good enough?

There are many examples of userland code which could be faster and more
memory efficient if they were written in C and compiled in, so the mere
fact this proposal may introduce a somewhat faster way of validating a JSON
string over decoding it is not necessarily a sufficient reason to include
it.

Are there are examples of raising issues for frameworks or systems saying
they need to validate some JSON but the only existing solutions available
to them are causing memory limit errors, or taking too long? The Stack
Overflow question linked on the RFC says "I need a really, really fast
method of checking if a string is JSON or not."

"Really, really fast" is subjective. No context or further information is
given about what that person would regard as an acceptable time to validate
what size blob of valid or invalid JSON, or why. Indeed that same page
offers a userland solution based around only going to json_decode if some
other much simpler checks on the input are indeterminate for validation
purposes. Haven't tested it personally but no doubt in the vast majority of
cases it is sufficiently performant.

In most real world use cases [that I've encountered over the years] JSON
blobs tend to be quite small. I have dealt with much, much larger JSON
blobs, up to a few hundred MB, and in those cases I've used a streaming
parser. If you're talking about JSON that size, a streaming parser is the
only realistic answer - you probably don't want to drop a 300MB string in
to this RFC's new function either, if performance and memory efficiency is
your concern.

So I'm curious as to whether a real world example can be given where the
efficiency difference between json_decode and a new json_validate function
would be important to the system, whether anyone's encountered a scenario
where this would have made a real difference to them.

Deleu

4 years ago

On Thu, Aug 25, 2022, 9:12 PM David Gebler <davidgebler@gmail.com> wrote:

>
> I agree that the number of userland implementations for a "is_valid_json"
> type function including in some widely used frameworks and systems
> indicates there's some degree of demand in the ecosystem for validating a
> JSON string.
>

I don't have an option about the rest of your email, but I wanted to point
out that the vast userland need for validating JSON right now is being
"hacked" for lack of a better functionality. Whether it's justifiable to
bring this to core is up for debate, but I at least take it as a
valid point that "asking a computer to do a task just to check whether it
failed" is odd and limiting to say the least

juan carlos morales

4 years ago

Thanks for participating. Cheers.

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

Implementation: https://github.com/php/php-src/pull/9399

juan carlos morales

4 years ago

Hello David, thanks for your feedback.

I believe that the answer to most of your concerns/questions related to
other projects that would benefit out of something like this is already
answered by me in the RFC. I mentioned major PHP projects that will
directly benefit out of something like this.

About the demand, well, that is also being written in the RFC.

Regarding the question in StackOverflow, was just mentioned to expose the
"demand" of validating a json string, and is one of the most ranked
questions, is in the top 10 of questions with php && json tags. I believe
that such a thing is talking by iself.

Providing real use cases wont make a difference, because the number of
examples are infinite honestly.

Yes there are userland packages that can do this for us, but in big
products on big companies, relying on 3rd party packages is not always
cool, I have being experiencing that in my company actually.

Regarding examples of raising issues ... there are a LOT in github, and the
only possible solution is always to increase the memory limit or use a 3rd
party package (which as I said, is not always suitable).

But ... lets be objective, or try to be at least ... PHP has a parser
already, this functionality is just allowing us to use it without the need
to consume memory.

I see it useful, actually, this RFC is the result of a real need we have in
my current company at the moment, and after digging around the internet I
found hat we were not the only ones.

Thanks for taking our time on this, I appreciate it.

Tim Düsterhus

4 years ago

Hi

On 8/25/22 21:11, David Gebler wrote:

> There are many examples of userland code which could be faster and more
> memory efficient if they were written in C and compiled in, so the mere
> fact this proposal may introduce a somewhat faster way of validating a JSON
> string over decoding it is not necessarily a sufficient reason to include
> it.
>
> Are there are examples of raising issues for frameworks or systems saying
> they need to validate some JSON but the only existing solutions available
> to them are causing memory limit errors, or taking too long? The Stack
> Overflow question linked on the RFC says "I need a really, really fast
> method of checking if a string is JSON or not."

The proposed function is meant to be used for validation. Validation
processes by definition need to deal with untrusted data. So the input
data might even be actively malicious in order to tie up resources on
the server (DoS attack - single D there).

> In most real world use cases [that I've encountered over the years] JSON
> blobs tend to be quite small. I have dealt with much, much larger JSON

Yes well-formed JSON from a trusted source tends to be small-ish. But a
validation function also needs to deal with non-well-formed JSON,
otherwise you would not need to validate it.

I was able to use up an extra 100 MB of RAM with a 3 MB input that is
invalid JSON when using json_decode(), just for it to reject the input.

For json_validate() the extra memory (as per memory_get_peak_usage())
required for the same operation effectively zero. It was able to deal
with 60 MB of input just fine.

I've attached the script I used for the test. I left out the actual JSON
string to not give script kiddies a loaded weapon, but you likely should
be able to craft some input yourself.

> blobs, up to a few hundred MB, and in those cases I've used a streaming
> parser. If you're talking about JSON that size, a streaming parser is the
> only realistic answer - you probably don't want to drop a 300MB string in
> to this RFC's new function either, if performance and memory efficiency is
> your concern.
>
> So I'm curious as to whether a real world example can be given where the
> efficiency difference between json_decode and a new json_validate function
> would be important to the system, whether anyone's encountered a scenario
> where this would have made a real difference to them.
>

While my example is not a real world example, I don't believe it's a
stretch to say it can be applied as-is to the real world.

So IMO:

- The proposed function does exactly what it promises to do, not more,
not less.
- If it's introduced, then it is going to be the obvious choice for JSON
validation and at the same time it is going to be the best choice for
JSON validation. I strongly believe it is a good thing if users are
steered to make the correct choice by default without needing to invest
brain cycles.
- The patch is pretty small, because the hard work of JSON parsing is
already implemented.
- Userland implementations are non-obvious and non-trivial as evidenced
by the examples in the RFC: They are all slightly different and one of
them even mishandles a plain `false` input, because it does not check
json_last_error().
- Userland implementations are also either less efficient (relying on
json_decode()) or potentially inconsistent (hand-rolling a validating
parser).

Best regards
Tim Düsterhus

juan carlos morales

4 years ago

Hey Tim, thanks a lot for participating and taking your time to use
the code and making your own tests, I highly appreciate it.

Regards.

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

Implementation: https://github.com/php/php-src/pull/9399

Michał Brzuchalski

4 years ago

czw., 25 sie 2022, 21:12 użytkownik David Gebler <davidgebler@gmail.com>
napisał:

> I'm not a voter on RFCs so my input may be largely irrelevant here but for
> discussion purposes:
>
> I remain unconvinced regarding the justification for this proposal. I'm not
> saying there's a strong reason to NOT implement it, but I'm not convinced
> it's really going to be a significant benefit to many people at all.
>
> I agree that the number of userland implementations for a "is_valid_json"
> type function including in some widely used frameworks and systems
> indicates there's some degree of demand in the ecosystem for validating a
> JSON string.
>
> But the more salient question is whether there is a significant demand for
> whatever memory and speed benefit the implementation of a new core ext_json
> function delivers; that is, has it been established that the use of
> json_decode or common userland solutions are in practice not good enough?
>
> There are many examples of userland code which could be faster and more
> memory efficient if they were written in C and compiled in, so the mere
> fact this proposal may introduce a somewhat faster way of validating a JSON
> string over decoding it is not necessarily a sufficient reason to include
> it.
>
> Are there are examples of raising issues for frameworks or systems saying
> they need to validate some JSON but the only existing solutions available
> to them are causing memory limit errors, or taking too long? The Stack
> Overflow question linked on the RFC says "I need a really, really fast
> method of checking if a string is JSON or not."
>
> "Really, really fast" is subjective. No context or further information is
> given about what that person would regard as an acceptable time to validate
> what size blob of valid or invalid JSON, or why. Indeed that same page
> offers a userland solution based around only going to json_decode if some
> other much simpler checks on the input are indeterminate for validation
> purposes. Haven't tested it personally but no doubt in the vast majority of
> cases it is sufficiently performant.
>
> In most real world use cases [that I've encountered over the years] JSON
> blobs tend to be quite small. I have dealt with much, much larger JSON
> blobs, up to a few hundred MB, and in those cases I've used a streaming
> parser. If you're talking about JSON that size, a streaming parser is the
> only realistic answer - you probably don't want to drop a 300MB string in
> to this RFC's new function either, if performance and memory efficiency is
> your concern.
>
> So I'm curious as to whether a real world example can be given where the
> efficiency difference between json_decode and a new json_validate function
> would be important to the system, whether anyone's encountered a scenario
> where this would have made a real difference to them.
>

I share the same opinion you expressed here even though you admit in recent
email that you changed your mind.

In recent versions we tend to accept more and more small standard library
functions with IMO questionable argumentation. The same goes here and I'm
not convinced we should introduce next small function that can be simply
implemented in user land.

Any example testing > 3MB JSON string are for me edge cases that normally
don't happen often to deserve special treatment.

If we keep the tendency to pollute already bloated standard library with an
army of small functions that could have not exists and be replaced with
normal PHP counterparts IMHO we'll end with frustration from developers as
I believe DX slowly falls down here.

Cheers,
Michał Marcin Brzuchalski

Dusk

4 years ago

On Aug 25, 2022, at 21:47, Michał Marcin Brzuchalski <michal.brzuchalski@gmail.com> wrote:

> The same goes here and I'm not convinced we should introduce next small function that can be simply implemented in user land.

What "simple implementation in userland" do you have in mind? Can you provide an example?

json_decode() is not an acceptable substitute here -- as David Gebler has observed, decoding a large JSON structure can have a significant impact on memory usage, even if the data is immediately discarded. Any implementation based on string processing, on the other hand, is likely to be dramatically slower, and may have subtle differences in behavior from PHP's JSON parser.

Michał Brzuchalski

4 years ago

Hi Dusk,

pt., 26 sie 2022 o 08:17 Dusk <dusk@woofle.net> napisał(a):

> On Aug 25, 2022, at 21:47, Michał Marcin Brzuchalski <
> michal.brzuchalski@gmail.com> wrote:
> > The same goes here and I'm not convinced we should introduce next small
> function that can be simply implemented in user land.
>
> What "simple implementation in userland" do you have in mind? Can you
> provide an example?
>
> json_decode() is not an acceptable substitute here -- as David Gebler has
> observed, decoding a large JSON structure can have a significant impact on
> memory usage, even if the data is immediately discarded. Any implementation
> based on string processing, on the other hand, is likely to be dramatically
> slower, and may have subtle differences in behavior from PHP's JSON parser.

A `json_decode()` is a substitute that IMO solves 99% of use cases.
If I'd follow your logic and accept every small addition that handles 1% of
use cases, somebody will raise another RFC
for simplexml_validate_string or yaml_validate and the next
PhpToken::validate.
All above can be valid if we trust that people normally validate 300MB
payloads to do nothing if they DON'T fail and there is nothing strange
about that.

Cheers,

Hans Henrik Bergan

4 years ago

quote

> you can't efficiently validate JSON in userland

Has anyone actually put that claim to the test? Has anyone actually made a
userland json validator (not just wrap json_decode()/json_last_error()) for
performance comparison?
( if not, https://www.json.org/JSON_checker/JSON_checker.c  would probably
be a good start)

On Fri, 26 Aug 2022 at 11:00, Michał Marcin Brzuchalski <
michal.brzuchalski@gmail.com> wrote:

Tim Düsterhus

4 years ago

Hi

On 8/26/22 11:14, Hans Henrik Bergan wrote:

>> you can't efficiently validate JSON in userland
>
> Has anyone actually put that claim to the test? Has anyone actually made a
> userland json validator (not just wrap json_decode()/json_last_error()) for
> performance comparison?
> ( if not, https://www.json.org/JSON_checker/JSON_checker.c  would probably
> be a good start)
>

Worded like "you can't efficiently" the claim is false. Of course you
can memory-efficiently validate the input by traversing the string byte
by byte and keeping track of the nesting.

However the points that make a userland implementation infeasible are:

1. Writing a JSON parser is non-trivial as evidenced by:
https://github.com/nst/JSONTestSuite. I expect userland implementations
to be subtly buggy in edge cases. The JSON parser in PHP 7.0+ is
certainly more battle-tested and in fact it appears to pass all of the
tests in the linked test suite.

2. Even if the userland implementation is written very carefully, it
might behave differently than the native implementation used by
json_decode() (e.g. because the latter is buggy for some reason or
because the correct behavior is undefined). This would imply that an
input string that was successfully validated by your userland parser
might ultimately fail to parse when passed to json_decode(). This is
exactly what you don't want to happen.

Best regards
Tim Düsterhus

Michał Brzuchalski

4 years ago

Hi Tim,

pt., 26 sie 2022 o 12:15 Tim Düsterhus <tim@bastelstu.be> napisał(a):

> Hi
>
> On 8/26/22 11:14, Hans Henrik Bergan wrote:
> >> you can't efficiently validate JSON in userland
> >
> > Has anyone actually put that claim to the test? Has anyone actually made
> a
> > userland json validator (not just wrap json_decode()/json_last_error())
> for
> > performance comparison?
> > ( if not, https://www.json.org/JSON_checker/JSON_checker.c  would
> probably
> > be a good start)
> >
>
> Worded like "you can't efficiently" the claim is false. Of course you
> can memory-efficiently validate the input by traversing the string byte
> by byte and keeping track of the nesting.
>
> However the points that make a userland implementation infeasible are:
>
> 1. Writing a JSON parser is non-trivial as evidenced by:
> https://github.com/nst/JSONTestSuite. I expect userland implementations
> to be subtly buggy in edge cases. The JSON parser in PHP 7.0+ is
> certainly more battle-tested and in fact it appears to pass all of the
> tests in the linked test suite.
>
> 2. Even if the userland implementation is written very carefully, it
> might behave differently than the native implementation used by
> json_decode() (e.g. because the latter is buggy for some reason or
> because the correct behavior is undefined). This would imply that an
> input string that was successfully validated by your userland parser
> might ultimately fail to parse when passed to json_decode(). This is
> exactly what you don't want to happen.
>

Now this is an argument I could think of.
But that one is not even mentioned in RFC.

The JSON_checker.c example delivered by json.org is probably not something
impossible
as it required around 1h of work to port it see working implementation
here https://gist.github.com/brzuchal/37e888d9b13937891c3e05fead5042bc

Cheers,
Michał Marcin Brzuchalski

juan carlos morales

4 years ago

Just Want to remind that this discussion is not about "a json parser can be
written in PHP or not?".

We Have a JSON parser already in the core, ready to be use for validation.

Does it make sense to have another parser in User land to do validation if
We already have one?

Is there a better way to do validation other than json_decode?

El vie., 26 de agosto de 2022 12:48, Michał Marcin Brzuchalski <
michal.brzuchalski@gmail.com> escribió:

juan carlos morales

4 years ago

El vie., 26 de agosto de 2022 13:08, juan carlos morales <
dev.juan.morales@gmail.com> escribió:

>
> Is there a better way to do validation other than json_decode?
>

Better way than json_decode... Always from the core perspective

juan carlos morales

4 years ago

El vie, 26 ago 2022 a las 11:00, Michał Marcin Brzuchalski
(<michal.brzuchalski@gmail.com>) escribió:

>
> A `json_decode()` is a substitute that IMO solves 99% of use cases.
> If I'd follow your logic and accept every small addition that handles 1% of use cases, somebody will raise another RFC
> for simplexml_validate_string or yaml_validate and the next PhpToken::validate.
> All above can be valid if we trust that people normally validate 300MB payloads to do nothing if they DON'T fail and there is nothing strange about that.
>
> Cheers,
>

How can you make such an assertion in those numbers (99% of use cases
and son on, that you mention) ? Can you give us more information about
this assertions?

I have provide real examples where the need to validate-only a
json-string is actually needed, also the need from our developers
community asking for this.

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

Implementation: https://github.com/php/php-src/pull/9399

juan carlos morales

4 years ago

El vie, 26 ago 2022 a las 11:26, juan carlos morales
(<dev.juan.morales@gmail.com>) escribió:

>
> How can you make such an assertion in those numbers (99% of use cases
> and son on, that you mention) ? Can you give us more information about
> this assertions?
>
> I have provide real examples where the need to validate-only a
> json-string is actually needed, also the need from our developers
> community asking for this.
>
> RFC: https://wiki.php.net/rfc/json_validate
>
> Implementation: https://github.com/php/php-src/pull/9399

By the way you dont need to frame this function to handle 300MB of
json-string, as I already mentioned a couple of times, that a 2 MB
json-sring validation with json_decode() needs a large amount of
memory to be validated, possibly hitting the allowed memory limit,
which usually is not that high at all.

RFC: https://wiki.php.net/rfc/json_validate
Implementation: https://github.com/php/php-src/pull/9399

Michał Brzuchalski

4 years ago

Hi Juan,

pt., 26 sie 2022 o 11:26 juan carlos morales <dev.juan.morales@gmail.com>
napisał(a):

> El vie, 26 ago 2022 a las 11:00, Michał Marcin Brzuchalski
> (<michal.brzuchalski@gmail.com>) escribió:
> >
> > A `json_decode()` is a substitute that IMO solves 99% of use cases.
> > If I'd follow your logic and accept every small addition that handles 1%
> of use cases, somebody will raise another RFC
> > for simplexml_validate_string or yaml_validate and the next
> PhpToken::validate.
> > All above can be valid if we trust that people normally validate 300MB
> payloads to do nothing if they DON'T fail and there is nothing strange
> about that.
> >
> > Cheers,
> >
>
> How can you make such an assertion in those numbers (99% of use cases
> and son on, that you mention) ? Can you give us more information about
> this assertions?
>
> I have provide real examples where the need to validate-only a
> json-string is actually needed, also the need from our developers
> community asking for this.
>

Your examples are a couple of functions.
Assuming that they're heavily used is as true as my assumptions.

Cheers,

juan carlos morales

4 years ago

El vie, 26 ago 2022 a las 11:35, Michał Marcin Brzuchalski
(<michal.brzuchalski@gmail.com>) escribió:

> Your examples are a couple of functions.
> Assuming that they're heavily used is as true as my assumptions.
>
> Cheers,

Is good you clarify that your numbers were assumptions.

By the way, I never said anything about how my examples were use in
real life, I just provided them as examples of a "need"/requirement to
do something.

Thanks for participating.

Cheers.

RFC: https://wiki.php.net/rfc/json_validate
Implementation: https://github.com/php/php-src/pull/9399

Andreas Leathley

4 years ago

On 26.08.22 11:00, Michał Marcin Brzuchalski wrote:

> A `json_decode()` is a substitute that IMO solves 99% of use cases.
> If I'd follow your logic and accept every small addition that handles 1% of
> use cases, somebody will raise another RFC
> for simplexml_validate_string or yaml_validate and the next
> PhpToken::validate.
> All above can be valid if we trust that people normally validate 300MB
> payloads to do nothing if they DON'T fail and there is nothing strange
> about that.

There is already a way to validate XML in PHP, and Yaml or PHP is
something within the control of a PHP programmer, while JSON is mostly
used as a format for communication in APIs, so you never know what you
get. If with a new function it becomes much easier to defend against a
Denial-of-Service attack for some parts of a JSON API, then this can be
a good addition just for security reasons.

But this reason, which most resonates with me, is currently missing in
the RFC, so I would suggest to add that fast / efficient validation of a
common communication format reduces the attack surface for
Denial-of-Service attacks.

juan carlos morales

4 years ago

El vie, 26 ago 2022 a las 11:43, Andreas Leathley
(<a.leathley@gmx.net>) escribió:

>
> On 26.08.22 11:00, Michał Marcin Brzuchalski wrote:
>
> There is already a way to validate XML in PHP, and Yaml or PHP is
> something within the control of a PHP programmer, while JSON is mostly
> used as a format for communication in APIs, so you never know what you
> get. If with a new function it becomes much easier to defend against a
> Denial-of-Service attack for some parts of a JSON API, then this can be
> a good addition just for security reasons.
>
> But this reason, which most resonates with me, is currently missing in
> the RFC, so I would suggest to add that fast / efficient validation of a
> common communication format reduces the attack surface for
> Denial-of-Service attacks.
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: https://www.php.net/unsub.php
>

For sure I will add this. Thanks a lot !!!!!! That is exactly why we
are having this discussion. Once again, Thanks!

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

Implementation: https://github.com/php/php-src/pull/9399

Tim Düsterhus

4 years ago

Hi

On 8/26/22 11:00, Michał Marcin Brzuchalski wrote:

> A `json_decode()` is a substitute that IMO solves 99% of use cases.
> If I'd follow your logic and accept every small addition that handles 1% of
> use cases, somebody will raise another RFC
> for simplexml_validate_string or yaml_validate and the next
> PhpToken::validate.
> All above can be valid if we trust that people normally validate 300MB
> payloads to do nothing if they DON'T fail and there is nothing strange
> about that.

You phrase this as if this was a bad thing.

But yes, if PHP would include a YAML parser in core (there appears to be
one in PECL) then having a yaml_validate() function with a value-add
compared to actually constructing the object representation then it
would certainly be an appropriate inclusion. I don't see why JSON should
be special here.

For XML my understanding is that the XMLReader class can already be used
to efficiently validate an XML input in userland [1], because that class
is defined to be a streaming parser. These types of parsers are not
uncommon in XML land.

Best regards
Tim Düsterhus

[1] https://www.php.net/manual/en/xmlreader.isvalid.php

Larry Garfield

4 years ago

On Fri, Aug 26, 2022, at 4:00 AM, Michał Marcin Brzuchalski wrote:

> A `json_decode()` is a substitute that IMO solves 99% of use cases.
> If I'd follow your logic and accept every small addition that handles 1% of
> use cases, somebody will raise another RFC
> for simplexml_validate_string or yaml_validate and the next
> PhpToken::validate.

The heuristic I use is that an API should be "reasonably complete" in one location.  Having a half-assed API in C and the rest left to inconsistent and redundant user-space implementations is a terrible API; the same would apply for a user-space library that is half-assed and leaves the rest to "someone else to write."  Naturally "reasonably complete" is a somewhat squishy term, which is why it's a heuristic.

By that metric, yes, str_starts_with() and friends absolutely belonged in core, because we already have a bunch of string functions and str_starts_with() is by a wide margin the most common usage of strpos().

By the same token, yes, json_validate() makes sense to include in the main API, which means in C.  If there's a performance benefit to doing so as well, that makes it an easy sell for me.

simplexml_validate_string: I could see the argument there as well, quite frankly.  Unless there is already an equivalently capable option in core, which in this case it sounds like there is.

yaml_validate: There is no YAML parser that ships with PHP, so no, this wouldn't make sense.  If at some point in the future PHP added a C-based YAML extension to the standard distribution, then including a validate function in that would make sense.  (Whether we should include a YAML extension in the standard distribution is an entirely separate question.)

PHPToken::validate: Again, PHPToken is in core, so why wouldn't we also include a validate for it, especially if it's easy to do and logically belongs in the same "surface area"?

Really, the whole "functions don't belong in C" argument is tired.  That ship sailed decades ago.  PHP has a mixed-implementation life, whether we like it or not.  Some APIs and standard lib components are in C, deal.  For those that are or make sense to be, they should be robust and self-contained and complete.  For those that aren't, they should be kept out as a set, or added "as a set."  That's the granularity where we should be talking, not the function level.

I'm in favor of this RFC.  However, I do agree that it should never throw, period.  Only return a boolean.  I'm not sure if JSON_INVALID_UTF8_IGNORE makes sense either, frankly.  That should get corrected before it goes to a vote.

--Larry Garfield

Kamil Tekiela

4 years ago

What is the reasoning behind the name? I can't find it explained in the
RFC. What about other alternatives like is_json or validate_json?

David Gebler

4 years ago

On Fri, Aug 26, 2022 at 6:29 PM Kamil Tekiela <tekiela246@gmail.com> wrote:

> What is the reasoning behind the name? I can't find it explained in the
> RFC. What about other alternatives like is_json or validate_json?
>

The name json_validate makes most sense to me; it groups itself together
nicely with the other json_* API functions. I think is_json is what was
originally proposed but besides being then inconsistent with the rest of
the JSON API function naming, consider most of the is_* functions are for
checking types (and some for file properties), not validation.

But on the function, the other question which remains for me is whether
returning boolean is the right thing to do at all. It seems obviously
intuitive it should, returning true for valid and false for invalid JSON
but then if you consider you're still going to be in the situation of
calling json_last_error() if you want to know why invalid JSON was invalid
and in particular you might not expect the "last error" to have changed
just from an attempt to check a string. How can there be an error when by
definition you weren't trying to do anything except check the validity of
some unknown data? Not sure what the answer is there...curious what other
people's views are on that. I don't think throwing an exception on invalid
JSON is the right answer in any case.

Ayesh - PHP.Watch

4 years ago

> whether returning boolean is the right thing to do at all. It seems obviously
> intuitive it should, returning true for valid and false for invalid JSON
> but then if you consider you're still going to be in the situation of
> calling json_last_error() if you want to know why invalid JSON was invalid
> and in particular you might not expect the "last error" to have changed
> just from an attempt to check a string. How can there be an error when by
> definition you weren't trying to do anything except check the validity of
> some unknown data? Not sure what the answer is there...curious what other
> people's views are on that. I don't think throwing an exception on invalid
> JSON is the right answer in any case.

One of the reasons why I like `json_validate` over `is_json` is that
the former does not imply a boolean return value as strongly as the
latter.

I think we should not modify any state with the `json_validate`
function, and I'd be happy with the function returning 0 for a valid
JSON, or the error code as an integer (`JSON_ERROR_STATE_MISMATCH`,
`JSON_ERROR_SYNTAX`, etc.). `JSON_ERROR_NONE` constant is already
assigned 0, so they align quite well too.

As for exceptions, I too think throwing an exception here is an
anti-pattern. It makes perfect sense to throw on `json_decode` if the
program doesn't want to deal with invalid JSON gracefully. But calling
`json_validate` is an intended call, and returning an error is
excepted, and not an exception.

Larry Garfield

4 years ago

On Fri, Aug 26, 2022, at 1:41 PM, Ayesh Karunaratne wrote:

>> whether returning boolean is the right thing to do at all. It seems obviously
>> intuitive it should, returning true for valid and false for invalid JSON
>> but then if you consider you're still going to be in the situation of
>> calling json_last_error() if you want to know why invalid JSON was invalid
>> and in particular you might not expect the "last error" to have changed
>> just from an attempt to check a string. How can there be an error when by
>> definition you weren't trying to do anything except check the validity of
>> some unknown data? Not sure what the answer is there...curious what other
>> people's views are on that. I don't think throwing an exception on invalid
>> JSON is the right answer in any case.
>
> One of the reasons why I like `json_validate` over `is_json` is that
> the former does not imply a boolean return value as strongly as the
> latter.
>
> I think we should not modify any state with the `json_validate`
> function, and I'd be happy with the function returning 0 for a valid
> JSON, or the error code as an integer (`JSON_ERROR_STATE_MISMATCH`,
> `JSON_ERROR_SYNTAX`, etc.). `JSON_ERROR_NONE` constant is already
> assigned 0, so they align quite well too.

The problem here is that you're using a falsy return (0) to indicate "it validated", which is logically true.  PHP's type juggling makes that a very bad idea.

I somehow doubt it will get much traction, but this is the sort of case where a "naked either" approach could work well: https://peakd.com/hive-168588/@crell/much-ado-about-null

Though there's also the risk then of all objects being "truthy", since there seems to be no appetite for a __toBool() method as that's been shot down before.  At some point, we need to realize that our current design philosophy boxes us in on too many sides and something is going to have to change.

I see the argument for json_last_error() not being a great fit, but given the limitations we've chosen to have in the language it seems like "return bool and use json_last_error()" is the least-bad option.

--Larry Garfield

Derick Rethans

4 years ago

On 26 August 2022 18:28:59 BST, Kamil Tekiela <tekiela246@gmail.com> wrote:

>What is the reasoning behind the name? I can't find it explained in the
>RFC. What about other alternatives like is_json or validate_json?

It's part of the json extension so it should start with json_ following naming guidelines for functions in extensions.

cheers
Derick

juan carlos morales

4 years ago

El vie, 26 ago 2022 a las 6:47, Michał Marcin Brzuchalski
(<michal.brzuchalski@gmail.com>) escribió:

>
>
> I share the same opinion you expressed here even though you admit in recent email that you changed your mind.
>
> In recent versions we tend to accept more and more small standard library functions with IMO questionable argumentation. The same goes here and I'm not convinced we should introduce next small function that can be simply implemented in user land.

Sorry but I dont think that a JSON parser with memory usage zero (or
maybe a few bytes) can be simply done in the userland. This function
(json_validate) is small by itself, but gives you access to the JSON
parser. Can you please provide an example of what you commented?

> Any example testing > 3MB JSON string are for me edge cases that normally don't happen often to deserve special treatment.

I don't agree with your definition of "edge case" here, as edge cases
depend/belong on/to the system under analysis.

By the way, the test case provided in the PR , "test 005" uses a
json-string of about 3 MB (maybe 3.1) , and in order to decode it
json_decode() needs something around 109 MB of memory. For me,
validation can be done in a better and efficient way, like it has been
probe with this proposal.

> If we keep the tendency to pollute already bloated standard library with an army of small functions that could have not exists and be replaced with normal PHP counterparts IMHO we'll end with frustration from developers as I believe DX slowly falls down here.

Last but not least, I want to say that the function json_validate() is
small, easy to maintain and extend if needed, and at the same gives us
access to something that is not trivial to write in userland .... the
existing JSON parser.


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

Implementation: https://github.com/php/php-src/pull/9399

Rowan Collins

4 years ago

On 25/08/2022 17:02, juan carlos morales wrote:

> I am glad to present to you the RFC for json_validate() function.
>
> RFC: https://wiki.php.net/rfc/json_validate

Hi,

Leaving aside the overall question of inclusion and looking at details,
I'm not convinced JSON_THROW_ON_ERROR belongs here.

With json_decode, throwing an exception is a useful feature because a)
there is no natural value to signal an error, false and null both being
valid results; and b) most code is written expecting the input to be
valid, so failure is an "exceptional state" that needs to break out of
normal flow.

With json_validate, neither reason applies: there is no ambiguity in
what false means, and no reason to write code that calls json_validate
but doesn't check that return value.


On a different note, I notice that several of your examples are not
actually checking the validity of JSON at all, they are parsing it in
order to re-serialize it - the Magento getJSONString, the Symfony
getPrettyJson, and the humhub actionIndex functions are all directly
using the output of json_decode, not just reporting whether it succeeded.

There's still plenty of other examples that make your point, but those
three should probably be removed.


Regards,

--
Rowan Tommins
[IMSoP]

juan carlos morales

4 years ago

Hello Rowan thanks for participating.

My answer for your comments would be:

=== Regarding JSON_THROW_ON_ERROR ===
I think is a valid point from your side, and I am open to change the
implementation if the maintainers believe so. I leave it there as it
existed in json_decode() and wanted to give the developer the option to use
it or not. But as I said, you hae a valid point, that I also like.

=== Regarding the examples you mentioned ===
The one from humhub and Symfony http-kernel, yes you are right, I will take
them out from the RFC right now. Nice catch.

But the Magento one, I took a look again, and I still believe is right
example, because the only reason is using json_decode() for, is to validate
only. Am I missing something here?

Regards.

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

Implementation: https://github.com/php/php-src/pull/9399

Peter Bowyer

4 years ago

Hi Juan,

On Thu, 25 Aug 2022 at 17:02, juan carlos morales <
dev.juan.morales@gmail.com> wrote:

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

Thanks for bringing forward this RFC. I am in favour of this change, as you
can't efficiently validate JSON in userland.

Like Rowan I'm not convinced JSON_THROW_ON_ERROR belongs here. I can't
think of a case where more than a boolean response is needed.

Peter

Paul

4 years ago

On 8/26/2022 1:28 AM, Peter Bowyer wrote:

> Hi Juan,
>
> On Thu, 25 Aug 2022 at 17:02, juan carlos morales <
> dev.juan.morales@gmail.com> wrote:
>
>> RFC: https://wiki.php.net/rfc/json_validate
>
>
> Thanks for bringing forward this RFC. I am in favour of this change, as you
> can't efficiently validate JSON in userland.
>
> Like Rowan I'm not convinced JSON_THROW_ON_ERROR belongs here. I can't
> think of a case where more than a boolean response is needed.
>
> Peter
>

I'm for this as well. Though something more than a boolean is useful to
indicate when the max depth has been exceeded as that's not strictly a
problem with the json's validity.

juan carlos morales

4 years ago

El vie, 26 ago 2022 a las 10:28, Peter Bowyer
(<phpmailinglists@gmail.com>) escribió:

>
> Hi Juan,
>
> On Thu, 25 Aug 2022 at 17:02, juan carlos morales <dev.juan.morales@gmail.com> wrote:
>>
>> RFC: https://wiki.php.net/rfc/json_validate
>
>
> Thanks for bringing forward this RFC. I am in favour of this change, as you can't efficiently validate JSON in userland.
>
> Like Rowan I'm not convinced JSON_THROW_ON_ERROR belongs here. I can't think of a case where more than a boolean response is needed.
>
> Peter

Seems that will be a good idea to take out the usage of the flag
JSON_THROW_ON_ERROR after all. That was a nice and objective critic
and I appreciate it. Thanks! I will take it out of the implementation.

Regards

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

Implementation: https://github.com/php/php-src/pull/9399

Tim Düsterhus

4 years ago

Hi

On 8/26/22 10:56, Paul Crovella wrote:

> I'm for this as well. Though something more than a boolean is useful to
> indicate when the max depth has been exceeded as that's not strictly a
> problem with the json's validity.
>

Yes, it's not strictly invalid JSON, but I expect json_validate() to be
used as a check to mean "will json_decode() be able to decode the JSON
with these parameters". Then `false` is the logical result when
exceeding the depth.

Also json_last_error() will do what you expect it to do:
https://github.com/php/php-src/pull/9399/files#diff-1cdb6c7eb68aa60b323b056a06d6f7427b38e9d8d967ce97992129ff65c7951eR55.
So you can find out the reason why the JSON is invalid.

Best regards
Tim Düsterhus

Jeffrey Dafoe

4 years ago

> -----Original Message-----
> From: juan carlos morales <dev.juan.morales@gmail.com>
> Sent: Thursday, August 25, 2022 12:02 PM
> To: PHP Internals List <internals@lists.php.net>
> Subject: [PHP-DEV] RFC json_validate() - status: Under Discussion
>
> Hello.
>
> I am glad to present to you the RFC for json_validate() function.
>
> The code/implementation still needs some workout, but seems to be fine
> enough to be presented to you all.
>
> I look forward for feedback
>
> Thanks in advance.
>
> Juan.
>
> RFC: https://wiki.php.net/rfc/json_validate
>
> Implementation: https://github.com/php/php-src/pull/9399

Not a voting member, but I'd like to voice support for this. JSON is widely used, core already supports this functionality, the scope of change is minimal, and the feature is useful.

-Jeff

Tim Düsterhus

3 years ago

Hi

On 8/25/22 18:02, juan carlos morales wrote:

> I am glad to present to you the RFC for json_validate() function.

It appears the discussion came to a halt. How do you plan to proceed
with your RFC?

Best regards
Tim Düsterhus

juan carlos morales

3 years ago

> It appears the discussion came to a halt. How do you plan to proceed
> with your RFC?

Thanks for the heads up email Tim.

People, following the procedure, this is a "heads up" email to all of
you regarding this RFC. If no significant issue/complain raises, then
I will move the RFC for voting by Wednesday 21 of September.

I also want to thank your support in the github pull request, the
hearts and thums up emojis were awesome
(https://github.com/php/php-src/pull/9399)

Regards

Juan.

Tim Düsterhus

3 years ago

Hi

On 9/19/22 23:26, juan carlos morales wrote:

>> It appears the discussion came to a halt. How do you plan to proceed
>> with your RFC?
>
> Thanks for the heads up email Tim.
>
> People, following the procedure, this is a "heads up" email to all of
> you regarding this RFC. If no significant issue/complain raises, then

The RFC still contains a non-empty "Open Issues" section. This needs to
be resolved before the vote starts.

I would also recommend inserting a closed voting widget (or multiple, if
you want to have additional votes for the details), so that it's clear
for everyone how exactly the vote will look like.

Best regards
Tim Düsterhus

juan carlos morales

3 years ago

> The RFC still contains a non-empty "Open Issues" section. This needs to
> be resolved before the vote starts.
>
> I would also recommend inserting a closed voting widget (or multiple, if
> you want to have additional votes for the details), so that it's clear
> for everyone how exactly the vote will look like.

Tim, how should I proceed with the Open Issues/Questions? is only
about the return value of the function.

Should I create a poll for that? Should I leave it like this and
decide during Voting phase? This question raised during the mailing
discussion and never moved forward, can you advice please? Or I should
delete the Open Issues/question and move forward with the current
approach I choosed?

A lot of days passed by already, and I would like to move on with the
RFC process.

Thanks in advance.

Tim Düsterhus

3 years ago

Hi

On 9/20/22 00:15, juan carlos morales wrote:

>> The RFC still contains a non-empty "Open Issues" section. This needs to
>> be resolved before the vote starts.
>>
>> I would also recommend inserting a closed voting widget (or multiple, if
>> you want to have additional votes for the details), so that it's clear
>> for everyone how exactly the vote will look like.
>
> Tim, how should I proceed with the Open Issues/Questions? is only
> about the return value of the function.
>
> Should I create a poll for that? Should I leave it like this and
> decide during Voting phase? This question raised during the mailing
> discussion and never moved forward, can you advice please? Or I should
> delete the Open Issues/question and move forward with the current
> approach I choosed?

It's your RFC, you do whatever you are comfortable with. If you feel
that a vote is the best option, then add a vote. If you as the author
want to decide what's "best", then that's also fine.

It just needs to be clear what the function will look and feel like
before the vote starts - so leaving it as it is and deciding yourself
*during* voting is not acceptable.

Best regards
Tim Düsterhus

juan carlos morales

3 years ago

The RFC is in voting phase now.

I want to thank everyone involved in the discussions related to this
RFC, the ones who reviewed the code in advance, and the ones that with
some emojis in github showed their interest and support.

James Gilliland

3 years ago

Don't think it makes a difference to the discussion at this point but just
thought I'd point out there's another example missing from the RFC list
that almost everyone probably has installed with their projects.

https://github.com/sebastianbergmann/phpunit/blob/main/src/Framework/Constraint/String/IsJson.php

On Wed, Sep 21, 2022 at 11:39 PM juan carlos morales <
dev.juan.morales@gmail.com> wrote:

juan carlos morales

3 years ago

The RFC got accepted and merge. Thanks for contributing :)