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-decode whole hostname when appropriate #67

Closed
wants to merge 1 commit into from

Conversation

jdp
Copy link

@jdp jdp commented Jun 9, 2016

Adds support for other special characters in UNIX socket path
names, like the colons in Cloud SQL paths.

See also: https://github.com/kennethreitz/dj-database-url/issues/66

Adds support for other special characters in UNIX socket path
names, like the colons in Cloud SQL paths.

See also:  https://github.com/kennethreitz/dj-database-url/issues/66
@kennethreitz
Copy link
Collaborator

This seems harmless....

@kennethreitz
Copy link
Collaborator

If anyone else can comment on this, would be much appreciated.

@kennethreitz
Copy link
Collaborator

If there's anything I've learned from Requests, it's never touch URL encoding unless you know you need to.

@benwilber
Copy link

This patch looks like the correct thing to do.

@jdp
Copy link
Author

jdp commented Jun 23, 2016

Percent-encoding is allowed in URI hosts per RFC 3986, provided that the host is intended to be interpreted as UTF-8:

reg-name = *( unreserved / pct-encoded / sub-delims )
...
URI producing applications must not use percent-encoding in host unless it is used to represent a UTF-8 character sequence.

The newer URL Standard tightens up the character restrictions in hosts in URL's though. Hosts can still be percent-encoded, but fail to parse if any restricted characters are still present after decoding:

If asciiDomain contains U+0000, U+0009, U+000A, U+000D, U+0020, "#", "%", "/", ":", "?", "@", "[", "", or "]", syntax violation, return failure.

I wouldn't be against changing the URL format to cloudsql:/cloudsql/project-id:instance-id?username=&password=&database= or something.

@kennethreitz
Copy link
Collaborator

needs a rebase!

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.

3 participants