Skip to content

Commit bba499a

Browse files
committed
WIP: Fail or warn on duplicated key during generation
I think I found a way that has very minimal overhead as long as the hash has only string or only symbols as keys.
1 parent 2cafd12 commit bba499a

File tree

3 files changed

+42
-3
lines changed

3 files changed

+42
-3
lines changed

ext/json/ext/generator/generator.c

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -991,6 +991,7 @@ struct hash_foreach_arg {
991991
struct generate_json_data *data;
992992
int first_key_type;
993993
bool first;
994+
bool mixed_keys_encountered;
994995
};
995996

996997
static VALUE
@@ -1008,6 +1009,17 @@ convert_string_subclass(VALUE key)
10081009
return key_to_s;
10091010
}
10101011

1012+
RBIMPL_ATTR_NOINLINE()
1013+
static void
1014+
json_inspect_hash_with_mixed_keys(struct hash_foreach_arg *arg)
1015+
{
1016+
if (arg->mixed_keys_encountered) {
1017+
return;
1018+
}
1019+
arg->mixed_keys_encountered = true;
1020+
rb_funcall(mJSON, rb_intern("on_mixed_keys_hash"), 1, arg->hash);
1021+
}
1022+
10111023
static int
10121024
json_object_i(VALUE key, VALUE val, VALUE _arg)
10131025
{
@@ -1018,7 +1030,6 @@ json_object_i(VALUE key, VALUE val, VALUE _arg)
10181030
JSON_Generator_State *state = data->state;
10191031

10201032
long depth = state->depth;
1021-
10221033
int key_type = rb_type(key);
10231034

10241035
if (arg->first) {
@@ -1043,7 +1054,7 @@ json_object_i(VALUE key, VALUE val, VALUE _arg)
10431054
switch (key_type) {
10441055
case T_STRING:
10451056
if (RB_UNLIKELY(arg->first_key_type != T_STRING)) {
1046-
fprintf(stderr, "mixed keys (str)\n");
1057+
json_inspect_hash_with_mixed_keys(arg);
10471058
}
10481059

10491060
if (RB_LIKELY(RBASIC_CLASS(key) == rb_cString)) {
@@ -1054,7 +1065,7 @@ json_object_i(VALUE key, VALUE val, VALUE _arg)
10541065
break;
10551066
case T_SYMBOL:
10561067
if (RB_UNLIKELY(arg->first_key_type != T_SYMBOL)) {
1057-
fprintf(stderr, "mixed keys (sym)\n");
1068+
json_inspect_hash_with_mixed_keys(arg);
10581069
}
10591070

10601071
key_to_s = rb_sym2str(key);

lib/json/common.rb

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,27 @@ def generator=(generator) # :nodoc:
186186

187187
private
188188

189+
# Called from the extension when a hash has both string and symbol keys
190+
def on_mixed_keys_hash(hash)
191+
set = {}
192+
hash.each_key do |key|
193+
key_str = case key
194+
when String
195+
key
196+
when Symbol
197+
key.name
198+
else
199+
key.to_s
200+
end
201+
202+
if set[key_str]
203+
raise GeneratorError, "Duplicate key #{key_str.inspect} in #{hash.inspect}"
204+
else
205+
set[key_str] = true
206+
end
207+
end
208+
end
209+
189210
def deprecated_singleton_attr_accessor(*attrs)
190211
args = RUBY_VERSION >= "3.0" ? ", category: :deprecated" : ""
191212
attrs.each do |attr|

test/json/json_generator_test.rb

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -828,4 +828,11 @@ def test_numbers_of_various_sizes
828828
assert_equal "[#{number}]", JSON.generate([number])
829829
end
830830
end
831+
832+
def test_generate_duplicate_keys
833+
error = assert_raise JSON::GeneratorError do
834+
JSON.generate({ foo: 1, "foo" => 2})
835+
end
836+
assert_equal 'Duplicate key "foo" in {foo: 1, "foo" => 2}', error.message
837+
end
831838
end

0 commit comments

Comments
 (0)