http icon indicating copy to clipboard operation
http copied to clipboard

Allow undescore character in Uri host

Open lulhum opened this issue 1 year ago • 4 comments

I don't understand why this validity check has been added. Has per rfc 2181 (https://datatracker.ietf.org/doc/html/rfc2181#section-11), underscore are valid character to use in an uri host.

For my specific usage, it broke for requests using docker internal hostnames.

lulhum avatar Apr 22 '24 11:04 lulhum

@lulhum Thanks for applying these changes :+1:

Looks good to me so far. As a last step, can you squash the two commits into one? I don't think we need two separate ones as they're related to each other.

SimonFrings avatar Apr 26 '24 13:04 SimonFrings

@SimonFrings If @lulhum is not responding on the squash in her fork is there anything we can do to speed the issue up? I also ran into this issue where our production containers have underscores in the host name where I needed to patch it.

As the fix is not that complex, but can be quite impactful I believe we should not wait for the reply for this long.

robertragas avatar Aug 17 '24 10:08 robertragas

@SimonFrings Sorry, I missed the squash request, thanks for the reminder @robertragas

lulhum avatar Aug 18 '24 09:08 lulhum

@lulhum Thanks for the feedback on the URI host validation check. It appears there may have been some confusion around where underscores are allowed in URIs. As the maintainer who introduced that change in #521 recently, let me try to add some context.

You're correct that per RFC 2181, underscores are valid characters to use in DNS labels, which form the hostname component of a URI. However, the hostname component of a URI is subject to stricter requirements, as outlined in RFC 1123 and RFC 952. These RFCs state that hostnames must consist of alphanumeric characters and hyphens, and underscores are not permitted (see e.g. https://en.wikipedia.org/wiki/Hostname#Syntax).

I can certainly understand the frustration if this broke functionality for your specific use case with Docker internal hostnames. Early versions of Docker Compose did indeed use underscores in container names, but the more recent v2.0 release (released 2021) switched to using hyphens instead, following the hostname restrictions (https://github.com/docker/compose/issues/472).

While some other projects may be more lenient and accept underscores in hostnames, the RFCs are clear that this is not the correct behavior. That said, I'm open to further discussion on the potential impact and whether a more flexible approach could be warranted. Perhaps there are ways to strike a balance between strict RFC compliance and practical usability while keeping security in mind (https://claroty.com/team82/research/exploiting-url-parsing-confusion).

What do you think would be the best path forward here? I'm open to suggestions on how to balance strict adherence to the standards with practical considerations for users. I'm happy to discuss further, so please feel free to share any additional thoughts or suggestions you may have.

clue avatar Aug 29 '24 13:08 clue

@lulhum Let's move forward with this one! :shipit: I've only fixed a minor typo, rebased your changes and added another test, changes look good to me otherwise as discussed above, so let's get this shipped! Keep it up! :+1:

clue avatar Nov 19 '24 22:11 clue