diff --git a/lib/committee/schema_validator/open_api_3.rb b/lib/committee/schema_validator/open_api_3.rb index ef6548a9..3e3879e1 100644 --- a/lib/committee/schema_validator/open_api_3.rb +++ b/lib/committee/schema_validator/open_api_3.rb @@ -59,8 +59,9 @@ def link_exist? attr_reader :validator_option - def coerce_path_params - Committee::RequestUnpacker.indifferent_params(@operation_object.coerce_path_parameter(@validator_option)) + def coerce_path_params(deserializer) + path_params = deserializer&.deserialize_path_params(@operation_object.path_params) + Committee::RequestUnpacker.indifferent_params(@operation_object.coerce_path_parameter(@validator_option, path_params)) end def request_schema_validation(request) @@ -95,24 +96,19 @@ def request_unpack(request) request_param, _is_form_params = unpacker.unpack_request_params(request) request.env[validator_option.request_body_hash_key] = request_param - request.env[validator_option.path_hash_key] = coerce_path_params + deserializer = ParameterDeserializer.new(@operation_object.request_operation) if validator_option.deserialize_parameters + request.env[validator_option.path_hash_key] = coerce_path_params(deserializer) query_param = unpacker.unpack_query_params(request) query_param.merge!(request_param) if request.get? && validator_option.allow_get_body && request_param.is_a?(Hash) - if @operation_object && validator_option.deserialize_parameters - deserializer = ParameterDeserializer.new(@operation_object.request_operation) - + if deserializer query_values = validator_option.allow_query_params ? Rack::Utils.parse_query(request.query_string, '&') : {} # Do not restore query values that were overridden by the request body. query_values.reject! { |name, _| request.POST.key?(name) } if validator_option.allow_non_get_query_params query_values.reject! { |name, _| request_param.key?(name) } if request.get? && validator_option.allow_get_body query_param = deserializer.deserialize_query_params(query_param, query_values) - path_param = request.env[validator_option.path_hash_key] - path_param = deserializer.deserialize_path_params(path_param) - request.env[validator_option.path_hash_key] = path_param - headers = request.env[validator_option.headers_key] headers = deserializer.deserialize_headers(headers) request.env[validator_option.headers_key] = headers diff --git a/lib/committee/schema_validator/open_api_3/operation_wrapper.rb b/lib/committee/schema_validator/open_api_3/operation_wrapper.rb index 96b239df..6915cf24 100644 --- a/lib/committee/schema_validator/open_api_3/operation_wrapper.rb +++ b/lib/committee/schema_validator/open_api_3/operation_wrapper.rb @@ -21,9 +21,13 @@ def http_method request_operation.http_method end - def coerce_path_parameter(validator_option) + def coerce_path_parameter(validator_option, path_params = nil) options = build_openapi_parser_path_option(validator_option) - validated_path_params = request_operation.validate_path_params(options) + validated_path_params = if path_params + request_operation.operation_object.validate_path_params(path_params, options) + else + request_operation.validate_path_params(options) + end return {} unless options.coerce_value validated_path_params diff --git a/lib/committee/schema_validator/open_api_3/parameter_deserializer.rb b/lib/committee/schema_validator/open_api_3/parameter_deserializer.rb index e62db8cd..9d329155 100644 --- a/lib/committee/schema_validator/open_api_3/parameter_deserializer.rb +++ b/lib/committee/schema_validator/open_api_3/parameter_deserializer.rb @@ -55,6 +55,9 @@ def deserialize_params_by_location(raw_params, location, query_values = {}) result = Committee::Utils.indifferent_hash params_for_location = @parameters.select { |p| p.in == location } + if location == 'path' && @request_operation.respond_to?(:path_item) && @request_operation.path_item + params_for_location += (@request_operation.path_item.parameters || []).select { |p| p.in == 'path' } + end # If no parameters are defined for this location, return raw params as-is return raw_params if params_for_location.empty? @@ -452,9 +455,19 @@ def deserialize_matrix_style(param_def, raw_params, explode) case schema.type when 'array' if explode - # explode=true: ;id=3;id=4 (multiple occurrences) - # Rack should have already collected these into an array - Array(value) + if value.is_a?(String) + prefix = "#{param_name}=" + return [] if value == ";#{param_name}" + raise ArgumentError, 'invalid matrix array' unless value.start_with?(";#{prefix}") + + value.split(';', -1).drop(1).map do |part| + raise ArgumentError, 'invalid matrix array' unless part.start_with?(prefix) + + part.delete_prefix(prefix) + end + else + Array(value) + end else # explode=false: ;id=3,4,5 extract_matrix_array_compact(value, param_name) diff --git a/test/middleware/request_validation_open_api_3_test.rb b/test/middleware/request_validation_open_api_3_test.rb index 865357e9..ac13dbc6 100644 --- a/test/middleware/request_validation_open_api_3_test.rb +++ b/test/middleware/request_validation_open_api_3_test.rb @@ -475,6 +475,80 @@ def app assert_match(/integer/i, last_response.body) end + describe 'serialized path parameters' do + [ + ['simple', false, '1,2'], + ['label', false, '.1,2'], + ['label', true, '.1.2'], + ['matrix', false, ';ids=1,2'], + ['matrix', true, ';ids=1;ids=2'], + ].each do |style, explode, value| + it "deserializes and validates #{style} array with explode=#{explode}" do + parameter = { 'name' => 'ids', 'in' => 'path', 'required' => true, 'style' => style, 'explode' => explode, 'schema' => { 'type' => 'array', 'items' => { 'type' => 'integer' } }, } + check_parameter = lambda { |env| + assert_equal [1, 2], env['committee.path_hash']['ids'] + assert_equal [1, 2], env['committee.params']['ids'] + [200, {}, []] + } + @app = new_rack_app_with_lambda(check_parameter, schema: path_param_schema(parameter)) + + get "/paths/#{value}" + + assert_equal 200, last_response.status + end + end + + [ + ['simple', false, 'role,admin,status,active'], + ['simple', true, 'role=admin,status=active'], + ['label', false, '.role,admin,status,active'], + ['label', true, '.role=admin.status=active'], + ['matrix', false, ';filter=role,admin,status,active'], + ['matrix', true, ';role=admin;status=active'], + ].each do |style, explode, value| + it "deserializes and validates #{style} object with explode=#{explode}" do + parameter = { 'name' => 'filter', 'in' => 'path', 'required' => true, 'style' => style, 'explode' => explode, 'schema' => { 'type' => 'object', 'required' => ['role', 'status'], 'properties' => { 'role' => { 'type' => 'string' }, 'status' => { 'type' => 'string' } } }, } + check_parameter = lambda { |env| + assert_equal({ 'role' => 'admin', 'status' => 'active' }, env['committee.path_hash']['filter']) + [200, {}, []] + } + @app = new_rack_app_with_lambda(check_parameter, schema: path_param_schema(parameter)) + + get "/paths/#{value}" + + assert_equal 200, last_response.status + end + end + + it 'rejects an invalid item after deserialization' do + parameter = { 'name' => 'ids', 'in' => 'path', 'required' => true, 'style' => 'simple', 'schema' => { 'type' => 'array', 'items' => { 'type' => 'integer' } }, } + @app = new_rack_app(schema: path_param_schema(parameter)) + + get '/paths/1,invalid' + + assert_equal 400, last_response.status + assert_match(/integer/, last_response.body) + end + + it 'deserializes a path item parameter before validation' do + parameter = { 'name' => 'ids', 'in' => 'path', 'required' => true, 'schema' => { 'type' => 'array', 'items' => { 'type' => 'integer' } }, } + document = query_param_document(parameter) + path_item = document['paths'].delete('/events') + path_item['parameters'] = path_item['get'].delete('parameters') + document['paths']['/paths/{ids}'] = path_item + schema = Committee::Drivers.load_from_data(document, nil, parser_options: { strict_reference_validation: true }) + check_parameter = lambda { |env| + assert_equal [1, 2], env['committee.path_hash']['ids'] + [200, {}, []] + } + @app = new_rack_app_with_lambda(check_parameter, schema: schema) + + get '/paths/1,2' + + assert_equal 200, last_response.status + end + end + describe "overwrite same parameter (old rule)" do # (high priority) path_hash_key -> request_body_hash -> query_param it "set query parameter to committee.params and query hash" do @@ -977,6 +1051,12 @@ def array_body_document } end + def path_param_schema(parameter) + document = query_param_document(parameter) + document['paths']["/paths/{#{parameter['name']}}"] = document['paths'].delete('/events') + Committee::Drivers.load_from_data(document, nil, parser_options: { strict_reference_validation: true }) + end + def bracket_notation_query_parameter { 'name' => 'filter[slug]', diff --git a/test/schema_validator/open_api_3/parameter_deserializer_test.rb b/test/schema_validator/open_api_3/parameter_deserializer_test.rb index aa329e84..da535ad4 100644 --- a/test/schema_validator/open_api_3/parameter_deserializer_test.rb +++ b/test/schema_validator/open_api_3/parameter_deserializer_test.rb @@ -186,6 +186,30 @@ assert_equal({ 'ids' => ['1', '2', '3'] }, result) end + it 'deserializes repeated matrix path segments into an array' do + raw_params = { 'ids' => ';ids=1;ids=2' } + param = create_param('ids', 'path', 'matrix', true, 'array', 'integer') + deserializer = create_deserializer([param]) + + assert_equal({ 'ids' => ['1', '2'] }, deserializer.deserialize_path_params(raw_params)) + end + + it 'deserializes an empty matrix array' do + param = create_param('ids', 'path', 'matrix', true, 'array', 'integer') + deserializer = create_deserializer([param]) + + assert_equal({ 'ids' => [] }, deserializer.deserialize_path_params('ids' => ';ids')) + end + + it 'rejects other names inside an exploded matrix array' do + param = create_param('ids', 'path', 'matrix', true, 'array', 'string') + deserializer = create_deserializer([param]) + + assert_raises(Committee::ParameterDeserializationError) do + deserializer.deserialize_path_params('ids' => ';ids=1;other=2') + end + end + it 'deserializes matrix style object with explode=false' do raw_params = { 'filter' => ';filter=role,admin,status,active' } param = create_param('filter', 'path', 'matrix', false, 'object', { 'role' => 'string', 'status' => 'string' })