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

std.json.parser does not support \uxxxx escape #2528

Closed
donpdonp opened this issue May 20, 2019 · 2 comments
Closed

std.json.parser does not support \uxxxx escape #2528

donpdonp opened this issue May 20, 2019 · 2 comments
Labels
contributor friendly This issue is limited in scope and/or knowledge of Zig internals. standard library This issue involves writing Zig code for the standard library.
Milestone

Comments

@donpdonp
Copy link

The streaming parser supports the "\u0040" => "@" escaping, but the non-streaming parser does not.

quick sanity check with jq util

$ jq .
"View from 15th Floor\u0040"
"View from 15th Floor@"

quick test patch

--- a/std/json.zig
+++ b/std/json.zig
@@ -1358,7 +1358,7 @@ test "json.parser.dynamic" {
-        \\      "Title":  "View from 15th Floor",
+        \\      "Title":  "View from 15th Floor\u0040",
@@ -1386,7 +1386,7 @@ test "json.parser.dynamic" {
-    testing.expect(mem.eql(u8, title.String, "View from 15th Floor"));
+    testing.expect(mem.eql(u8, title.String, "View from 15th Floor@"));

test fails

/home/donp/code/zig/github/std/json.zig:1389:19: 0x262a5c in test "json.parser.dynamic" (test)
    testing.expect(mem.eql(u8, title.String, "View from 15th Floor@"));

Also line 1352 says test "json.parser.dynamic" { when its testing the non-streaming parser.

@tiehuis
Copy link
Member

tiehuis commented May 21, 2019

The streaming parser is built atop the non-streaming parser. Both verify unicode escape sequences during decoding, however neither currently transforms the escape sequences to their byte representations.

I'm not 100% on if we should handle this transform for the streaming parser since the non-allocating property is nice (and it is also zero-copy). However, this is marked as a TODO in the tree parser. In particular see this function:

zig/std/json.zig

Line 1337 in 163a8e9

fn parseString(p: *Parser, allocator: *Allocator, token: Token, input: []const u8, i: usize) !Value {

We should check the string_has_escape field on the token and if present, re-parse the string and transform the escape sequences.

@tiehuis tiehuis added contributor friendly This issue is limited in scope and/or knowledge of Zig internals. standard library This issue involves writing Zig code for the standard library. labels May 21, 2019
@andrewrk andrewrk added this to the 0.6.0 milestone May 28, 2019
@andrewrk andrewrk modified the milestones: 0.6.0, 0.7.0 Jan 5, 2020
@marnix
Copy link

marnix commented Sep 2, 2020

It looks like this has been implemented already? Applying the provided "quick test patch" keeps things working.

Perhaps this was implemented in #3648?

@andrewrk andrewrk modified the milestones: 0.7.0, 0.8.0 Oct 30, 2020
@andrewrk andrewrk modified the milestones: 0.8.0, 0.9.0 Nov 6, 2020
@andrewrk andrewrk modified the milestones: 0.9.0, 0.10.0 May 19, 2021
@ifreund ifreund closed this as completed Aug 4, 2021
@ifreund ifreund modified the milestones: 0.10.0, 0.7.0 Aug 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributor friendly This issue is limited in scope and/or knowledge of Zig internals. standard library This issue involves writing Zig code for the standard library.
Projects
None yet
Development

No branches or pull requests

5 participants