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

checker, json: return error on invalid string input instead of empty string result #21184

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ttytm
Copy link
Member

@ttytm ttytm commented Apr 4, 2024

With the PR we get errors for invalid Cjson input.

Currently, there are many occasions where decoding panics without an error.
Or the main concern in the related issue, is an empty result return without an any error in case of using quotation marks when they shouldn't be used.

Atm:

import json

struct TestStruct {
	host string
}

json.decode(TestStruct, "abc")! // Panics, but without an error value

//quote ⬇️ should not be there
txt := '"{"host": "localhost"}'
json.decode(TestStruct, txt)! // Currently an empty string

New errors:

error: json.decode: invalid json string
    5 | }
    6 | 
    7 | txt := '"{"host": "localhost"}'
      |        ~~~~~~~~~~~~~~~~~~~~~~~~
    8 | json.decode(TestStruct, txt)!
error: json.decode: invalid json string
    5 | }
    6 | 
    7 | json.decode(TestStruct, 'abc')!
      |                         ~~~~~
    8 | 

The change is simple but should hopefully do most validation needed for the module in regards to the nice progress that is being made on the json2 module.

Resolves #19801

@ttytm ttytm marked this pull request as draft April 4, 2024 13:18
@spytheman
Copy link
Member

spytheman commented Apr 4, 2024

Imho the error should be a runtime one (done in the json module preferably), since the passed text will be only seldom a hardcoded json string (it is done in the issue just for as an illustration/brevity).

@enghitalo enghitalo added the Fixed in x.json2 A JSON related issue, that is fixed in `x.json2` module. label Nov 3, 2024
@JalonSolov JalonSolov added the Needs Rebase The code of the PR must be rebased over current master before it can be approved. label Dec 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fixed in x.json2 A JSON related issue, that is fixed in `x.json2` module. Needs Rebase The code of the PR must be rebased over current master before it can be approved.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

json [raw] no error message given when fails
4 participants