From 1cbbb5c1da7de60bfabcc2193b9db5d22f84a935 Mon Sep 17 00:00:00 2001 From: Yaroslav Markin Date: Tue, 6 Oct 2026 14:26:47 +0400 Subject: [PATCH 1/5] Patch: refuse slash-free pointers instead of reading them as the root split_pointer took any pointer without a '/' for the root "", so add (and move/copy, which add) replaced the whole document with the value, and remove reported "cannot remove the root". Only "" is the root now; any other pointer missing its leading slash raises the crate's own InvalidPointer error as ArgumentError, the message at_pointer gives. Found by the patch fuzzer. The new seed pins the shape for CI. --- CHANGELOG.md | 10 +++++++ ext/nosj/fuzz/seeds/patch/slashless_path.bin | Bin 0 -> 43 bytes ext/nosj/src/patch.rs | 26 ++++++++++++++----- spec/patch_spec.rb | 22 ++++++++++++++++ 4 files changed, 52 insertions(+), 6 deletions(-) create mode 100644 ext/nosj/fuzz/seeds/patch/slashless_path.bin diff --git a/CHANGELOG.md b/CHANGELOG.md index 40ddbc3..a676f1a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,13 @@ +## [Unreleased] + +- 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/ext/nosj/fuzz/seeds/patch/slashless_path.bin b/ext/nosj/fuzz/seeds/patch/slashless_path.bin new file mode 100644 index 0000000000000000000000000000000000000000..8f5d4532e8c02170c94b60a1cdf5914812d765d7 GIT binary patch literal 43 ucmbs8ktYwIvQkP+Nm0^KDo8BJ0C9lKvc#OyR3$5;+E@S#CJcB0 literal 0 HcmV?d00001 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/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})) From 9ed77375cd538ac899ea8034d41fd261d9146307 Mon Sep 17 00:00:00 2001 From: Yaroslav Markin Date: Tue, 6 Oct 2026 14:26:55 +0400 Subject: [PATCH 2/5] fuzz: lines round-trip lifts the overflow-float refusal with allow_nan The json 3 cleanup made generate_lines a must-succeed stage on the belief that lone surrogates were the only ungenerable parse output. An overflow float (1e999) parses to Infinity, which plain generate refuses: both weekly runs since failed on it, and 64 inputs of the local corpus already did. Like reformat_case, retry under allow_nan, which lifts exactly that refusal, and read the result back under it. The new seed fails on the previous prelude. --- ext/nosj/fuzz/seeds/lines/overflow_float.ndjson | 2 ++ ext/nosj/fuzz/src/prelude.rb | 16 ++++++++++++---- 2 files changed, 14 insertions(+), 4 deletions(-) create mode 100644 ext/nosj/fuzz/seeds/lines/overflow_float.ndjson 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/src/prelude.rb b/ext/nosj/fuzz/src/prelude.rb index d6aacdb..2cd8206 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 From 4aeee2ad336a9d38e2f49ed162e11b8317a12efc Mon Sep 17 00:00:00 2001 From: Yaroslav Markin Date: Tue, 6 Oct 2026 14:29:18 +0400 Subject: [PATCH 3/5] 0.5.1 --- CHANGELOG.md | 2 +- Cargo.lock | 2 +- Gemfile.lock | 2 +- ext/nosj/Cargo.toml | 2 +- lib/nosj/version.rb | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a676f1a..f0977fe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,4 @@ -## [Unreleased] +## [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` 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/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 From dd6b93d4293d25f22e86dd74521666d517df158c Mon Sep 17 00:00:00 2001 From: Yaroslav Markin Date: Tue, 6 Oct 2026 14:46:05 +0400 Subject: [PATCH 4/5] fuzz: the patch reference refuses non-finite values like generation splice, add, and replace generate their values with default options, which refuse non-finite floats, while an overflow literal in the spec (1e999) parses to Infinity: native raised GeneratorError where the reference inserted Infinity. The reference now refuses such values at the same ops. test compares and move/copy splice raw document bytes, so a 1e999 already in the document still passes there; a seed pins each side of that line. --- .../fuzz/seeds/patch/overflow_literal.bin | Bin 0 -> 91 bytes ext/nosj/fuzz/seeds/patch/overflow_splice.bin | Bin 0 -> 21 bytes ext/nosj/fuzz/seeds/patch/overflow_value.bin | Bin 0 -> 52 bytes ext/nosj/fuzz/src/prelude.rb | 23 ++++++++++++++++-- 4 files changed, 21 insertions(+), 2 deletions(-) create mode 100644 ext/nosj/fuzz/seeds/patch/overflow_literal.bin create mode 100644 ext/nosj/fuzz/seeds/patch/overflow_splice.bin create mode 100644 ext/nosj/fuzz/seeds/patch/overflow_value.bin 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 0000000000000000000000000000000000000000..ab5eb62ba427646a2ae17152981f18d6bea985fc GIT binary patch literal 91 zcmbs8kOtrMMtYwIvQkRUFQ`<~QA#Vy&js@I6M^i4#F7jUJ4vZl2db(h TwYUUDMn|bEF{d;YW?U=)tWz31 literal 0 HcmV?d00001 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 0000000000000000000000000000000000000000..3f151f9825bdd9a196779ec69e53247235d0259c GIT binary patch literal 21 acmbs8ktYxTH(g!kh4O1;GEo%WhCs9nHjJ%hh^|)3FHo{lN=!*n(orf%EXe?J^b_@UfxNQBoYGV!E5lSvOUv3= E0Ea*h$^ZZW literal 0 HcmV?d00001 diff --git a/ext/nosj/fuzz/src/prelude.rb b/ext/nosj/fuzz/src/prelude.rb index 2cd8206..5ed4ad0 100644 --- a/ext/nosj/fuzz/src/prelude.rb +++ b/ext/nosj/fuzz/src/prelude.rb @@ -235,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] @@ -270,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"] @@ -374,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) } From 747e135b2c293f79c04a891154636a0cc4bf2130 Mon Sep 17 00:00:00 2001 From: Yaroslav Markin Date: Tue, 6 Oct 2026 14:46:07 +0400 Subject: [PATCH 5/5] CI: pin json per Rails compat line The Rails jobs re-resolve without the lockfile, and json was only ever held at 2.x by standard's rubocop. standard 1.57 (2026-10-05) moved to rubocop 1.91, which allows json 3, and stock ActiveSupport 7.1 and 8.0 pass quirks_mode to JSON.generate and JSON.parse, which json 3 rejects even without nosj. Pin json explicitly per line: 7.1 and 8.0 on json 2, 7.2 and 8.1 on json 3 (both verified working stock). --- .github/workflows/test.yml | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) 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