From 3a97ce94339f9185b925a21db7ff61968d41c04b Mon Sep 17 00:00:00 2001 From: Hugo Heuzard Date: Thu, 2 Nov 2023 21:40:38 +0100 Subject: [PATCH 01/10] Add missing len argument to yaml_to_string --- lib/yaml.ml | 4 ++-- lib/yaml.mli | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/yaml.ml b/lib/yaml.ml index f3eabba..2ef53a0 100644 --- a/lib/yaml.ml +++ b/lib/yaml.ml @@ -122,8 +122,8 @@ let to_string_exn ?len ?encoding ?scalar_style ?layout_style s = | Ok s -> s | Error (`Msg m) -> raise (Invalid_argument m) -let yaml_to_string ?(encoding = `Utf8) ?scalar_style ?layout_style v = - emitter () >>= fun t -> +let yaml_to_string ?len ?(encoding = `Utf8) ?scalar_style ?layout_style v = + emitter ?len () >>= fun t -> stream_start t encoding >>= fun () -> document_start t >>= fun () -> let rec iter = function diff --git a/lib/yaml.mli b/lib/yaml.mli index 285f107..ea4e1cb 100644 --- a/lib/yaml.mli +++ b/lib/yaml.mli @@ -169,6 +169,7 @@ val yaml_of_string : string -> yaml res Yaml-specific information such as anchors. *) val yaml_to_string : + ?len:int -> ?encoding:encoding -> ?scalar_style:scalar_style -> ?layout_style:layout_style -> From 032dd16b3911568669d5480580d9420e2b72a54e Mon Sep 17 00:00:00 2001 From: Hugo Heuzard Date: Thu, 2 Nov 2023 21:55:32 +0100 Subject: [PATCH 02/10] Fix doc --- lib/yaml.mli | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/yaml.mli b/lib/yaml.mli index ea4e1cb..e81d2e1 100644 --- a/lib/yaml.mli +++ b/lib/yaml.mli @@ -144,7 +144,7 @@ val to_string : (** [to_string v] converts the JSON value to a Yaml string representation. The [encoding], [scalar_style] and [layout_style] control the various output parameters. The current implementation uses a non-resizable internal string - buffer of 64KB, which can be increased via [len]. *) + buffer of 256KB, which can be increased via [len]. *) val to_string_exn : ?len:int -> @@ -178,7 +178,7 @@ val yaml_to_string : (** [yaml_to_string v] converts the Yaml value to a string representation. The [encoding], [scalar_style] and [layout_style] control the various output parameters. The current implementation uses a non-resizable internal string - buffer of 16KB, which can be increased via [len]. *) + buffer of 256KB, which can be increased via [len]. *) (** {2 JSON/Yaml conversion functions} *) From 98fe751d115b004e2ece543d40bc227bb4665d0b Mon Sep 17 00:00:00 2001 From: Hugo Heuzard Date: Fri, 3 Nov 2023 10:27:54 +0100 Subject: [PATCH 03/10] Fix: emitter buffer could move under its feet because of GC --- ffi/bindings/yaml_bindings.ml | 2 +- lib/stream.ml | 11 ++++++----- lib/yaml.ml | 4 ++-- lib/yaml.mli | 2 +- tests/test_emit.ml | 2 +- tests/test_reflect.ml | 2 +- 6 files changed, 12 insertions(+), 11 deletions(-) diff --git a/ffi/bindings/yaml_bindings.ml b/ffi/bindings/yaml_bindings.ml index 9f83ff0..e99b01c 100644 --- a/ffi/bindings/yaml_bindings.ml +++ b/ffi/bindings/yaml_bindings.ml @@ -57,7 +57,7 @@ module M (F : Ctypes.FOREIGN) = struct foreign "yaml_emitter_set_output_string" C.( ptr T.Emitter.t - @-> ocaml_bytes + @-> ptr char @-> size_t @-> ptr size_t @-> returning void) diff --git a/lib/stream.ml b/lib/stream.ml index 7606a71..189831a 100644 --- a/lib/stream.ml +++ b/lib/stream.ml @@ -261,7 +261,7 @@ let do_parse { p; event } = type emitter = { e : T.Emitter.t Ctypes.structure Ctypes.ptr; event : T.Event.t Ctypes.structure Ctypes.ptr; - buf : Bytes.t; + buf : char Ctypes.ptr; written : Unsigned.size_t Ctypes.ptr; } @@ -273,15 +273,16 @@ let emitter ?(len = 65535 * 4) () = let event = Ctypes.(allocate_n T.Event.t ~count:1) in let written = Ctypes.allocate_n Ctypes.size_t ~count:1 in let r = B.emitter_init e in - let buf = Bytes.create len in - let len = Bytes.length buf |> Unsigned.Size_t.of_int in - B.emitter_set_output_string e (Ctypes.ocaml_bytes_start buf) len written; + let buf = Ctypes.(allocate_n Ctypes.char ~count:len) in + let len = Unsigned.Size_t.of_int len in + B.emitter_set_output_string e buf len written; match r with | 1 -> Ok { e; event; written; buf } | n -> Error (`Msg ("error initialising emitter: " ^ string_of_int n)) let emitter_buf { buf; written } = - Ctypes.(!@written) |> Unsigned.Size_t.to_int |> Bytes.sub buf 0 + let length = Ctypes.(!@written) |> Unsigned.Size_t.to_int in + Ctypes.string_from_ptr buf ~length let check l a = match a with diff --git a/lib/yaml.ml b/lib/yaml.ml index 2ef53a0..6479fa0 100644 --- a/lib/yaml.ml +++ b/lib/yaml.ml @@ -115,7 +115,7 @@ let to_string ?len ?(encoding = `Utf8) ?scalar_style ?layout_style (v : value) = document_end t >>= fun () -> stream_end t >>= fun () -> let r = Stream.emitter_buf t in - Ok (Bytes.to_string r) + Ok r let to_string_exn ?len ?encoding ?scalar_style ?layout_style s = match to_string ?len ?encoding ?scalar_style ?layout_style s with @@ -151,7 +151,7 @@ let yaml_to_string ?len ?(encoding = `Utf8) ?scalar_style ?layout_style v = document_end t >>= fun () -> stream_end t >>= fun () -> let r = Stream.emitter_buf t in - Ok (Bytes.to_string r) + Ok r let yaml_of_string s = let open Event in diff --git a/lib/yaml.mli b/lib/yaml.mli index e81d2e1..bd244ab 100644 --- a/lib/yaml.mli +++ b/lib/yaml.mli @@ -278,7 +278,7 @@ module Stream : sig buffer that the output is written into is. In the future, [len] will be redundant as the buffer will be dynamically allocated. *) - val emitter_buf : emitter -> Bytes.t + val emitter_buf : emitter -> string val emit : emitter -> Event.t -> unit res val document_start : ?version:version -> ?implicit:bool -> emitter -> unit res val document_end : ?implicit:bool -> emitter -> unit res diff --git a/tests/test_emit.ml b/tests/test_emit.ml index a49fc8e..814c269 100644 --- a/tests/test_emit.ml +++ b/tests/test_emit.ml @@ -43,5 +43,5 @@ let v () = S.stream_end t >>= fun () -> Printf.printf "written: %d\n%!" (S.emitter_written t); let r = S.emitter_buf t in - print_endline (Bytes.to_string r); + print_endline r; Ok () diff --git a/tests/test_reflect.ml b/tests/test_reflect.ml index ca60e2c..eec0e25 100644 --- a/tests/test_reflect.ml +++ b/tests/test_reflect.ml @@ -31,5 +31,5 @@ let v file = iter_until_done (reflect e) >>= fun () -> let r = Yaml.Stream.emitter_buf e in print_endline buf; - print_endline (Bytes.to_string r); + print_endline r; Ok () From 72e876655c32033dea9af6eed61a1d9813447335 Mon Sep 17 00:00:00 2001 From: Anil Madhavapeddy Date: Sat, 4 Nov 2023 17:07:46 +0000 Subject: [PATCH 04/10] opam: lower bound on alcotest --- yaml.opam | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/yaml.opam b/yaml.opam index a783d42..256116c 100644 --- a/yaml.opam +++ b/yaml.opam @@ -30,7 +30,7 @@ depends: [ "fmt" {with-test} "logs" {with-test} "mdx" {with-test & >= "2.1.0"} - "alcotest" {with-test} + "alcotest" {>="1.5.0" & with-test} "crowbar" {with-test} "junit_alcotest" {with-test} "ezjsonm" {with-test} From 2bf0064ab4032d601d6aa539a887a736caf10525 Mon Sep 17 00:00:00 2001 From: Anil Madhavapeddy Date: Sat, 4 Nov 2023 17:11:12 +0000 Subject: [PATCH 05/10] gha: update ocaml compiler matrix --- .github/workflows/test.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 60a1794..d3fb214 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -7,12 +7,12 @@ jobs: strategy: matrix: operating-system: [macos-latest, ubuntu-latest, windows-latest] - ocaml-compiler: [ '4.12.0', '4.10.2', '4.06.2', '4.05.1'] + ocaml-compiler: [ '5.1.0', '4.14.1', '4.05.1'] steps: - uses: actions/checkout@master - uses: ocaml/setup-ocaml@v2 with: - ocaml-version: ${{ matrix.ocaml-version }} + ocaml-compiler: ${{ matrix.ocaml-compiler }} - run: opam pin add yaml.dev -n . - run: opam pin add yaml-sexp.dev -n . - name: Packages From 218fc6982297f93ec8c2c561b9ebda178909cdef Mon Sep 17 00:00:00 2001 From: Anil Madhavapeddy Date: Sat, 4 Nov 2023 17:15:58 +0000 Subject: [PATCH 06/10] opam: lower bound on junit_alcotest --- yaml.opam | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/yaml.opam b/yaml.opam index 256116c..90f8b8a 100644 --- a/yaml.opam +++ b/yaml.opam @@ -32,7 +32,7 @@ depends: [ "mdx" {with-test & >= "2.1.0"} "alcotest" {>="1.5.0" & with-test} "crowbar" {with-test} - "junit_alcotest" {with-test} + "junit_alcotest" {>= "2.0.2" & with-test} "ezjsonm" {with-test} ] build: [ From 5659645f6d6e0343c4b2a030b9f4b21d649e1148 Mon Sep 17 00:00:00 2001 From: Anil Madhavapeddy Date: Sat, 4 Nov 2023 17:16:54 +0000 Subject: [PATCH 07/10] gh: update compiler matrix --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index d3fb214..78e6614 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -7,7 +7,7 @@ jobs: strategy: matrix: operating-system: [macos-latest, ubuntu-latest, windows-latest] - ocaml-compiler: [ '5.1.0', '4.14.1', '4.05.1'] + ocaml-compiler: [ '5.1.0', '4.14.1', '4.08.0'] steps: - uses: actions/checkout@master - uses: ocaml/setup-ocaml@v2 From 6f82ef60da8347a4db632becd26457b6c6851f46 Mon Sep 17 00:00:00 2001 From: Anil Madhavapeddy Date: Sat, 4 Nov 2023 17:18:45 +0000 Subject: [PATCH 08/10] gh: turn off windows tests for now --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 78e6614..4939019 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -6,7 +6,7 @@ jobs: runs-on: ${{ matrix.operating-system }} strategy: matrix: - operating-system: [macos-latest, ubuntu-latest, windows-latest] + operating-system: [macos-latest, ubuntu-latest] ocaml-compiler: [ '5.1.0', '4.14.1', '4.08.0'] steps: - uses: actions/checkout@master From 6c375a630c80fc5c5806e91b266c1dfa861582af Mon Sep 17 00:00:00 2001 From: Anil Madhavapeddy Date: Sat, 4 Nov 2023 17:45:46 +0000 Subject: [PATCH 09/10] gh: update ocaml versions --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 4939019..01f8fef 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -7,7 +7,7 @@ jobs: strategy: matrix: operating-system: [macos-latest, ubuntu-latest] - ocaml-compiler: [ '5.1.0', '4.14.1', '4.08.0'] + ocaml-compiler: [ '5.1.0', '4.14.1', '4.13.0'] steps: - uses: actions/checkout@master - uses: ocaml/setup-ocaml@v2 From 9b6af217fe737497d889ce80d7a26cf51d80fde8 Mon Sep 17 00:00:00 2001 From: Anil Madhavapeddy Date: Sat, 4 Nov 2023 18:16:56 +0000 Subject: [PATCH 10/10] release v3.2.0 --- CHANGES.md | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index 8d2623d..dbe8a9a 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,7 +1,15 @@ +## v3.2.0 (04/111/2023) + +* Fix potential GC corruption when serialising large Yaml + buffers (#75 @hhugo) + +* Add missing `?len` argument to `yaml_of_string` to specify + buffer size (#74 @hhugo) + ## v3.1.0 (27/03/2022) * Support MSVC with .obj and .lib extensions, and defining - -DDYAML_DECLARE_EXPORT (@jonahbeckford #53) + `-DDYAML_DECLARE_EXPORT` (@jonahbeckford #53) * Upgrade to dune 2 (@TheLortex, #54)