From 4e438f3e11ab3e09d4ac8aacb14b106c65d37cb1 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Fri, 19 Jun 2026 14:06:47 +0200 Subject: [PATCH] ResumableParser: accept only keyword arguments Fix: https://github.com/ruby/json/pull/1016#issuecomment-4744487710 `json` takes option hashes across the board, mostly because its API predates the introduction of keyword arguments. I'd like to change that to only take keyword arguments and error when an unknown argument is passed, but I'm not yet sure of the backward compatibility consequences, so it might wait for the next major. But in the meantime, `ResumableParser` being a new API, it can safely use keyword arguments. --- ext/json/ext/parser/parser.c | 46 +++++++++++++++++++++--------- test/json/resumable_parser_test.rb | 20 +++++++++++-- 2 files changed, 51 insertions(+), 15 deletions(-) diff --git a/ext/json/ext/parser/parser.c b/ext/json/ext/parser/parser.c index 057367bc..e4dc3fb0 100644 --- a/ext/json/ext/parser/parser.c +++ b/ext/json/ext/parser/parser.c @@ -1966,6 +1966,8 @@ static VALUE convert_encoding(VALUE source) struct parser_config_init_args { JSON_ParserConfig *config; VALUE self; + VALUE unknown_keywords; + bool strict; }; static void parser_config_wb_write(VALUE self, VALUE *dest, VALUE val) @@ -2019,27 +2021,43 @@ static int parser_config_init_i(VALUE key, VALUE val, VALUE data) } } } + else if (args->strict) { + if (!args->unknown_keywords) { + args->unknown_keywords = rb_obj_hide(rb_ary_new()); + } + rb_ary_push(args->unknown_keywords, key); + } return ST_CONTINUE; } -static void parser_config_init(JSON_ParserConfig *config, VALUE opts, VALUE self) +static void parser_config_init(JSON_ParserConfig *config, VALUE opts, VALUE self, bool strict) { config->max_nesting = 100; struct parser_config_init_args args = { .config = config, .self = self, + .strict = strict, }; - if (!NIL_P(opts)) { - Check_Type(opts, T_HASH); - if (RHASH_SIZE(opts) > 0) { - // We assume in most cases few keys are set so it's faster to go over - // the provided keys than to check all possible keys. - rb_hash_foreach(opts, parser_config_init_i, (VALUE)&args); - } + if (NIL_P(opts)) return; + Check_Type(opts, T_HASH); + if (RHASH_SIZE(opts) == 0) return; + + // We assume in most cases few keys are set so it's faster to go over + // the provided keys than to check all possible keys. + rb_hash_foreach(opts, parser_config_init_i, (VALUE)&args); + if (RB_UNLIKELY(args.unknown_keywords)) { + if (RARRAY_LEN(args.unknown_keywords) == 1) { + rb_raise(rb_eArgError, "unknown keyword: %" PRIsVALUE, RARRAY_AREF(args.unknown_keywords, 0)); + } + else { + VALUE keywords = rb_ary_join(args.unknown_keywords, rb_utf8_str_new_cstr(", ")); + rb_raise(rb_eArgError, "unknown keywords: %s", RSTRING_PTR(keywords)); + RB_GC_GUARD(keywords); + } } } @@ -2057,7 +2075,7 @@ static VALUE cParserConfig_initialize(VALUE self, VALUE opts) rb_check_frozen(self); GET_PARSER_CONFIG; - parser_config_init(config, opts, self); + parser_config_init(config, opts, self, false); return self; } @@ -2153,7 +2171,7 @@ static VALUE cParser_m_parse(VALUE klass, VALUE Vsource, VALUE opts) { JSON_ParserConfig _config = {0}; JSON_ParserConfig *config = &_config; - parser_config_init(config, opts, false); + parser_config_init(config, opts, Qfalse, false); return cParser_parse(config, Vsource); } @@ -2342,12 +2360,14 @@ static inline JSON_ResumableParser *cResumableParser_get(VALUE self) */ static VALUE cResumableParser_initialize(int argc, VALUE *argv, VALUE self) { - rb_check_arity(argc, 0, 1); rb_check_frozen(self); + + VALUE opts = Qfalse; + rb_scan_args_kw(RB_SCAN_ARGS_LAST_HASH_KEYWORDS, argc, argv, "0:", &opts); JSON_ResumableParser *parser = cResumableParser_get(self); - VALUE opts = argc > 0 ? argv[0] : Qnil; - parser_config_init(&parser->config, opts, self); + opts = argc > 0 ? argv[0] : Qnil; + parser_config_init(&parser->config, opts, self, true); return self; } diff --git a/test/json/resumable_parser_test.rb b/test/json/resumable_parser_test.rb index 1a981e6b..800502f4 100644 --- a/test/json/resumable_parser_test.rb +++ b/test/json/resumable_parser_test.rb @@ -9,6 +9,22 @@ def setup @parser = new_parser end + def test_keyword_arguments + new_parser + new_parser({}) + new_parser(allow_nan: true) + + error = assert_raise(ArgumentError) do + new_parser(doesnt_exist: true, allow_nan: true) + end + assert_equal "unknown keyword: doesnt_exist", error.message + + error = assert_raise(ArgumentError) do + new_parser(doesnt_exist: true, allow_nan: true, a: 1, b: 2) + end + assert_equal "unknown keywords: doesnt_exist, a, b", error.message + end + def test_value refute_predicate @parser, :value? assert_raise(ArgumentError) { @parser.value } @@ -444,7 +460,7 @@ def assert_parse_stream(expected, json, parser = @parser) assert_equal(expected, actual) end - def new_parser(options = nil) - JSON::ResumableParser.new(options) + def new_parser(...) + JSON::ResumableParser.new(...) end end