fix: avoid double-escaping in url_escape #30
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
plobsing
force-pushed
the
patch-1
branch
2 times, most recently
from
52378a8
to
8a9e26d
Compare
Because the escapes for other characters will put `%` into the output text, we need to perform replacements for `%` first. This ensures that all chars we replace this way were from the input text and not put there by some prior replacement step. The prior implementation would produce double-escaped values for the following characters: | Input char | Double-escaped | Fixed | | ---------- | -------------- | ------- | | `" "` | `"%2520"` | `"%20"` | | `"!"` | `"%2521"` | `"%21"` | | `'"'` | `"%2522"` | `"%22"` | | `"#"` | `"%2523"` | `"%23"` | | `"$"` | `"%2524"` | `"%24"` |
imjasonh
approved these changes
Nov 30, 2023
plobsing
added a commit
to plobsing/bazel-lib
that referenced
this pull request
Nov 22, 2024
This pair of utilities from Python's core are helpful when encoding or escaping strings. Unlike the common alternative — repeated application of `str.replace` — a `str.translate` implementation performs its work in a single pass. This isn't principally about efficiency — although a single-pass implementation may be more efficient — but rather about correctness. Doing all the translation in a single pass sidesteps the issue of double-encoding errors which are possible under repeated-processing schemes when when substitution input/output aliasing is present (i.e. some substitutions produce output that other substitutions recognize as to-be-replaced input). See chainguard-dev/rules_apko#30 for an concrete example of a double-encoding issue resulting from a repeated-processing translation implementation.
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.
Because the escapes for other characters will put
%into the output text, we need to perform replacements for%first. This ensures that all chars we replace this way were from the input text and not put there by some prior replacement step.The prior implementation would produce double-escaped values for the following characters:
" ""%2520""%20""!""%2521""%21"'"'"%2522""%22""#""%2523""%23""$""%2524""%24"