Skip to content

Commit e3de4cc

Browse files
committed
Improve duplicate key warning and errors to include the key name
Followup: #818
1 parent 6d29d75 commit e3de4cc

File tree

5 files changed

+73
-8
lines changed

5 files changed

+73
-8
lines changed

CHANGES.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
### Unreleased
44

5+
* Improve duplicate key warning and errors to include the key name.
6+
57
### 2025-07-24 (2.13.1)
68

79
* Fix support for older compilers without `__builtin_cpu_supports`.

ext/json/ext/parser/parser.c

Lines changed: 47 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -426,6 +426,7 @@ static void emit_parse_warning(const char *message, JSON_ParserState *state)
426426
}
427427

428428
#define PARSE_ERROR_FRAGMENT_LEN 32
429+
429430
#ifdef RBIMPL_ATTR_NORETURN
430431
RBIMPL_ATTR_NORETURN()
431432
#endif
@@ -830,21 +831,64 @@ static inline VALUE json_decode_array(JSON_ParserState *state, JSON_ParserConfig
830831
return array;
831832
}
832833

834+
static VALUE json_find_duplicated_key(size_t count, const VALUE *pairs)
835+
{
836+
VALUE set = rb_hash_new_capa(count / 2);
837+
for (size_t index = 0; index < count; index += 2) {
838+
size_t before = RHASH_SIZE(set);
839+
VALUE key = pairs[index];
840+
rb_hash_aset(set, key, Qtrue);
841+
if (RHASH_SIZE(set) == before) {
842+
if (RB_SYMBOL_P(key)) {
843+
return rb_sym2str(key);
844+
}
845+
return key;
846+
}
847+
}
848+
return Qfalse;
849+
}
850+
851+
static void emit_duplicate_key_warning(JSON_ParserState *state, VALUE duplicate_key)
852+
{
853+
VALUE message = rb_sprintf(
854+
"detected duplicate key %"PRIsVALUE" in JSON object. This will raise an error in json 3.0 unless enabled via `allow_duplicate_key: true`",
855+
rb_inspect(duplicate_key)
856+
);
857+
858+
emit_parse_warning(RSTRING_PTR(message), state);
859+
RB_GC_GUARD(message);
860+
}
861+
862+
#ifdef RBIMPL_ATTR_NORETURN
863+
RBIMPL_ATTR_NORETURN()
864+
#endif
865+
static void raise_duplicate_key_error(JSON_ParserState *state, VALUE duplicate_key)
866+
{
867+
VALUE message = rb_sprintf(
868+
"duplicate key %"PRIsVALUE,
869+
rb_inspect(duplicate_key)
870+
);
871+
872+
raise_parse_error(RSTRING_PTR(message), state);
873+
RB_GC_GUARD(message);
874+
}
875+
833876
static inline VALUE json_decode_object(JSON_ParserState *state, JSON_ParserConfig *config, size_t count)
834877
{
835878
size_t entries_count = count / 2;
836879
VALUE object = rb_hash_new_capa(entries_count);
837-
rb_hash_bulk_insert(count, rvalue_stack_peek(state->stack, count), object);
880+
const VALUE *pairs = rvalue_stack_peek(state->stack, count);
881+
rb_hash_bulk_insert(count, pairs, object);
838882

839883
if (RB_UNLIKELY(RHASH_SIZE(object) < entries_count)) {
840884
switch (config->on_duplicate_key) {
841885
case JSON_IGNORE:
842886
break;
843887
case JSON_DEPRECATED:
844-
emit_parse_warning("detected duplicate keys in JSON object. This will raise an error in json 3.0 unless enabled via `allow_duplicate_key: true`", state);
888+
emit_duplicate_key_warning(state, json_find_duplicated_key(count, pairs));
845889
break;
846890
case JSON_RAISE:
847-
raise_parse_error("duplicate key", state);
891+
raise_duplicate_key_error(state, json_find_duplicated_key(count, pairs));
848892
break;
849893
}
850894
}

java/src/json/ext/ParserConfig.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2125,10 +2125,10 @@ else if ( _widec > _JSON_object_trans_keys[_mid+1] )
21252125
if (((RubyHash)result).hasKey(lastName)) {
21262126
if (config.deprecateDuplicateKey) {
21272127
context.runtime.getWarnings().warning(
2128-
"detected duplicate keys in JSON object. This will raise an error in json 3.0 unless enabled via `allow_duplicate_key: true`"
2128+
"detected duplicate key " + name.inspect() + " in JSON object. This will raise an error in json 3.0 unless enabled via `allow_duplicate_key: true`"
21292129
);
21302130
} else {
2131-
throw parsingError(context, "duplicate key", p, pe);
2131+
throw parsingError(context, "duplicate key" + name.inspect(), p, pe);
21322132
}
21332133
}
21342134
}

java/src/json/ext/ParserConfig.rl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -692,10 +692,10 @@ public class ParserConfig extends RubyObject {
692692
if (((RubyHash)result).hasKey(lastName)) {
693693
if (config.deprecateDuplicateKey) {
694694
context.runtime.getWarnings().warning(
695-
"detected duplicate keys in JSON object. This will raise an error in json 3.0 unless enabled via `allow_duplicate_key: true`"
695+
"detected duplicate key " + name.inspect() + " in JSON object. This will raise an error in json 3.0 unless enabled via `allow_duplicate_key: true`"
696696
);
697697
} else {
698-
throw parsingError(context, "duplicate key", p, pe);
698+
throw parsingError(context, "duplicate key" + name.inspect(), p, pe);
699699
}
700700
}
701701
}

test/json/json_parser_test.rb

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -333,11 +333,30 @@ def test_parse_big_integers
333333

334334
def test_parse_duplicate_key
335335
expected = {"a" => 2}
336+
expected_sym = {a: 2}
337+
336338
assert_equal expected, parse('{"a": 1, "a": 2}', allow_duplicate_key: true)
337339
assert_raise(ParserError) { parse('{"a": 1, "a": 2}', allow_duplicate_key: false) }
338-
assert_deprecated_warning(/duplicate keys/) do
340+
assert_raise(ParserError) { parse('{"a": 1, "a": 2}', allow_duplicate_key: false, symbolize_names: true) }
341+
342+
assert_deprecated_warning(/duplicate key "a"/) do
339343
assert_equal expected, parse('{"a": 1, "a": 2}')
340344
end
345+
assert_deprecated_warning(/duplicate key "a"/) do
346+
assert_equal expected_sym, parse('{"a": 1, "a": 2}', symbolize_names: true)
347+
end
348+
349+
unless RUBY_ENGINE == 'jruby'
350+
assert_raise(ParserError) do
351+
fake_key = Object.new
352+
JSON.load('{"a": 1, "a": 2}', -> (obj) { obj == "a" ? fake_key : obj }, allow_duplicate_key: false)
353+
end
354+
355+
assert_deprecated_warning(/duplicate key #<Object:0x/) do
356+
fake_key = Object.new
357+
JSON.load('{"a": 1, "a": 2}', -> (obj) { obj == "a" ? fake_key : obj })
358+
end
359+
end
341360
end
342361

343362
def test_some_wrong_inputs

0 commit comments

Comments
 (0)