Skip to content

[ruby] fix: explode object query parameters #24869

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wiebren wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[ruby] fix: explode object query parameters #24869

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions modules/openapi-generator/src/main/resources/ruby-client/api.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,23 @@ module {{moduleName}}
query_params[:'{{{baseName}}}'] = {{{paramName}}}.to_json
{{/queryIsJsonMimeType}}
{{^queryIsJsonMimeType}}
{{#isMap}}
{{#isExplode}}
{{^isDeepObject}}
# form style explodes an object into one query parameter per entry, keyed by the property name alone
{{{paramName}}}.each { |name, value| query_params[name.to_s] = value }

@cubic-dev-ai cubic-dev-ai Bot Sep 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: The PR description claims exploded-map collisions with other declared parameters are "handled without overwriting", but query_params[name.to_s] = value is a plain hash assignment that overwrites any existing entry. If a map key equals another declared query parameter's name, the value depends purely on parameter ordering in the generated method and the earlier entry is silently dropped. Either drop the collision claim or guard the assignment (e.g. query_params[name.to_s] = value unless query_params.key?(name.to_s)), keeping the explicit map entries from clobbering declared parameters.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/resources/ruby-client/api.mustache, line 187:

<comment>The PR description claims exploded-map collisions with other declared parameters are "handled without overwriting", but `query_params[name.to_s] = value` is a plain hash assignment that overwrites any existing entry. If a map key equals another declared query parameter's name, the value depends purely on parameter ordering in the generated method and the earlier entry is silently dropped. Either drop the collision claim or guard the assignment (e.g. `query_params[name.to_s] = value unless query_params.key?(name.to_s)`), keeping the explicit map entries from clobbering declared parameters.</comment>

<file context>
@@ -180,7 +180,23 @@ module {{moduleName}}
+      {{#isExplode}}
+      {{^isDeepObject}}
+      # form style explodes an object into one query parameter per entry, keyed by the property name alone
+      {{{paramName}}}.each { |name, value| query_params[name.to_s] = value }
+      {{/isDeepObject}}
+      {{#isDeepObject}}
</file context>

{{/isDeepObject}}
{{#isDeepObject}}
query_params[:'{{{baseName}}}'] = {{{paramName}}}
{{/isDeepObject}}
{{/isExplode}}
{{^isExplode}}
query_params[:'{{{baseName}}}'] = {{{paramName}}}
{{/isExplode}}
{{/isMap}}
{{^isMap}}
query_params[:'{{{baseName}}}'] = {{#collectionFormat}}@api_client.build_collection_param({{{paramName}}}, :{{{collectionFormat}}}){{/collectionFormat}}{{^collectionFormat}}{{{paramName}}}{{/collectionFormat}}
{{/isMap}}
{{/queryIsJsonMimeType}}
{{/required}}
{{/queryParams}}
Expand All @@ -190,7 +206,23 @@ module {{moduleName}}
query_params[:'{{{baseName}}}'] = opts[:'{{{paramName}}}'].to_json if !opts[:'{{{paramName}}}'].nil?
{{/queryIsJsonMimeType}}
{{^queryIsJsonMimeType}}
{{#isMap}}
{{#isExplode}}
{{^isDeepObject}}
# form style explodes an object into one query parameter per entry, keyed by the property name alone
opts[:'{{{paramName}}}'].each { |name, value| query_params[name.to_s] = value } if !opts[:'{{{paramName}}}'].nil?
{{/isDeepObject}}
{{#isDeepObject}}
query_params[:'{{{baseName}}}'] = opts[:'{{{paramName}}}'] if !opts[:'{{{paramName}}}'].nil?
{{/isDeepObject}}
{{/isExplode}}
{{^isExplode}}
query_params[:'{{{baseName}}}'] = opts[:'{{{paramName}}}'] if !opts[:'{{{paramName}}}'].nil?
{{/isExplode}}
{{/isMap}}
{{^isMap}}
query_params[:'{{{baseName}}}'] = {{#collectionFormat}}@api_client.build_collection_param(opts[:'{{{paramName}}}'], :{{{collectionFormat}}}){{/collectionFormat}}{{^collectionFormat}}opts[:'{{{paramName}}}']{{/collectionFormat}} if !opts[:'{{{paramName}}}'].nil?
{{/isMap}}
{{/queryIsJsonMimeType}}
{{/required}}
{{/queryParams}}
Expand Down
36 changes: 36 additions & 0 deletions .../openapi-generator/src/test/java/org/openapitools/codegen/ruby/RubyClientCodegenTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,42 @@ public void testGenerateRubyClientWithHtmlEntity() throws Exception {
}
}

@Test(description = "Verify an object query parameter is exploded, whether or not it declares its properties")
public void testExplodedObjectQueryParameter() throws Exception {
final File output = Files.createTempDirectory("test").toFile();
output.deleteOnExit();

final OpenAPI openAPI = TestUtils.parseFlattenSpec("src/test/resources/3_0/exploded-object-query-param.yaml");
RubyClientCodegen codegen = new RubyClientCodegen();
codegen.setOutputDir(output.getAbsolutePath());

ClientOptInput clientOptInput = new ClientOptInput().openAPI(openAPI).config(codegen);
DefaultGenerator generator = new DefaultGenerator();
List<File> files = generator.opts(clientOptInput).generate();
files.forEach(File::deleteOnExit);

File apiFile = files.stream()
.filter(f -> f.getName().equals("default_api.rb"))
.findFirst()
.orElseThrow(() -> new AssertionError("default_api.rb not found in generated files"));

// form style with explode - the default - puts every entry on the wire under its own
// property name. Assigning the whole hash under the parameter name left the http
// library to serialize it in bracket style, which is what used to happen.
TestUtils.assertFileContains(apiFile.toPath(),
"opts[:'filter'].each { |name, value| query_params[name.to_s] = value } if !opts[:'filter'].nil?");
TestUtils.assertFileNotContains(apiFile.toPath(), "query_params[:'filter']");

// a declared map behaves the same way
TestUtils.assertFileContains(apiFile.toPath(),
"opts[:'typed_filter'].each { |name, value| query_params[name.to_s] = value } if !opts[:'typed_filter'].nil?");

// deepObject and form without explode both keep a single parameter
TestUtils.assertFileContains(apiFile.toPath(),
"query_params[:'deepFilter'] = opts[:'deep_filter'] if !opts[:'deep_filter'].nil?",
"query_params[:'flatFilter'] = opts[:'flat_filter'] if !opts[:'flat_filter'].nil?");
}

@Test
public void testInitialConfigValues() {
final RubyClientCodegen codegen = new RubyClientCodegen();
Expand Down
51 changes: 51 additions & 0 deletions modules/openapi-generator/src/test/resources/3_0/exploded-object-query-param.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
openapi: 3.0.3
info:
title: Exploded object query parameters
description: >
Object typed query parameters, covering the four combinations of style and explode that
decide how an object is put on the wire. The free-form variants matter because a
free-form object is flagged isMap but not isContainer.
version: 1.0.0
servers:
- url: localhost:8080
paths:
/items:
get:
operationId: listItems
parameters:
# style and explode both left out, so the form/true defaults apply: every entry
# becomes its own parameter, keyed by the property name alone.
- in: query
name: filter
schema:
type: object
# the same, but declared as a map rather than as a free-form object
- in: query
name: typedFilter
schema:
type: object
additionalProperties:
type: string
# deepObject nests each entry under the parameter name: deepFilter[key]=value
- in: query
name: deepFilter
style: deepObject
explode: true
schema:
type: object
# form without explode keeps a single parameter carrying the whole object
- in: query
name: flatFilter
style: form
explode: false
schema:
type: object
responses:
'200':
description: a list of items
content:
application/json:
schema:
type: array
items:
type: string
3 changes: 2 additions & 1 deletion samples/client/petstore/ruby-autoload/lib/petstore/api/fake_api.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1556,7 +1556,8 @@ def test_query_parameter_collection_format_with_http_info(pipe, ioutil, http, ur
query_params[:'url'] = @api_client.build_collection_param(url, :csv)
query_params[:'context'] = @api_client.build_collection_param(context, :multi)
query_params[:'allowEmpty'] = allow_empty
query_params[:'language'] = opts[:'language'] if !opts[:'language'].nil?
# form style explodes an object into one query parameter per entry, keyed by the property name alone
opts[:'language'].each { |name, value| query_params[name.to_s] = value } if !opts[:'language'].nil?

# header parameters
header_params = opts[:header_params] || {}
Expand Down
3 changes: 2 additions & 1 deletion samples/client/petstore/ruby-faraday/lib/petstore/api/fake_api.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1571,7 +1571,8 @@ def test_query_parameter_collection_format_with_http_info(pipe, ioutil, http, ur
query_params[:'url'] = @api_client.build_collection_param(url, :csv)
query_params[:'context'] = @api_client.build_collection_param(context, :multi)
query_params[:'allowEmpty'] = allow_empty
query_params[:'language'] = opts[:'language'] if !opts[:'language'].nil?
# form style explodes an object into one query parameter per entry, keyed by the property name alone
opts[:'language'].each { |name, value| query_params[name.to_s] = value } if !opts[:'language'].nil?

@cubic-dev-ai cubic-dev-ai Bot Sep 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: The exploded entries use string keys (query_params[name.to_s]), while every other declared parameter in this method is set with a symbol key (query_params[:'pipe'], query_params[:'context'], etc.). Because Ruby treats :"pipe" and "pipe" as distinct hash keys, a language map entry whose name collides with a declared parameter is not overwritten as the PR description claims - it is serialized in addition to the declared parameter, producing two same-named query parameters on the wire (e.g. the array context plus context=<string>). Use the same key namespace or document the collision behavior; mixing symbol-keyed declared params with string-keyed exploded parts makes the "no overwriting" guarantee produce ambiguous duplicate params instead.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/petstore/ruby-faraday/lib/petstore/api/fake_api.rb, line 1575:

<comment>The exploded entries use string keys (`query_params[name.to_s]`), while every other declared parameter in this method is set with a symbol key (`query_params[:'pipe']`, `query_params[:'context']`, etc.). Because Ruby treats `:"pipe"` and `"pipe"` as distinct hash keys, a `language` map entry whose name collides with a declared parameter is not overwritten as the PR description claims - it is serialized in addition to the declared parameter, producing two same-named query parameters on the wire (e.g. the array `context` plus `context=<string>`). Use the same key namespace or document the collision behavior; mixing symbol-keyed declared params with string-keyed exploded parts makes the "no overwriting" guarantee produce ambiguous duplicate params instead.</comment>

<file context>
@@ -1571,7 +1571,8 @@ def test_query_parameter_collection_format_with_http_info(pipe, ioutil, http, ur
       query_params[:'allowEmpty'] = allow_empty
-      query_params[:'language'] = opts[:'language'] if !opts[:'language'].nil?
+      # form style explodes an object into one query parameter per entry, keyed by the property name alone
+      opts[:'language'].each { |name, value| query_params[name.to_s] = value } if !opts[:'language'].nil?
 
       # header parameters
</file context>


# header parameters
header_params = opts[:header_params] || {}
Expand Down
3 changes: 2 additions & 1 deletion samples/client/petstore/ruby-httpx/lib/petstore/api/fake_api.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1571,7 +1571,8 @@ def test_query_parameter_collection_format_with_http_info(pipe, ioutil, http, ur
query_params[:'url'] = @api_client.build_collection_param(url, :csv)
query_params[:'context'] = @api_client.build_collection_param(context, :multi)
query_params[:'allowEmpty'] = allow_empty
query_params[:'language'] = opts[:'language'] if !opts[:'language'].nil?
# form style explodes an object into one query parameter per entry, keyed by the property name alone
opts[:'language'].each { |name, value| query_params[name.to_s] = value } if !opts[:'language'].nil?

# header parameters
header_params = opts[:header_params] || {}
Expand Down
3 changes: 2 additions & 1 deletion samples/client/petstore/ruby/lib/petstore/api/fake_api.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1571,7 +1571,8 @@ def test_query_parameter_collection_format_with_http_info(pipe, ioutil, http, ur
query_params[:'url'] = @api_client.build_collection_param(url, :csv)
query_params[:'context'] = @api_client.build_collection_param(context, :multi)
query_params[:'allowEmpty'] = allow_empty
query_params[:'language'] = opts[:'language'] if !opts[:'language'].nil?
# form style explodes an object into one query parameter per entry, keyed by the property name alone
opts[:'language'].each { |name, value| query_params[name.to_s] = value } if !opts[:'language'].nil?

# header parameters
header_params = opts[:header_params] || {}
Expand Down