-
Notifications
You must be signed in to change notification settings - Fork 1.4k
fix: 🐛 request.url getter fails when using IPv6 and HTTP2 [#4560] #4559
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
base: master
Are you sure you want to change the base?
fix: 🐛 request.url getter fails when using IPv6 and HTTP2 [#4560] #4559
Conversation
3eb41e7 to
4f47a71
Compare
|
|
||
| const req = request.raw.req; | ||
| const host = req.headers.host ? req.headers.host.trim() : ''; | ||
| const host = (req.headers.host || req.headers[':authority'] || '').trim(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps we could test if the req is an IPv6 request somehow, and choose Host or :authority accordingly. WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that would be great and expect if we want this included. I've done some light digging and come up with the following to help this along:
https://github.com/hapijs/hapi/blob/master/lib/core.js#L334 listens for whatever host / address you pass it, you can set it to loopback :: to bind to ipv6
Test it using builtins that only request IPv6
const http = require('http');
const options = {
hostname: '::1', // The IPv6 loopback address of your local server
port: 3000,
path: '/',
method: 'GET',
family: 6 // Force IPv6
};
const req = http.request(options, (res) => {
console.log(`STATUS: ${res.statusCode}`);
res.setEncoding('utf8');
res.on('data', (chunk) => {
console.log(`BODY: ${chunk}`);
});
});
req.on('error', (e) => {
console.error(`problem with request: ${e.message}`);
});
req.end();
or alternatively, reuse some of the patterns in hapi tests: https://github.com/hapijs/hapi/blob/master/test/core.js#L925
The right place to add those tests would be https://github.com/hapijs/hapi/blob/master/test/core.js
Checkout some of the commentary around ipv6:
https://github.com/hapijs/hapi/blob/master/test/core.js#L102
https://github.com/hapijs/hapi/blob/master/test/core.js#L711
Seems like there's no explicit ipv6 tests, and they would need to be done only if the host machine has ipv6 enabled to prevent false positives.
Hope this helps!
…#242241) ## Summary Workarounds #236380 The official way to address this is with the following issue + PR on hapijs/hapi side: * hapijs/hapi#4560 * hapijs/hapi#4559 Until then, we can inject the Host information that Hapi relies on when it builds the `url` string.
…elastic#242241) ## Summary Workarounds elastic#236380 The official way to address this is with the following issue + PR on hapijs/hapi side: * hapijs/hapi#4560 * hapijs/hapi#4559 Until then, we can inject the Host information that Hapi relies on when it builds the `url` string. (cherry picked from commit 0d41ec2)
…elastic#242241) ## Summary Workarounds elastic#236380 The official way to address this is with the following issue + PR on hapijs/hapi side: * hapijs/hapi#4560 * hapijs/hapi#4559 Until then, we can inject the Host information that Hapi relies on when it builds the `url` string. (cherry picked from commit 0d41ec2)
…elastic#242241) ## Summary Workarounds elastic#236380 The official way to address this is with the following issue + PR on hapijs/hapi side: * hapijs/hapi#4560 * hapijs/hapi#4559 Until then, we can inject the Host information that Hapi relies on when it builds the `url` string. (cherry picked from commit 0d41ec2)
…elastic#242241) ## Summary Workarounds elastic#236380 The official way to address this is with the following issue + PR on hapijs/hapi side: * hapijs/hapi#4560 * hapijs/hapi#4559 Until then, we can inject the Host information that Hapi relies on when it builds the `url` string. (cherry picked from commit 0d41ec2)
Summary
Fixes #4560
Problem
In HTTP2 the
Host:header is no longer present. According to the spec:Host:header.Host:header is missing, so the request.url getter) uses thehost:portfrom the _core.info.::1), the getter ends up building an invalid URL, as the host name is not surrounded by square brackets. An IPv6 address like2001:db8::1:8080would be ambiguous, as8080could be interpreted as the last segment of the IP address rather than the port.Solution
The PR updates the request logic in 2 places:
:authorityifHostheader is NOT present.::1) in square brackets[::1].