Skip to content

Commit 8ea3ddd

Browse files
committed
Fix segfault on duplicate JSON keys and add tests
1 parent a5c5b81 commit 8ea3ddd

3 files changed

Lines changed: 65 additions & 7 deletions

File tree

native/torque_nif/src/types.rs

Lines changed: 36 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
1-
use rustler::sys::{enif_make_list_from_array, enif_make_map_from_arrays, ERL_NIF_TERM};
1+
use rustler::sys::{
2+
enif_make_list_from_array, enif_make_map_from_arrays, enif_make_map_put, enif_make_new_map,
3+
ERL_NIF_TERM,
4+
};
25
use rustler::{Env, NewBinary, Term};
36
use sonic_rs::{JsonContainerTrait, JsonType, JsonValueTrait};
47

@@ -83,14 +86,18 @@ pub fn value_to_term<'a>(env: Env<'a>, value: &sonic_rs::Value) -> Term<'a> {
8386
}
8487
let mut map: ERL_NIF_TERM = 0;
8588
unsafe {
86-
enif_make_map_from_arrays(
89+
if enif_make_map_from_arrays(
8790
env.as_c_arg(),
8891
keys.as_ptr(),
8992
vals.as_ptr(),
9093
count,
9194
&mut map,
92-
);
93-
Term::new(env, map)
95+
) != 0
96+
{
97+
Term::new(env, map)
98+
} else {
99+
build_map_dedup(env, obj)
100+
}
94101
}
95102
} else {
96103
let mut keys: Vec<ERL_NIF_TERM> = Vec::with_capacity(count);
@@ -101,16 +108,38 @@ pub fn value_to_term<'a>(env: Env<'a>, value: &sonic_rs::Value) -> Term<'a> {
101108
}
102109
let mut map: ERL_NIF_TERM = 0;
103110
unsafe {
104-
enif_make_map_from_arrays(
111+
if enif_make_map_from_arrays(
105112
env.as_c_arg(),
106113
keys.as_ptr(),
107114
vals.as_ptr(),
108115
count,
109116
&mut map,
110-
);
111-
Term::new(env, map)
117+
) != 0
118+
{
119+
Term::new(env, map)
120+
} else {
121+
build_map_dedup(env, obj)
122+
}
112123
}
113124
}
114125
}
115126
}
116127
}
128+
129+
/// Fallback for objects with duplicate keys. Iterates all pairs so that the
130+
/// last value for each duplicate key wins, matching common JSON parser behaviour.
131+
/// Marked `#[cold]` so the optimiser keeps the duplicate-free fast path hot.
132+
#[cold]
133+
fn build_map_dedup<'a>(env: Env<'a>, obj: &sonic_rs::Object) -> Term<'a> {
134+
unsafe {
135+
let mut map = enif_make_new_map(env.as_c_arg());
136+
for (k, v) in obj.iter() {
137+
let key = make_binary_term(env, k).as_c_arg();
138+
let val = value_to_term(env, v).as_c_arg();
139+
let mut new_map: ERL_NIF_TERM = 0;
140+
enif_make_map_put(env.as_c_arg(), map, key, val, &mut new_map);
141+
map = new_map;
142+
}
143+
Term::new(env, map)
144+
}
145+
}

test/decode_test.exs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,18 @@ defmodule Torque.DecodeTest do
7272
assert {:ok, 9_223_372_036_854_775_808} = Torque.decode("9223372036854775808")
7373
end
7474

75+
test "duplicate keys - last value wins" do
76+
assert {:ok, %{"a" => 2}} = Torque.decode(~s({"a":1,"a":2}))
77+
end
78+
79+
test "duplicate keys in nested object - last value wins" do
80+
assert {:ok, %{"x" => %{"a" => 2}}} = Torque.decode(~s({"x":{"a":1,"a":2}}))
81+
end
82+
83+
test "duplicate keys with different value types" do
84+
assert {:ok, %{"k" => "str"}} = Torque.decode(~s({"k":1,"k":true,"k":"str"}))
85+
end
86+
7587
test "invalid json returns error" do
7688
assert {:error, _reason} = Torque.decode("{invalid}")
7789
end

test/torque_test.exs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,23 @@ defmodule Torque.PointerTest do
231231
end
232232
end
233233

234+
describe "duplicate keys" do
235+
test "parse + get on object with duplicate keys - last value wins" do
236+
{:ok, doc} = Torque.parse(~s({"a":1,"b":2,"a":3}))
237+
assert {:ok, %{"a" => 3, "b" => 2}} = Torque.get(doc, "")
238+
end
239+
240+
test "parse + get nested object with duplicate keys" do
241+
{:ok, doc} = Torque.parse(~s({"x":{"k":"first","k":"last"}}))
242+
assert {:ok, %{"k" => "last"}} = Torque.get(doc, "/x")
243+
end
244+
245+
test "parse + get_many with duplicate key object" do
246+
{:ok, doc} = Torque.parse(~s({"a":1,"a":2}))
247+
assert [{:ok, %{"a" => 2}}] = Torque.get_many(doc, [""])
248+
end
249+
end
250+
234251
describe "parse/1 errors" do
235252
test "invalid json" do
236253
assert {:error, _} = Torque.parse("{invalid}")

0 commit comments

Comments
 (0)