Skip to content
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

Redundant percent-encoding when building url query string #4291

Open
joemull opened this issue Jun 24, 2024 · 0 comments
Open

Redundant percent-encoding when building url query string #4291

joemull opened this issue Jun 24, 2024 · 0 comments
Assignees
Labels
bug Something's not working

Comments

@joemull
Copy link
Member

joemull commented Jun 24, 2024

Problem

The dictionary that is passed into utils.logic.build_url is sent through urllib.parse.urlencode and then also through urllib.parse.quote_plus.

janeway/src/utils/logic.py

Lines 153 to 154 in 9251796

if query:
query = quote_plus(urlencode(query))

This is redundant and results in unusable query strings. Compare:

In [3]: quote_plus(urlencode({'a': 'b', 'x': 'y', 'next': '/target/page/?d=e&f=g'}))
Out[3]: 'a%3Db%26x%3Dy%26next%3D%252Ftarget%252Fpage%252F%253Fd%253De%2526f%253Dg'
In [4]: urlencode({'a': 'b', 'x': 'y', 'next': '/target/page/?d=e&f=g'})
Out[4]: 'a=b&x=y&next=%2Ftarget%2Fpage%2F%3Fd%3De%26f%3Dg'

I believe we just want urlencode here.

If a URL is needed for inclusion in another URL, as a redirect callback, like with the ORCID API, then the function that builds the ORCID URL (utils.orcid.build_redirect_uri) should separately quote or escape the full URL that is returned from build_url.

Proposed solution

Allow a Django QueryDict, a plain dict, or a query string that already has percent-encoded values to be passed to the function. URL-encode the first two with '/' marked safe, to match the default behavior of the |urlencode template filter. For query strings, do nothing except pass it on to SplitResult.

@joemull joemull added the bug Something's not working label Jun 24, 2024
@joemull joemull self-assigned this Jun 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something's not working
Projects
None yet
Development

No branches or pull requests

1 participant