Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:118342
Date: Tue, 02 Aug 2022 10:08:36 -0700 (PDT)
To: <internals@lists.php.net>
Message-ID: <C622EC5A-F2DA-4638-B93A-46A8D6407F60@automattic.com>
MIME-Version: 1.0
Content-Type: multipart/alternative;
	boundary="=_2AABE5D5-7299-4DFC-8805-96640C4AFE52"
Subject: Re: RFC Idea - is_json - looking for feedback
From: dennis.snell@automattic.com (Dennis Snell)

--=_2AABE5D5-7299-4DFC-8805-96640C4AFE52
Content-Type: text/plain;
	charset=utf-8
Content-Transfer-Encoding: quoted-printable

Still new here so what I say probably doesn't amount to much, but I too see=
 this as encouraging the generally-less-appropriate sequence of: parse to d=
etermine if it's valid JSON via `is_json()`; and then parse again using `js=
on_decode()`


The name `is_valid_json()` rings truer to me because we are dealing with va=
lues that may or may not be "JSON" even if they can parse as such. For exam=
ple, the string `"false"` is valid JSON _if interpreted as JSON_ but need n=
ot _be_ JSON in truth. The same is true for `"[1, 2, 3]"` and even still I =
suppose it's possible to have a string value that works as a JSON map _if i=
nterpreted as JSON_ but that data may not have been created with the purpos=
e of being JSON. Again, it's _valid JSON_ if interpreted that way, but may =
not in fact _be_ JSON.


That being said, the idea of putting this behind `filter_var()` seems like =
it would do less to encourage poor practice (as in, it would be a good thin=
g). That one is likely harder to find, doesn't sound as appealing as `is_js=
on()` or `is_valid_json()`, and would stick out more if someone were to use=
 it casually where `json_decode()` alone is more appropriate. Also that fun=
ction already deals with validation and provides a way to pass the options =
we would need (max depth, JSON_INVALID_UTF8_SUBSTITUTE, and=C2=A0JSON_INVAL=
ID_UTF8_IGNORE) if we wanted this to match the behavior of `json_decode()`.


The examples you found are good motivations for a leaner validation check, =
but I wonder if they actually represent a great need. If a good user-space =
solution exists those few frameworks could use that effectively and presuma=
bly the need for a core function would disappear. I think personally I woul=
d rather see this go through if our performance improved substantially more=
 than it shows in the posted benchmarks. While I realize what you are doing=
 so far is reusing the existing JSON parser and so we shouldn't expect an e=
ntirely different performance profile, I agree with the others that the imp=
lementation here is somewhat intrinsically bound with the idea itself.


In comparison I have created a naive version in PHP itself with PCRE calls =
advancing the JSON tokenization:=C2=A0https://pastebin.com/Cf8BZn1H
There are certainly glaring bugs in this because I tossed it together while=
 on a train and only wanted to get a reasonable approximation of the perfor=
mance characteristics for in-PHP code. In your benchmark it runs about twic=
e as slow as `json_decode()` but uses essentially no memory (matching your =
results with `is_json()`). I'm pretty sure there are easy ways to eliminate=
 some low-hanging performance bottlenecks, though it doesn't check for vali=
d UTF8 or valid escaped strings or check for a maximum depth. I feel like i=
f we are going to add a new native function for this it should be way faste=
r than four times as fast as a primitive function in PHP user-space.


Warmly,
Dennis Snell





--=_2AABE5D5-7299-4DFC-8805-96640C4AFE52--