Skip to content

Commit 479b757

Browse files
authored
Fix OpenAPI 3 path coercion to respect coerce_path_params independently of query coercion (#467)
This PR fixes OpenAPI 3 request coercion behavior where path parameter coercion was unintentionally coupled to `coerce_query_params`. `OperationWrapper#coerce_path_parameter` was building its parser options using `coerce_query_params` instead of `coerce_path_params`. As a result, this configuration behaved incorrectly. - `coerce_path_params: true` - `coerce_query_params: false` Path coercion could be disabled even when explicitly enabled. Additionally, this PR fixes a related validation gap: path params must still be validated even when `coerce_path_params` is `false`. `coerce_path_params: false` now means: - path params are validated - path params are not coerced and remains independent from `coerce_query_params`. Tests were updated to cover both option combinations and to assert query coercion behavior using actual query params. `bundle exec rake test` passes.
1 parent 7ff2876 commit 479b757

File tree

4 files changed

+65
-3
lines changed

4 files changed

+65
-3
lines changed

lib/committee/schema_validator/open_api_3.rb

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,6 @@ def link_exist?
5555
attr_reader :validator_option
5656

5757
def coerce_path_params
58-
return Committee::Utils.indifferent_hash unless validator_option.coerce_path_params
5958
Committee::RequestUnpacker.indifferent_params(@operation_object.coerce_path_parameter(@validator_option))
6059
end
6160

lib/committee/schema_validator/open_api_3/operation_wrapper.rb

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,10 @@ def http_method
2323

2424
def coerce_path_parameter(validator_option)
2525
options = build_openapi_parser_path_option(validator_option)
26+
validated_path_params = request_operation.validate_path_params(options)
2627
return {} unless options.coerce_value
2728

28-
request_operation.validate_path_params(options)
29+
validated_path_params
2930
rescue OpenAPIParser::OpenAPIError => e
3031
raise Committee::InvalidRequest.new(e.message, original_error: e)
3132
end
@@ -90,6 +91,11 @@ def build_openapi_parser_body_option(validator_option)
9091

9192
# @return [OpenAPIParser::SchemaValidator::Options]
9293
def build_openapi_parser_path_option(validator_option)
94+
build_openapi_parser_option(validator_option, validator_option.coerce_path_params)
95+
end
96+
97+
# @return [OpenAPIParser::SchemaValidator::Options]
98+
def build_openapi_parser_request_parameter_option(validator_option)
9399
build_openapi_parser_option(validator_option, validator_option.coerce_query_params)
94100
end
95101

@@ -125,6 +131,9 @@ def validate_post_request_params(path_params, query_params, body_params, headers
125131
end
126132

127133
def validate_path_and_query_params(path_params, query_params, headers, validator_option)
134+
path_params ||= {}
135+
query_params ||= {}
136+
128137
# it's currently impossible to validate path params and query params separately
129138
# so we have to resort to this workaround
130139

@@ -133,7 +142,7 @@ def validate_path_and_query_params(path_params, query_params, headers, validator
133142

134143
merged_params = query_params.merge(path_params)
135144

136-
request_operation.validate_request_parameter(merged_params, headers, build_openapi_parser_path_option(validator_option))
145+
request_operation.validate_request_parameter(merged_params, headers, build_openapi_parser_request_parameter_option(validator_option))
137146

138147
merged_params.each do |k, v|
139148
path_params[k] = v if path_keys.include?(k)

test/middleware/request_validation_open_api_3_test.rb

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -363,6 +363,26 @@ def app
363363
get "/coerce_path_params/1"
364364
end
365365

366+
it "coerces path params even when query param coercion is disabled" do
367+
check_parameter_string = lambda { |env|
368+
assert env['committee.path_hash']['integer'].is_a?(Integer)
369+
assert env['committee.params']['integer'].is_a?(Integer)
370+
[200, {}, []]
371+
}
372+
373+
@app = new_rack_app_with_lambda(check_parameter_string, schema: open_api_3_schema, coerce_path_params: true, coerce_query_params: false)
374+
get "/coerce_path_params/1"
375+
assert_equal 200, last_response.status
376+
end
377+
378+
it "does not coerce path params when path coercion is disabled even if query coercion is enabled" do
379+
@app = new_rack_app(schema: open_api_3_schema, coerce_path_params: false, coerce_query_params: true)
380+
get "/coerce_path_params/1"
381+
382+
assert_equal 400, last_response.status
383+
assert_match(/integer/i, last_response.body)
384+
end
385+
366386
describe "overwrite same parameter (old rule)" do
367387
# (high priority) path_hash_key -> request_body_hash -> query_param
368388
it "set query parameter to committee.params and query hash" do

test/schema_validator/open_api_3/operation_wrapper_test.rb

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,5 +171,39 @@ def operation_object
171171
assert_equal [], operation_object.request_content_types
172172
end
173173
end
174+
175+
describe 'coercion option wiring' do
176+
before do
177+
@path = '/coerce_path_params/1'
178+
@method = 'get'
179+
end
180+
181+
it 'uses coerce_path_params for explicit path coercion' do
182+
disabled = Committee::SchemaValidator::Option.new({ coerce_path_params: false, coerce_query_params: true }, open_api_3_schema, :open_api_3)
183+
enabled = Committee::SchemaValidator::Option.new({ coerce_path_params: true, coerce_query_params: false }, open_api_3_schema, :open_api_3)
184+
185+
error = assert_raises(Committee::InvalidRequest) do
186+
operation_object.coerce_path_parameter(disabled)
187+
end
188+
assert_match(/expected integer, but received String: "1"/i, error.message)
189+
190+
coerced = operation_object.coerce_path_parameter(enabled)
191+
assert_kind_of(Integer, coerced['integer'])
192+
end
193+
194+
it 'uses coerce_query_params for request parameter validation' do
195+
query_coercion_disabled = Committee::SchemaValidator::Option.new({ coerce_query_params: false }, open_api_3_schema, :open_api_3)
196+
query_coercion_enabled = Committee::SchemaValidator::Option.new({ coerce_query_params: true }, open_api_3_schema, :open_api_3)
197+
198+
error = assert_raises(Committee::InvalidRequest) do
199+
@path = '/characters'
200+
operation_object.validate_request_params({}, { "limit" => "1" }, {}, HEADER, query_coercion_disabled)
201+
end
202+
assert_match(/expected integer, but received String: "1"/i, error.message)
203+
204+
@path = '/characters'
205+
operation_object.validate_request_params({}, { "limit" => "1" }, {}, HEADER, query_coercion_enabled)
206+
end
207+
end
174208
end
175209
end

0 commit comments

Comments
 (0)