View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0037837 | FPC | FCL | public | 2020-09-29 14:17 | 2020-11-08 17:42 |
| Reporter | Benito van der Zander | Assigned To | Michael Van Canneyt | ||
| Priority | normal | Severity | minor | Reproducibility | have not tried |
| Status | resolved | Resolution | fixed | ||
| Platform | amd64 | OS | linux | ||
| Product Version | 3.3.1 | ||||
| Fixed in Version | 3.3.1 | ||||
| Summary | 0037837: json parser allows garbage after single values | ||||
| Description | DoParse stops after parsing the first value, so afterwards anything can be in the string: writeln(GetJSON('"abc"xxxxxxx').AsString); writeln(GetJSON('1234...xxxxxxx').AsFloat:4:4); writeln(GetJSON('[1,2,3]xxxxxxx').Items[0].AsFloat:4:4); | ||||
| Tags | No tags attached. | ||||
| Fixed in Revision | 47342 | ||||
| FPCOldBugId | |||||
| FPCTarget | 3.2.2 | ||||
| Attached Files |
| ||||
|
|
I'm not sure I consider this a bug. A pascal program can also contain garbage after the final end. I will think about it. |
|
|
I am trying to make it pass all these tests: https://github.com/nst/JSONTestSuite (or rather another test suite that includes all those json tests as well) They do not allow this (I do not care. I do not even use the parser, only the scanner) Another failing case is a garbage 0, i.e. '123'#0 where the scanner thinks the number ends at #0 It could check it with the joStrict option Or, return a pchar, so we know where it stopped parsing. Then it could also take a pchar as input, i.e. parse pchar instead of string/stream, and then continue parsing from there, so you could parse '123 456 789' without array and without copying the string . |
|
|
No, no pchar. Agreed on the check with joStrict. This should be as simple as adding a if joStrict in Options then begin Repeat GetNextToken ; Until CurrentToken<>tkWhiteSpace; If CurrentToken<>tkEOF then Raise Exception.CreateFmt('Expected EOF, but got %s',[CurrentTokenString]); end; after the code in TBaseJSONReader.DoExecute |
|
|
> No, no pchar. pchars can be very useful, I have had always plans to make a pchar only version of the scanner. pchars in and pchars out, and zero allocations. That has a good chance to be twice as fast (and decoding \u... would be moved elsewhere) > Until CurrentToken<>tkWhiteSpace; > If CurrentToken<>tkEOF then > Raise Exception.CreateFmt('Expected EOF, but got %s',[CurrentTokenString]); That is basically what I have in my parser It is passing all these garbage tests, except for '123'#0 |
|
|
You don't need to convince me of the usefulness of pchar :-) But in this case I don't want pchar because at some point the code must run in the browser, which does not know pchar. |
|
|
> But in this case I don't want pchar because at some point the code must run in the browser, which does not know pchar. No pchar? But the json scanner uses pchar everywhere in fetchtoken But it could also be an offset in the string. Return parsed x chars, or take a start at char y... Why must it run in the browser anyways? All the browsers can already parse json themselves |
|
|
It's one thing to use pchar internally, it's another to build the API around it. The pas2js parser also runs in the browser, this was possible because the pchar usage is limited. The browser can indeed parse JSON, but that is one-shot. (that is how it works currently) By allowing the scanner to run in the browser the other classes (reader, evented reader) and options that the browser does not offer can also be reused. |
|
|
Fixed as proposed. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2020-09-29 14:17 | Benito van der Zander | New Issue | |
| 2020-09-29 15:40 | Michael Van Canneyt | Note Added: 0125957 | |
| 2020-09-29 15:40 | Michael Van Canneyt | Assigned To | => Michael Van Canneyt |
| 2020-09-29 15:40 | Michael Van Canneyt | Status | new => assigned |
| 2020-09-29 17:36 | Benito van der Zander | Note Added: 0125963 | |
| 2020-09-29 17:56 | Michael Van Canneyt | Note Added: 0125965 | |
| 2020-09-29 18:24 | Benito van der Zander | Note Added: 0125966 | |
| 2020-09-29 19:13 | Michael Van Canneyt | Note Added: 0125968 | |
| 2020-10-12 23:36 | Benito van der Zander | Note Added: 0126274 | |
| 2020-10-13 08:58 | Michael Van Canneyt | Note Added: 0126275 | |
| 2020-11-08 17:42 | Michael Van Canneyt | Status | assigned => resolved |
| 2020-11-08 17:42 | Michael Van Canneyt | Resolution | open => fixed |
| 2020-11-08 17:42 | Michael Van Canneyt | Fixed in Version | => 3.3.1 |
| 2020-11-08 17:42 | Michael Van Canneyt | Fixed in Revision | => 47342 |
| 2020-11-08 17:42 | Michael Van Canneyt | FPCTarget | => 3.2.2 |
| 2020-11-08 17:42 | Michael Van Canneyt | Note Added: 0126795 |
