Skip to content

Comments

Do not IDNA-encode IPv6 literal hosts in to_uri() (#188)#189

Open
mvaught02 wants to merge 1 commit intopython-hyper:masterfrom
mvaught02:ipv6-literal-host-fix
Open

Do not IDNA-encode IPv6 literal hosts in to_uri() (#188)#189
mvaught02 wants to merge 1 commit intopython-hyper:masterfrom
mvaught02:ipv6-literal-host-fix

Conversation

@mvaught02
Copy link

Summary

This PR fixes a long-standing bug in hyperlink.URL.to_uri() where plain IPv6 literal hosts (e.g., [::1]) are incorrectly passed to idna.encode().

This bug was originally reported in #68 and #188 and affects downstream users (e.g., treq) even on current releases.

@mvaught02
Copy link
Author

@wsanchez / @glyph can you give this one a look?

@glyph
Copy link
Collaborator

glyph commented Feb 21, 2026

ugh, looks like our CI needs some updates first, to catch up with more recent pythons and linuxes

@mvaught02
Copy link
Author

Hmm, I can probably tackle this a couple of ways.

When I opened this PR, I was trying to keep it low-touch in hopes we could get a quick release with the fix without dragging in a bunch of unrelated changes. I can stick with that theme and just “make it work” — update the Actions config to use a modern Python for the checks and switch the legacy test versions over to containers so they still run. I’m happy to wire that up in this PR.

Or, if it makes more sense, I can either expand this PR to uplift the project to currently supported Python versions, or open a separate PR to handle that cleanup first and then we can merge this one afterward.

I’m good either way — just let me know what you’d prefer and I can take care of it. At a glance this doesn’t seem like a major lift. Mostly just trying to get this fix in since it keeps coming back to annoy me 😅

@glyph
Copy link
Collaborator

glyph commented Feb 21, 2026

Or, if it makes more sense, I can either expand this PR to uplift the project to currently supported Python versions, or open a separate PR to handle that cleanup first and then we can merge this one afterward.

A separate PR for CI cleanup would be ideal. But this change is relatively small and I think uncontroversial so if you preferred to clean it up here I wouldn't complain too much.

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