Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:118473
Content-Type: multipart/mixed; boundary="------------F8QhGn0Bo47s5idxFVM0OVBU"
Message-ID: <59c258e7-9410-8572-b904-e8421aae9867@bastelstu.be>
Date: Thu, 25 Aug 2022 22:22:02 +0200
MIME-Version: 1.0
Content-Language: en-US
To: David Gebler <davidgebler@gmail.com>,
 juan carlos morales <dev.juan.morales@gmail.com>
Cc: PHP Internals List <internals@lists.php.net>
References: <CANMwmbN8_Km+urXbNO7oWvNteYA2mTPNYyRgfaoypEK1Cw4F0A@mail.gmail.com>
 <CA+p1FaZDm3-h_LoOwv619wFyPg+2pAM8qC2La438sU_SPHZnCg@mail.gmail.com>
In-Reply-To: <CA+p1FaZDm3-h_LoOwv619wFyPg+2pAM8qC2La438sU_SPHZnCg@mail.gmail.com>
Subject: Re: [PHP-DEV] RFC json_validate() - status: Under Discussion
From: tim@bastelstu.be (=?UTF-8?Q?Tim_D=c3=bcsterhus?=)

--------------F8QhGn0Bo47s5idxFVM0OVBU
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit

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
--------------F8QhGn0Bo47s5idxFVM0OVBU
Content-Type: application/x-php; name="json-test.php"
Content-Disposition: attachment; filename="json-test.php"
Content-Transfer-Encoding: base64

PD9waHAKCiRzdHIgPSAnZXhlcmNpc2UgbGVmdCBmb3IgdGhlIHJlYWRlcic7CnZhcl9kdW1w
KHN0cmxlbigkc3RyKSk7Cgp2YXJfZHVtcChtZW1vcnlfZ2V0X3BlYWtfdXNhZ2UoKSAvIDEw
MjQgLyAxMDI0KTsKCnRyeSB7Cgl2YXJfZHVtcChqc29uX2RlY29kZSgkc3RyLCBmbGFnczog
SlNPTl9USFJPV19PTl9FUlJPUikpOwp9IGNhdGNoIChcRXhjZXB0aW9uICRlKSB7CgllY2hv
ICRlLT5nZXRNZXNzYWdlKCksIFBIUF9FT0w7Cn0KCnZhcl9kdW1wKG1lbW9yeV9nZXRfcGVh
a191c2FnZSgpIC8gMTAyNCAvIDEwMjQpOwo=

--------------F8QhGn0Bo47s5idxFVM0OVBU--