-
-
Notifications
You must be signed in to change notification settings - Fork 404
fixed urlencode for repo name in /put #1508
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
base: master
Are you sure you want to change the base?
Conversation
|
Would this also apply to all other methods receiving a 'name' param ? |
|
Can't say it for sure. Other methods do not have this problem at all, acting like there is no url encode, as stated in the issue |
|
@theoborealis could you wanna add your self to the AUTHORS file ? also we should add a little system test to keep the coverage. you could also allow edit access to the maintainers for this PR, then I can help with the testing... |
|
turns out, there is currently no test for this API call: hence the 0% patch coverage... -> we should add a test in system/t12_api/repos.py ... |
|
rebased on latest master... |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1508 +/- ##
==========================================
- Coverage 76.94% 76.73% -0.22%
==========================================
Files 160 160
Lines 14727 14733 +6
==========================================
- Hits 11332 11305 -27
- Misses 2264 2296 +32
- Partials 1131 1132 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
and path escape the new name param
|
Hi @theoborealis, I have added a test for repoEdit, coverage is up a bit :) then I looked a bit further into the change, and think the name field from the URL is already escaped, like all the other url parameters in other functions. what was not escaped, is the optional new name to set was not escaped. I think the current changes solve the problem, could yu give it a try and confirm ? |
|
Hi @theoborealis, I am still not quite clear what we are fixing here.. could you provide an example of what is not working in details ? |
Fixes #1507
Description of the Change
Changes wrong comparison taking url encode into account
Checklist
AUTHORS