Make path param regex more strict #83
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…' into remove-legacy-ruby-rails-support
…/bwillis/versioncake into remove-legacy-ruby-rails-support
…upport Removes legacy Ruby/Rails support
…versioncake into bump-for-security-fixes
Bumping for security patches
Adding minimum Rails support to 5 and upgrading security patches
Bumps [actionview](https://github.com/rails/rails) from 5.1.4 to 5.2.4.1. - [Release notes](https://github.com/rails/rails/releases) - [Changelog](https://github.com/rails/rails/blob/v6.0.2.1/actionview/CHANGELOG.md) - [Commits](rails/rails@v5.1.4...v5.2.4.1) Signed-off-by: dependabot[bot] <[email protected]>
…iew-5.2.4.1 Bump actionview from 5.1.4 to 5.2.4.1
|
Nice investigation and thanks for fixing! I've just resolved some long outstanding CI pipeline issues and rebased so hopefully CI passes now! |
Fix Rails 5.2 support and add Rails 5 2 specs
Support for Rails 6
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
In upgrading from 2.6 to 3.4, we were noticing some mysterious behavior in our VersionCake-managed API. Most requests to the API were fine, but a small, seemingly random subset were having their version parsed as
0, resulting in anUnsupportedVersionError.The format of our API paths includes encoded parameters, which look like
/api/resource/ynv0-00c5-96ua?api_version=5. It turns out that the path parameter extraction strategy was yanking thev0out ofynv0-00c5-96uaand interpreting that as the API version. To resolve this in our app, I replaced:path_parameterwith a near-identical custom one, except with more strict regex - end-of-string anchors on either side of the version to be extracted.That a path parameter version will always take the format of
/\Av\d+\z/seems like the original intent of the strategy and a reasonable constraint on its use, such that it should be part of the builtin.Changes