diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 1d25782..35e2796 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -43,19 +43,26 @@ jobs: # The Rails-facing specs against the oldest supported ActiveSupport # lines (the main matrix already exercises the newest release): the # drop-in's quirks_mode path and the encoder seam differ across them. + # + # json is pinned per line, never left to resolve: unpinned, it follows + # whatever standard's rubocop allows, and standard 1.57 let json 3 in + # overnight. ActiveSupport 7.1 and 8.0 pass quirks_mode to JSON.generate + # and JSON.parse, which json 3 rejects even without nosj, so they stay + # on json 2; 7.2 and 8.1 run on json 3. rails-compat: - name: Rails compat (activesupport ${{ matrix.activesupport }}) + name: Rails compat (activesupport ${{ matrix.activesupport }}, json ${{ matrix.json }}) runs-on: ubuntu-latest strategy: fail-fast: false matrix: include: - - { activesupport: "~> 7.1.0", ruby: "3.3" } - - { activesupport: "~> 7.2.0", ruby: "3.4" } - - { activesupport: "~> 8.0.0", ruby: "3.4" } - - { activesupport: "~> 8.1.0", ruby: "4.0" } + - { activesupport: "~> 7.1.0", json: "< 3", ruby: "3.3" } + - { activesupport: "~> 7.2.0", json: "~> 3.0", ruby: "3.4" } + - { activesupport: "~> 8.0.0", json: "< 3", ruby: "3.4" } + - { activesupport: "~> 8.1.0", json: "~> 3.0", ruby: "4.0" } env: NOSJ_ACTIVESUPPORT_VERSION: ${{ matrix.activesupport }} + NOSJ_JSON_VERSION: ${{ matrix.json }} steps: - uses: actions/checkout@v6 @@ -67,8 +74,8 @@ jobs: cargo-cache: true # The committed lockfile resolves the newest ActiveSupport; the - # pin needs a fresh resolution. - - name: Bundle with the pinned ActiveSupport + # pins need a fresh resolution. + - name: Bundle with the pinned ActiveSupport and json run: | rm Gemfile.lock bundle install diff --git a/CHANGELOG.md b/CHANGELOG.md index 40ddbc3..f0977fe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,13 @@ +## [0.5.1] - 2026-10-06 + +- Fixed: `NOSJ.patch` read a `path` without the leading slash (`"a"` + instead of `"/a"`) as the root pointer, so `add`, `copy`, and `move` + silently replaced the whole document with the value instead of + raising, and `remove` reported `cannot remove the root`. Such paths + now raise the same `ArgumentError` as `NOSJ.at_pointer` + (`JSON Pointer must be empty or start with '/'`) in every op. The + bug dates back to 0.3.0. Found by fuzzing. + ## [0.5.0] - 2026-09-25 **json 3.0 compatibility.** nosj now matches the behavior of the json diff --git a/Cargo.lock b/Cargo.lock index 99f88ab..557f080 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -200,7 +200,7 @@ dependencies = [ [[package]] name = "nosj_native" -version = "0.5.0" +version = "0.5.1" dependencies = [ "ahash", "magnus", diff --git a/Gemfile.lock b/Gemfile.lock index 73d21f9..be507a7 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - nosj (0.5.0) + nosj (0.5.1) rb_sys (~> 0.9.91) GEM diff --git a/ext/nosj/Cargo.toml b/ext/nosj/Cargo.toml index 3c97fe4..c28133e 100644 --- a/ext/nosj/Cargo.toml +++ b/ext/nosj/Cargo.toml @@ -5,7 +5,7 @@ # required by lib/nosj.rb as "nosj/nosj" (Init_nosj set explicitly in # lib.rs). Gem name, module, and API are plain nosj. name = "nosj_native" -version = "0.5.0" +version = "0.5.1" edition = "2021" authors = ["Yaroslav Markin "] license = "MIT" diff --git a/ext/nosj/fuzz/seeds/lines/overflow_float.ndjson b/ext/nosj/fuzz/seeds/lines/overflow_float.ndjson new file mode 100644 index 0000000..98e5ef3 --- /dev/null +++ b/ext/nosj/fuzz/seeds/lines/overflow_float.ndjson @@ -0,0 +1,2 @@ +{"big":1e999} +-124.4e044754 diff --git a/ext/nosj/fuzz/seeds/patch/overflow_literal.bin b/ext/nosj/fuzz/seeds/patch/overflow_literal.bin new file mode 100644 index 0000000..ab5eb62 Binary files /dev/null and b/ext/nosj/fuzz/seeds/patch/overflow_literal.bin differ diff --git a/ext/nosj/fuzz/seeds/patch/overflow_splice.bin b/ext/nosj/fuzz/seeds/patch/overflow_splice.bin new file mode 100644 index 0000000..3f151f9 Binary files /dev/null and b/ext/nosj/fuzz/seeds/patch/overflow_splice.bin differ diff --git a/ext/nosj/fuzz/seeds/patch/overflow_value.bin b/ext/nosj/fuzz/seeds/patch/overflow_value.bin new file mode 100644 index 0000000..bed45f2 Binary files /dev/null and b/ext/nosj/fuzz/seeds/patch/overflow_value.bin differ diff --git a/ext/nosj/fuzz/seeds/patch/slashless_path.bin b/ext/nosj/fuzz/seeds/patch/slashless_path.bin new file mode 100644 index 0000000..8f5d453 Binary files /dev/null and b/ext/nosj/fuzz/seeds/patch/slashless_path.bin differ diff --git a/ext/nosj/fuzz/src/prelude.rb b/ext/nosj/fuzz/src/prelude.rb index d6aacdb..5ed4ad0 100644 --- a/ext/nosj/fuzz/src/prelude.rb +++ b/ext/nosj/fuzz/src/prelude.rb @@ -140,11 +140,19 @@ def lines_case(bytes) raise "yielded values diverge from per-line parses" unless yielded.eql?(reference) if status == :ok && !yielded.empty? - # Parsed values are always generable: parse refuses lone - # surrogates, the only source of unencodable strings. - ndjson = stage("generate_lines") { NOSJ.generate_lines_native(yielded, nil) } + # Parse refuses lone surrogates, the only unencodable strings, but + # an overflow float (1e999, 124.4e044754) parses to Infinity, which + # generates only under allow_nan; it lifts exactly that refusal, + # and the round-trip then reads Infinity back under it too. + opts = nil + ndjson = begin + NOSJ.generate_lines_native(yielded, nil) + rescue NOSJ::GeneratorError + opts = {allow_nan: true} + stage("generate_lines with allow_nan") { NOSJ.generate_lines_native(yielded, opts) } + end back = [] - NOSJ.each_line_native(ndjson, nil) { |v| back << v } + NOSJ.each_line_native(ndjson, opts) { |v| back << v } raise "generate_lines does not round-trip" unless back.eql?(yielded) end nil @@ -227,6 +235,7 @@ def rfc6902_case(doc, tree_status, tree, ops) # --- the pure-Ruby reference implementations ------------------------ def ref_splice(tree, edits) + edits.each_value { |value| generable(value) } token_lists = edits.keys.map { |ptr| ptr_tokens(ptr) } token_lists.combination(2) do |a, b| short, long = (a.length <= b.length) ? [a, b] : [b, a] @@ -262,8 +271,8 @@ def ref_patch(tree, ops) path = op["path"] raise RefFail unless path.is_a?(String) case name - when "add" then ref_add(box, ptr_tokens(path), fetch_value(op)) - when "replace" then ref_replace(box, ptr_tokens(path), fetch_value(op)) + when "add" then ref_add(box, ptr_tokens(path), generable(fetch_value(op))) + when "replace" then ref_replace(box, ptr_tokens(path), generable(fetch_value(op))) when "remove" then ref_remove(box, ptr_tokens(path)) when "move" from = op["from"] @@ -366,6 +375,24 @@ def fetch_value(op) op["value"] end + # splice, add, and replace generate their values with default options, + # which refuse non-finite floats, and an overflow literal in the spec + # (1e999) parses to Infinity. test compares, and move/copy splice raw + # document bytes, so a 1e999 already in the document passes there. + def generable(value) + raise RefFail unless finite_floats?(value) + value + end + + def finite_floats?(v) + case v + when Float then v.finite? + when Array then v.all? { |e| finite_floats?(e) } + when Hash then v.each_value.all? { |e| finite_floats?(e) } + else true + end + end + def deep_dup(v) case v when Hash then v.transform_values { |e| deep_dup(e) } diff --git a/ext/nosj/src/patch.rs b/ext/nosj/src/patch.rs index 8e983e7..0259292 100644 --- a/ext/nosj/src/patch.rs +++ b/ext/nosj/src/patch.rs @@ -119,11 +119,25 @@ fn container_children( } /// Split an RFC 6901 pointer into (parent, unescaped last token). -/// Returns None for the root pointer "". -fn split_pointer(pointer: &str) -> Option<(&str, String)> { - let cut = pointer.rfind('/')?; +/// Returns None for the root pointer "" only: any other pointer must +/// start with '/', else the crate's own InvalidPointer error raises as +/// ArgumentError, exactly like `span_at`. (A slash-free path once read +/// as the root, and `add` replaced the whole document.) +fn split_pointer<'p>(ruby: &Ruby, pointer: &'p str) -> Result, Error> { + if pointer.is_empty() { + return Ok(None); + } + if !pointer.starts_with('/') { + // Byte 0 is where the crate reports a missing leading slash. + let invalid = nosj::ParseError { + offset: 0, + kind: nosj::ErrorKind::InvalidPointer, + }; + return Err(arg_error(ruby, invalid.to_string())); + } + let cut = pointer.rfind('/').unwrap_or(0); let token = pointer[cut + 1..].replace("~1", "/").replace("~0", "~"); - Some((&pointer[..cut], token)) + Ok(Some((&pointer[..cut], token))) } /// RFC 6902 array index token: digits only, no leading zeros. @@ -211,7 +225,7 @@ fn op_add( value: &Insert, cfg: &GenConfig, ) -> Result, Error> { - let Some((parent, token)) = split_pointer(path) else { + let Some((parent, token)) = split_pointer(ruby, path)? else { // Root: the value becomes the entire document. return apply_edit(ruby, doc, 0, doc.len(), value, cfg); }; @@ -295,7 +309,7 @@ fn extend_insert( /// RFC 6902 `remove`: the entry disappears along with its key and one /// separating comma; surrounding formatting stays. fn op_remove(ruby: &Ruby, doc: &[u8], path: &str) -> Result, Error> { - let Some((parent, token)) = split_pointer(path) else { + let Some((parent, token)) = split_pointer(ruby, path)? else { return Err(patch_error(ruby, "cannot remove the root".into())); }; let Some((pstart, pend)) = span_at(ruby, doc, parent)? else { diff --git a/lib/nosj/version.rb b/lib/nosj/version.rb index b942fa9..cf91bd2 100644 --- a/lib/nosj/version.rb +++ b/lib/nosj/version.rb @@ -2,5 +2,5 @@ module NOSJ # The gem version. - VERSION = "0.5.0" + VERSION = "0.5.1" end diff --git a/spec/patch_spec.rb b/spec/patch_spec.rb index 762536f..4d08363 100644 --- a/spec/patch_spec.rb +++ b/spec/patch_spec.rb @@ -215,6 +215,28 @@ def apply(doc, patch) end end + # Found by the patch fuzzer: a slash-free path used to read as the + # root, so add replaced the whole document. + it "refuses pointers without a leading slash like at_pointer, never as the root" do + doc = %({"a":1}) + message = begin + NOSJ.at_pointer(doc, "a") + rescue ArgumentError => e + e.message + end + [ + {"op" => "add", "path" => "a", "value" => 2}, + {"op" => "remove", "path" => "a"}, + {"op" => "replace", "path" => "a", "value" => 2}, + {"op" => "test", "path" => "a", "value" => 1}, + {"op" => "move", "from" => "/a", "path" => "b"}, + {"op" => "copy", "from" => "/a", "path" => "b"}, + {"op" => "copy", "from" => "a", "path" => "/b"} + ].each do |op| + expect { NOSJ.patch(doc, [op]) }.to raise_error(ArgumentError, message), op.inspect + end + end + it "move onto itself is a no-op" do expect(NOSJ.patch(%({"a":1}), [{"op" => "move", "from" => "/a", "path" => "/a"}])) .to eq(%({"a":1}))