fix(url): normalizeHost with ipv6 hostname#201
Conversation
pimterry
left a comment
There was a problem hiding this comment.
Core change here all looks good to me! Thanks @chimurai, and great to hear you're finding Mockttp useful in http-proxy-middleware.
There is a broken test here though - not really for a good reason, but an example test value that was used as an invalid hostname in client-error events is now detected as IPv6 and gets square brackets inserted in the error report 😆. Can you have a look?
This isn't a valid IPv6 hostname, so I'd guess our regex is too relaxed there. If there's a quick safe change we can make to tighten that up then great, or if not we can just change the test data to something less ambiguous. The specific parsing behaviour for totally broken input like this isn't guaranteed (or really even guaranteeable) so as long as the test ends up passing and overall behaviour is sensible then it's not a big a deal.
|
Updated the test with less ambiguous invalid ipv6 address: -`a:1:2`
+`a^1:2`Tried different popular IPv6 regex pattern without success. Also tried using the already installed ipaddr.js dependency:
|
|
Merged, thanks @chimurai! I'll release as v4.4.2 now. |
|
Thanks for the quick reply and support 🙏 |
Hi,
Been using
mockttpfor a long time inhttp-proxy-middlewareto mock the proxy target server.Great work on this library 🙏
Recently I updated
httpxy(dependency ofhttp-proxy-middleware) with improved IPv6 support: unjs/httpxy#136After updating
httpxysome of existing IPv6 tests started to fail: https://github.com/chimurai/http-proxy-middleware/blob/ffbf5bd59712704c9df178d5df61d96062a11b22/test/e2e/ipv6.spec.tsThese tests are using
mockttpwith as target server.I applied the fixes from this PR in this draft PR: chimurai/http-proxy-middleware#1234
Looks like it fixes the issue.
For full transparency, I used Copilot to help with analysis and creating this PR and the description below.
Summary
Fix IPv6 host serialization when reconstructing absolute request URLs from parsed destination data.
Mockttp already parses bracketed IPv6 Host headers correctly, but when rebuilding
host[:port]strings it was dropping the brackets. For requests like Host:[::1]:8000, that produced invalid absolute URLs such ashttp://::1:8000/api, which fail URL parsing and trigger a400before request handlers run.Changes
:80for HTTP and:443for HTTPS are still omitted[::1]:8000Example
Before:
Host:
[::1]:8000reconstructed URL:
http://::1:8000/apiURL parsing fails, request returns
400After:
Host:
[::1]:8000reconstructed URL:
http://[::1]:8000/apirequest is processed normally