Skip to content
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.

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

Add json validator #65

Merged
merged 14 commits into from
Apr 12, 2020
Merged

Add json validator #65

merged 14 commits into from
Apr 12, 2020

Conversation

romkatsu
Copy link
Member

Q A
Is bugfix?
New feature? ✔️
Breaks BC?
Tests pass? ✔️
Fixed issues
  • add json validator
  • add trait
  • edit composer.json

@romkatsu romkatsu requested a review from a team April 12, 2020 09:45
src/Rule/Json.php Outdated Show resolved Hide resolved
tests/Rule/JsonTest.php Show resolved Hide resolved
src/HasValidationMessage.php Show resolved Hide resolved
src/Rule/Json.php Outdated Show resolved Hide resolved
@xepozz xepozz requested a review from a team April 12, 2020 10:01
@romkatsu
Copy link
Member Author

Json validator

@romkatsu romkatsu requested a review from a team April 12, 2020 18:53
@samdark samdark added the status:code review The pull request needs review. label Apr 12, 2020
@samdark samdark merged commit 841b5a9 into yiisoft:master Apr 12, 2020
@samdark
Copy link
Member

samdark commented Apr 12, 2020

👍

@armpogart
Copy link
Contributor

Not sure if it really matters but I have a benchmark (if I implemented it correctly) that shows that using regex for validating json is faster, and the bigger the json, the more is the difference. For real world case the difference would not be visible at all, but another disadvantage of using json_decode for validation purposes here is that it will also consume memory by creating all that objects for the decoded data.

See benchmark

@armpogart
Copy link
Contributor

armpogart commented Apr 13, 2020

Also json_decode successfully decodes any number (which you have check for), simple strings and boolean true (which also don't pass is_string check), which is not valid json in most languages. But simple strings still pass your validation, but they are not valid for javascript JSON.parse.

That's due to the fact that PHP implements a superset of JSON specified in RFC 7159 vs older standard that is implemented by javascript and majority: RFC 4267.

Also there is a newer revision of json standard: RFC 8259, which I doubt any language implemented at all.

@samdark
Copy link
Member

samdark commented Apr 13, 2020

@armpogart are you sure it is complete regex that validates JSON?

@armpogart
Copy link
Contributor

armpogart commented Apr 13, 2020

@samdark The regex is from RFC 4267 section 6. I'll open another discussion (issue) with details posted here and references to all the data I got. There are also json validation test cases on json.org which can be used for json validator testing.

Anyways I'll post all the details in an issue reference it here. I think there are some points that need further discussion.

@samdark
Copy link
Member

samdark commented Apr 13, 2020

Alright. Overall benchmarks look good enough to try it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:code review The pull request needs review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants