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 RequestFactory gsub for base_path_from_servers #591

Merged
merged 5 commits into from
Mar 18, 2023
Merged

Fix RequestFactory gsub for base_path_from_servers #591

merged 5 commits into from
Mar 18, 2023

Conversation

saturnflyer
Copy link
Contributor

@saturnflyer saturnflyer commented Jan 5, 2023

Problem

Commit 2264b03 introduced a change to this method where the gsub replacement of the server url switched from the global Regexp match variable of $1 to the block argument for gsub.

This change made it so that the argument to the variable name lookup include the braces used in the string for the url variable.

That lead to the hash not returing any matching keys and values and the generated URI string of '://' being passed to the URI() method which raise an error.

Solution

The solution is to use tr to replace matching { and } characters from the block argument passed to gsub! use the global Regex $1 match variable in Rswag::Specs::RequestFactory#base_path_from_servers

This concerns this parts of the Open API Specification:

The changes I made are compatible with:

  • OAS2
  • OAS3
  • OAS3.1

Related Issues

None

Checklist

  • Added tests
  • Changelog updated
  • Added documentation to README.md

Steps to Test or Reproduce

The configuration in the spec was updated to include a protocol value in the server variables with a :default key. Only this change is necessary to trigger an Invalid URI error.

saturnflyer and others added 2 commits March 14, 2023 15:06
Commit 2264b03 introduced a change to this method
where the gsub replacement of the server url switched from the global Regexp match
variable of \$1 to the block argument for gsub.
This change made it so that the argument to the variable name lookup include the
braces used in the string for the url variable.
That lead to the hash not returing any matching keys and values and the generated
URI string of '://' being passed to the URI() method which raise an error.
Co-authored-by: Julien Girard <julien09girard@gmail.com>
Copy link
Member

@romanblanco romanblanco left a comment

Choose a reason for hiding this comment

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

@saturnflyer Thank you for fixing it!

Please, just update the CHANGELOG.md with the PR number and document the change in README.md examples. Than it's good to be merged 👍

CHANGELOG.md Outdated Show resolved Hide resolved
@romanblanco
Copy link
Member

A change in test-app would also be nice for demonstration.

@romanblanco
Copy link
Member

Fixes #597
Fixes #601

saturnflyer and others added 2 commits March 18, 2023 11:15
Co-authored-by: Roman Blanco <1187051+romanblanco@users.noreply.github.com>
Copy link
Member

@romanblanco romanblanco left a comment

Choose a reason for hiding this comment

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

LGTM 👍, Thank you, @saturnflyer!

@romanblanco romanblanco merged commit 6bc733a into rswag:master Mar 18, 2023
@romanblanco romanblanco added this to the Gem 2.X.0 milestone Mar 18, 2023
@romanblanco romanblanco removed this from the Gem 2.10.0 milestone May 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants