Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Open
FelixMartel wants to merge8 commits intozaproxy:main
base:main
Choose a base branch
Loading
fromFelixMartel:main

Conversation

@FelixMartel
Copy link

@FelixMartelFelixMartel commentedMar 7, 2024
edited
Loading

Currently proxying IPv6 reference URIs is not possible#3705. This is becauseHttpRequestHeader.parseURI first attempts to repair URIs containing unencoded characters by encoding them before callingnew URI withencoded=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 toHttpRequestHeader while keeping the URI repairing behavior the same. Only callingnew URI withencoded=false gives a different result (for example % becomes %25) and so it is only used to detect IPv6 reference URIs and parse out the host:

  • First callnew URI withencoded=false.
  • If this was an IPv6 reference then apply repairing to the rest of URI.
  • Otherwise repair the whole URI.
  • Finally callnew URI withencoded=true.

@github-actions
Copy link

github-actionsbot commentedMar 7, 2024
edited
Loading

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@thc202
Copy link
Member

Better accept the CLA before continuing with more work, without that the changes will not be merged.

@thc202
Copy link
Member

This will require tests (a lot of them).

FelixMartel reacted with thumbs up emoji

@FelixMartel
Copy link
Author

I have read the CLA Document and I hereby sign the CLA

@FelixMartel

This comment has been minimized.

zapbot added a commit to zaproxy/cla that referenced this pull requestMar 8, 2024
}

privatestaticStream<Arguments>ipv6referenceUrlHostPairs() {
List<Arguments>urls =newArrayList<>();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Could useList.of

Copy link
Member

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
Copy link
Member

Seems to have broken binary compatability

@kingthorin
Copy link
Member

kingthorin commentedMar 9, 2024
edited
Loading

Does <username>:<password>@<host> parse as expected if password has square brackets?

@FelixMartel
Copy link
Author

FelixMartel commentedMar 9, 2024
edited
Loading

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 reacted with thumbs up emoji

@kingthorin
Copy link
Member

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.

FelixMartel reacted with thumbs up emoji

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@kingthorinkingthorinkingthorin left review comments

At least 2 approving reviews are required to merge this pull request.

Assignees

No one assigned

Labels

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@FelixMartel@thc202@kingthorin

[8]ページ先頭

©2009-2025 Movatter.jp