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

URL.path: don't unquote slashes as this corrupts the path #1406

Closed
wants to merge 1 commit into from

Conversation

vdwees
Copy link

@vdwees vdwees commented Dec 1, 2020

This PR addresses issue #1405

Using unquote in a blanket way on the path can result in corrupting the path if the path contains encoded slashes. This change has some precedent in https://docs.python.org/3/library/urllib.parse.html#urllib.parse.quote, where the default behaviour is to treat the slash as a special case.

Personally, I am not sure URL.path should be doing unquote at all. My preference would be to drop the unquote behaviour entirely. Nevertheless, with this PR at least URL.path does not corrupt paths.

I'd be happy to write some tests if this approach is approved.

@vdwees
Copy link
Author

vdwees commented Dec 2, 2020

I think this change is ugly, and #1407 solves #1405 much better, so I am closing it. However, I still am concerned that if URL.path was accidentally used for constructing paths within httpx itself, wouldn't users be likely to make the mistake also?

@vdwees vdwees closed this Dec 2, 2020
@tomchristie
Copy link
Member

We're very careful and deliberate around our URL API.

.path returns a string, without the query, and with any percent-encoded characters being URL escaped. That'll give you the same behaviour you'd typically see within a web application framework.

.raw_path returns full raw bytes, of the entire path (including any query string component.) No url escaping. No nonsense. This is what gets used for the TARGET part when constructing an HTTP request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants