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

fix(request): if host is ipv6 address, enclose it in square brackets #92

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

elrafoon
Copy link

@elrafoon elrafoon commented Oct 8, 2024

Hello,

related RFC requires it this way, and also at least apache would reject non-bracketed ipv6 host address with Bad Request.

I don't know any better way how to do this, except for modifying the nourl parser to return hostname type (ipv4, ipv6, domain name).

Also see corresponding work here:
rmja/nourl#2

Related RFC requires it this way, and also at least apache would reject
non-bracketed ipv6 host address with Bad Request.
@rmja
Copy link
Member

rmja commented Oct 9, 2024

Hi,

Is it not possible to simply let the brackets be part of the host value? Or is that incorrect?

@elrafoon
Copy link
Author

elrafoon commented Oct 9, 2024

That host value would be fed into underlying impl embedded_nal_aync::Dns dns resolver, that in my opinion would not accept square-bracketed IPv6 addresses, because no internet standard (RFCs) requires it to do so.

When looking into sources of core::net::Ipv6Addr, it seems it would fail parsing bracketed ipv6 address.

So this is not an option IMHO.

@elrafoon
Copy link
Author

elrafoon commented Oct 9, 2024

Other option would be to extend nourl to return some enum with resolved host type - domain name / ipv4 / ipv6 - and use that information.

@rmja
Copy link
Member

rmja commented Oct 14, 2024

Year, that is probably a better idea. It seems wrong to re-parse the host, only to determine if it is an ipv6 host.

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