From 79a01433851219bc03fdba3b7bb6786b48b678d9 Mon Sep 17 00:00:00 2001 From: Martin Jambon Date: Thu, 21 Jul 2022 00:55:24 +0200 Subject: [PATCH 1/6] Fix out-of-bounds error that can be obtained when an atdgen parser parses an unquoted JSON field. TODO: write a unit test. --- lib/read.mll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/read.mll b/lib/read.mll index 9e3cb54f..dd69224c 100644 --- a/lib/read.mll +++ b/lib/read.mll @@ -151,7 +151,7 @@ let map_lexeme f lexbuf = let len = lexbuf.lex_curr_pos - lexbuf.lex_start_pos in - f (Bytes.sub_string lexbuf.lex_buffer lexbuf.lex_start_pos len) lexbuf.lex_start_pos len + f (Bytes.sub_string lexbuf.lex_buffer lexbuf.lex_start_pos len) 0 len type variant_kind = [ `Edgy_bracket | `Square_bracket | `Double_quote ] type tuple_kind = [ `Parenthesis | `Square_bracket ] From 5f029bd1bc3c771bdb087b893b66627f341f4bc7 Mon Sep 17 00:00:00 2001 From: Martin Jambon Date: Thu, 21 Jul 2022 01:21:57 +0200 Subject: [PATCH 2/6] Update changelog --- CHANGES.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index 03593f46..7f264862 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -14,6 +14,9 @@ ### Fixed +- Fix out-of-bounds error occurring when parsing object field names + with atdgen parsers using `map_ident` or `map_lexeme` (@mjambon, #150) + ## 2.0.1 *2022-06-28* From 8e618d84a894243bd6666703891fdfe19cf0953b Mon Sep 17 00:00:00 2001 From: Marek Kubica Date: Fri, 22 Jul 2022 15:36:31 +0200 Subject: [PATCH 3/6] Add tests --- test/fixtures.ml | 4 ++++ test/fixtures.mli | 3 +++ test/test_read.ml | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 53 insertions(+) diff --git a/test/fixtures.ml b/test/fixtures.ml index 74a68254..8ad60876 100644 --- a/test/fixtures.ml +++ b/test/fixtures.ml @@ -20,6 +20,10 @@ let json_string = ^ {|"list":[0,1,2]|} ^ "}" +let unquoted_json = {|{foo: null}|} + +let unquoted_value = `Assoc [("foo", `Null)] + let json_string_newline = json_string ^ "\n" diff --git a/test/fixtures.mli b/test/fixtures.mli index 14af1b0c..a2cf228c 100644 --- a/test/fixtures.mli +++ b/test/fixtures.mli @@ -8,3 +8,6 @@ val json_string : string (** The same JSON string terminated with a newline *) val json_string_newline : string + +val unquoted_json : string +val unquoted_value : Yojson.Safe.t diff --git a/test/test_read.ml b/test/test_read.ml index 8960931d..f4ea25fd 100644 --- a/test/test_read.ml +++ b/test/test_read.ml @@ -12,7 +12,53 @@ let from_file () = Alcotest.(check Testable.yojson) __LOC__ Fixtures.json_value (Yojson.Safe.from_file input_file); Sys.remove input_file +let unquoted_from_string () = + Alcotest.(check Testable.yojson) + __LOC__ + Fixtures.unquoted_value + (Yojson.Safe.from_string Fixtures.unquoted_json) + +let unquoted_from_lexbuf () = + let lexbuf = Lexing.from_string "{foo: null, bar: null}" in + let lexer_state = Yojson.init_lexer () in + + let ident_expected expectation reference start len = + let identifier = String.sub reference start len in + Alcotest.(check string) + (Format.asprintf "Reference %s start %d len %d matches %s" reference start len expectation) + expectation + identifier; + () + in + let skip_over f = + f lexer_state lexbuf + in + let map_ident f = + Yojson.Safe.map_ident lexer_state f lexbuf + in + + skip_over Yojson.Safe.read_lcurl; + map_ident (ident_expected "foo"); + skip_over Yojson.Safe.read_colon; + skip_over Yojson.Safe.read_space; + skip_over Yojson.Safe.read_null; + skip_over Yojson.Safe.read_comma; + skip_over Yojson.Safe.read_space; + map_ident (ident_expected "bar"); + skip_over Yojson.Safe.read_colon; + skip_over Yojson.Safe.read_space; + skip_over Yojson.Safe.read_null; + + (match Yojson.Safe.read_object_end lexbuf with + | _ -> Alcotest.fail "Object end expected but did not happen" + | exception Yojson.End_of_object -> ()); + + (* successfully made it here, pass the test *) + () + let single_json = [ "from_string", `Quick, from_string; "from_file", `Quick, from_file; + "unquoted_from_string", `Quick, unquoted_from_string; + "unquoted_from_lexbuf", `Quick, unquoted_from_lexbuf; ] From 68dbc4a8b1a75634dd392d2cf13502c390942138 Mon Sep 17 00:00:00 2001 From: Marek Kubica Date: Fri, 22 Jul 2022 17:11:03 +0200 Subject: [PATCH 4/6] Also test `map_string` --- test/test_read.ml | 16 +++++++++++----- test/testable.ml | 14 ++++++++++++++ test/testable.mli | 1 + 3 files changed, 26 insertions(+), 5 deletions(-) diff --git a/test/test_read.ml b/test/test_read.ml index f4ea25fd..4f34ec01 100644 --- a/test/test_read.ml +++ b/test/test_read.ml @@ -19,13 +19,13 @@ let unquoted_from_string () = (Yojson.Safe.from_string Fixtures.unquoted_json) let unquoted_from_lexbuf () = - let lexbuf = Lexing.from_string "{foo: null, bar: null}" in + let lexbuf = Lexing.from_string {|{foo: null, bar: "hello"}|} in let lexer_state = Yojson.init_lexer () in let ident_expected expectation reference start len = let identifier = String.sub reference start len in Alcotest.(check string) - (Format.asprintf "Reference %s start %d len %d matches %s" reference start len expectation) + (Format.asprintf "Reference '%s' start %d len %d matches '%s'" reference start len expectation) expectation identifier; () @@ -33,9 +33,11 @@ let unquoted_from_lexbuf () = let skip_over f = f lexer_state lexbuf in - let map_ident f = - Yojson.Safe.map_ident lexer_state f lexbuf + let map_f mapper f = + mapper lexer_state f lexbuf in + let map_ident = map_f Yojson.Safe.map_ident in + let map_string = map_f Yojson.Safe.map_string in skip_over Yojson.Safe.read_lcurl; map_ident (ident_expected "foo"); @@ -47,7 +49,11 @@ let unquoted_from_lexbuf () = map_ident (ident_expected "bar"); skip_over Yojson.Safe.read_colon; skip_over Yojson.Safe.read_space; - skip_over Yojson.Safe.read_null; + + let variant = skip_over Yojson.Safe.start_any_variant in + Alcotest.(check Testable.variant_kind) "String starts with double quote" `Double_quote variant; + + map_string (ident_expected "hello"); (match Yojson.Safe.read_object_end lexbuf with | _ -> Alcotest.fail "Object end expected but did not happen" diff --git a/test/testable.ml b/test/testable.ml index 58dbac08..3a722154 100644 --- a/test/testable.ml +++ b/test/testable.ml @@ -1 +1,15 @@ let yojson = Alcotest.testable Yojson.Safe.pp Yojson.Safe.equal + +let variant_kind_pp fmt = function + | `Edgy_bracket -> Format.fprintf fmt "`Edgy_bracket" + | `Square_bracket -> Format.fprintf fmt "`Square_bracket" + | `Double_quote -> Format.fprintf fmt "`Double_quote" + +let variant_kind_equal a b = + match a, b with + | `Edgy_bracket, `Edgy_bracket -> true + | `Square_bracket, `Square_bracket -> true + | `Double_quote, `Double_quote -> true + | _ -> false + +let variant_kind = Alcotest.testable variant_kind_pp variant_kind_equal diff --git a/test/testable.mli b/test/testable.mli index 364708c2..ec9dc127 100644 --- a/test/testable.mli +++ b/test/testable.mli @@ -1 +1,2 @@ val yojson : Yojson.Safe.t Alcotest.testable +val variant_kind : Yojson.Safe.variant_kind Alcotest.testable From 3a0865dde376fce285e22fc16c1fcdb94d622051 Mon Sep 17 00:00:00 2001 From: Marek Kubica Date: Fri, 22 Jul 2022 17:31:12 +0200 Subject: [PATCH 5/6] Expect exception with Alcotest --- test/test_read.ml | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/test/test_read.ml b/test/test_read.ml index 4f34ec01..4626fcd8 100644 --- a/test/test_read.ml +++ b/test/test_read.ml @@ -55,12 +55,10 @@ let unquoted_from_lexbuf () = map_string (ident_expected "hello"); - (match Yojson.Safe.read_object_end lexbuf with - | _ -> Alcotest.fail "Object end expected but did not happen" - | exception Yojson.End_of_object -> ()); - - (* successfully made it here, pass the test *) - () + Alcotest.check_raises + "Reading } raises End_of_object" + Yojson.End_of_object + (fun () -> Yojson.Safe.read_object_end lexbuf) let single_json = [ "from_string", `Quick, from_string; From db9d0ad81ee289708d95855a2912abbd156b1caa Mon Sep 17 00:00:00 2001 From: Marek Kubica Date: Mon, 25 Jul 2022 14:55:56 +0200 Subject: [PATCH 6/6] Apply suggestions from the review --- test/test_read.ml | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/test/test_read.ml b/test/test_read.ml index 4626fcd8..83c341cc 100644 --- a/test/test_read.ml +++ b/test/test_read.ml @@ -18,8 +18,8 @@ let unquoted_from_string () = Fixtures.unquoted_value (Yojson.Safe.from_string Fixtures.unquoted_json) -let unquoted_from_lexbuf () = - let lexbuf = Lexing.from_string {|{foo: null, bar: "hello"}|} in +let map_ident_and_string () = + let lexbuf = Lexing.from_string {|{foo:"hello"}|} in let lexer_state = Yojson.init_lexer () in let ident_expected expectation reference start len = @@ -42,13 +42,6 @@ let unquoted_from_lexbuf () = skip_over Yojson.Safe.read_lcurl; map_ident (ident_expected "foo"); skip_over Yojson.Safe.read_colon; - skip_over Yojson.Safe.read_space; - skip_over Yojson.Safe.read_null; - skip_over Yojson.Safe.read_comma; - skip_over Yojson.Safe.read_space; - map_ident (ident_expected "bar"); - skip_over Yojson.Safe.read_colon; - skip_over Yojson.Safe.read_space; let variant = skip_over Yojson.Safe.start_any_variant in Alcotest.(check Testable.variant_kind) "String starts with double quote" `Double_quote variant; @@ -64,5 +57,5 @@ let single_json = [ "from_string", `Quick, from_string; "from_file", `Quick, from_file; "unquoted_from_string", `Quick, unquoted_from_string; - "unquoted_from_lexbuf", `Quick, unquoted_from_lexbuf; + "map_ident/map_string", `Quick, map_ident_and_string; ]