Skip to content
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.

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

Fix encoding of query params when openapi is v3 #671

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

HiroNonoyama
Copy link

@HiroNonoyama HiroNonoyama commented Aug 7, 2023

Problem

URL encoding is not processed correctly when I define a string query parameter as { schema: { type: :string } } in OpenAPI v3. This issue occurs even if I define the schema as simply { type: :string } when I have multiple test cases. This is because { type: :string } is automatically converted to { schema: { type: :string } } after one test case finishes and before the next one starts.

Solution

Update build_query_string_part and handle query parameter defined as { schema: { type: :string } }

This concerns this parts of the OpenAPI Specification:

The changes I made are compatible with:

  • OAS2
  • OAS3
  • OAS3.1

Related Issues

#606 (comment)

Checklist

  • Added tests
  • Changelog updated
    I didn't update readme because this is bug fix

Steps to Test or Reproduce

Add a query parameter with { schema: { type: :string } } that is in datetime format. Before this PR, the '+' symbol in the parameters would be replaced with a whitespace after encoding.

@HiroNonoyama HiroNonoyama marked this pull request as ready for review August 7, 2023 08:41
@NebojsaStrbac997
Copy link
Contributor

@romanblanco Checkout this PR, we had a discussion here

@romanblanco romanblanco added this to the Gem 3.0.0 milestone Aug 17, 2023
@@ -173,6 +173,8 @@ def build_query_string_part(param, value, swagger_doc)
end
return "#{escaped_name}=" + value.to_a.flatten.map{|v| CGI.escape(v.to_s) }.join(separator)
end
when :string
return "#{escaped_name}=#{CGI.escape(value.to_s)}"
else
return "#{name}=#{value}"

Choose a reason for hiding this comment

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

Why not just change this line instead? That way you'll never be adding unescaped values.

@BrianHawley
Copy link

#725 fixes the same bug but does the change I suggested instead. It should be used instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants