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.

Sign up for GitHub

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

Jump to bottom

URL validation fails when absolute path contains a colon #1076

Closed
whickman opened this issue Dec 9, 2020 · 0 comments · Fixed by #2360
Closed

URL validation fails when absolute path contains a colon #1076

whickman opened this issue Dec 9, 2020 · 0 comments · Fixed by #2360

Comments

@whickman
Copy link

whickman commented Dec 9, 2020 •
edited by pkoenig10
Loading

What happened?

Dialogue will fail to initialize a client for a URL of the form https://example.com:443/foo.bar:1/baz, due to the colon character, however https://tools.ietf.org/html/rfc3986#section-3.3 indicates this should be allowed as long as the path is not a relative one, which these are not. RFC is a bit tough to parse, but I checked with some other URL validators and this form appears valid. It also works for go clients.

issue is in this fn

dialogue/dialogue-core/src/main/java/com/palantir/dialogue/core/BaseUrl.java

Line 216 in 7d12856

static class UrlEncoder {

What did you want to happen?

allow : in URL paths
@sixinli sixinli mentioned this issue Jun 4, 2021
Closed
carterkozak added a commit that referenced this issue Sep 23, 2024
@carterkozak
fix #1076: Dialogue allows colons in http paths and query parameters
0340bcb 
This is required by some GCP APIs, where encoded colons are not
respected.
[rfc3986 section 3.3](https://datatracker.ietf.org/doc/html/rfc3986#section-3.3)
clearly allows colon as a pchar, however not all servers are necessarily fully
compliant with the rfc. We've had issues with specific sub-delimiters
in the past. This shouldn't be terribly risky, esepcially given this
implementaiton has never allowed unencoded colons before, and we have
no evidence to suggest they should be problematic.
@carterkozak carterkozak mentioned this issue Sep 23, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix #1076: Dialogue allows colons in http paths and query parameters palantir/dialogue
1 participant
@whickman