- Notifications
You must be signed in to change notification settings - Fork2.5k
Prevent encoding of [] in the host part of IPv6 reference URIs#8390
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
github-actionsbot commentedMar 7, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
thc202 commentedMar 8, 2024
Better accept the CLA before continuing with more work, without that the changes will not be merged. |
thc202 commentedMar 8, 2024
This will require tests (a lot of them). |
FelixMartel commentedMar 8, 2024
I have read the CLA Document and I hereby sign the CLA |
This comment has been minimized.
This comment has been minimized.
| } | ||
| privatestaticStream<Arguments>ipv6referenceUrlHostPairs() { | ||
| List<Arguments>urls =newArrayList<>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Could useList.of
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Or even Stream.of I think, should be other examples in the repo
kingthorin commentedMar 9, 2024
Seems to have broken binary compatability |
kingthorin commentedMar 9, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Does <username>:<password>@<host> parse as expected if password has square brackets? |
FelixMartel commentedMar 9, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Looks like it behaves the same as it does now when given asdf:[]@example.com. In practice I don't think clients supply credentials there. |
kingthorin commentedMar 9, 2024
They "shouldn't", but it's 'allowed' by spec and browsers so it'd be good to add a test just to ensure that nothing breaks in the future, for my two cents. |
Uh oh!
There was an error while loading.Please reload this page.
Currently proxying IPv6 reference URIs is not possible#3705. This is because
HttpRequestHeader.parseURIfirst attempts to repair URIs containing unencoded characters by encoding them before callingnew URIwithencoded=true. Otherwise new URI would throw a URIException. This includes the[]characters that are part of the IPv6 reference section and sonew URI(http://[::1]/)becomesnew URI(http://%5B::1%5C).Here I want to avoid having to add parsing of the authority section to
HttpRequestHeaderwhile keeping the URI repairing behavior the same. Only callingnew URIwithencoded=falsegives a different result (for example % becomes %25) and so it is only used to detect IPv6 reference URIs and parse out the host:new URIwithencoded=false.new URIwithencoded=true.