fix: handle tags like debian/2%2.6-21 as identifiers #1336
Merged
Conversation
This file contains hidden or 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
JohnVillalovos
requested changes
Feb 26, 2021
gitlab/utils.py
Outdated
| @@ -55,15 +55,15 @@ def copy_dict(dest: Dict[str, Any], src: Dict[str, Any]) -> None: | |||
| dest[k] = v | |||
|
|
|||
|
|
|||
| def clean_str_id(id: str) -> str: | |||
| return id.replace("/", "%2F").replace("#", "%23") | |||
| def clean_str_id(id): | |||
There was a problem hiding this comment.
Can the type-hints not be removed?
There was a problem hiding this comment.
Uuuuh, absolutely yes, sorry. Fixing it now!
|
Overall looks good to me. Please put the type-hints back in. Also change "fix: Handle..." to "fix: handle" to fix the lint issue. Thanks. |
f213a90 to
41dc7bf
Compare
Codecov Report
@@ Coverage Diff @@
## master #1336 +/- ##
=======================================
Coverage 80.21% 80.21%
=======================================
Files 73 73
Lines 3801 3801
=======================================
Hits 3049 3049
Misses 752 752
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Git refnames are relatively free-form and can contain all sort for special characters, not just `/` and `#`, see http://git-scm.com/docs/git-check-ref-format In particular, Debian's DEP-14 standard for storing packaging in git repositories mandates the use of the `%` character in tags in some cases like `debian/2%2.6-21`. Unfortunately python-gitlab currently only escapes `/` to `%2F` and in some cases `#` to `%23`. This means that when using the commit API to retrieve information about the `debian/2%2.6-21` tag only the slash is escaped before being inserted in the URL path and the `%` is left untouched, resulting in something like `/api/v4/projects/123/repository/commits/debian%2F2%2.6-21`. When urllib3 seees that it detects the invalid `%` escape and then urlencodes the whole string, resulting in `/api/v4/projects/123/repository/commits/debian%252F2%252.6-21`, where the original `/` got escaped twice and produced `%252F`. To avoid the issue, fully urlencode identifiers and parameters to avoid the urllib3 auto-escaping in all cases. Signed-off-by: Emanuele Aina <emanuele.aina@collabora.com>
41dc7bf to
b4dac5c
Compare
JohnVillalovos
approved these changes
Mar 5, 2021
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.
Git refnames are relatively free-form and can contain all sort for
special characters, not just
/and#, seehttp://git-scm.com/docs/git-check-ref-format
In particular, Debian's DEP-14 standard for storing packaging in git
repositories mandates the use of the
%character in tags in somecases like
debian/2%2.6-21.Unfortunately python-gitlab currently only escapes
/to%2Fand insome cases
#to%23. This means that when using the commit API toretrieve information about the
debian/2%2.6-21tag only the slash isescaped before being inserted in the URL path and the
%is leftuntouched, resulting in something like
/api/v4/projects/123/repository/commits/debian%2F2%2.6-21. Whenurllib3 sees that, it detects the invalid
%escape and then urlencodesthe whole string, resulting in
/api/v4/projects/123/repository/commits/debian%252F2%252.6-21, wherethe original
/got escaped twice and produced%252F.To avoid the issue, fully urlencode identifiers and parameters to avoid
the urllib3 auto-escaping in all cases.
I've run the unit tests but not the integration ones, so in theory this does
not break anything but I can't really confirm it. All I can firmly say is that it
fixes the issue I hit. :D