Remove deprecated URI.escape #993
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
letaniaferreira
approved these changes
Aug 28, 2020
| # | ||
| # URI.encode_www_form follows the spec at https://url.spec.whatwg.org/#concept-urlencoded-serializer | ||
| # and percent encodes square brackets. Here we change them back. | ||
| query.gsub('%5B', '[').gsub('%5D', ']') |
There was a problem hiding this comment.
Simple but interesting stuff. I didn't know about gsub before.
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 of the change
Removes the deprecated
URI.escape, which has been deprecated for a long time, but the warnings are now more visible in Ruby 2.7.The previous code had ended up performing several transforms in order to unencode square brackets while keeping everything else correctly encoded. Condensed, it was effectively like this:
URI.escape(CGI.unescape(URI.encode_www_form(params)))This is all necessary because
URI.encode_www_formpercent encodes square brackets, but we want them unencoded.This PR refactors and takes a simpler approach. As a side note, the debate about whether unencoded brackets should be allowed in URLs is not as relevant here since these are only used as debugging data in Rollbar payloads. They aren't being used to perform actual http requests.
The original URL may or may not have been percent encoded. The previous code didn't preserve the original state, and neither does this PR. The unencoded brackets are more readable, and is what everyone has been expecting and getting in their Rollbar dashboard, so we keep it that way.
Type of change
Related issues
Fixes: #989
ch71576
Development
Code review