Allow IPv6 loopback address #907
Conversation
|
Hi! I made these changes on github so I hope they work. 😅 |
lib/Server.js
Outdated
|
|
||
| // always allow localhost host, for convience | ||
| if(hostname === "127.0.0.1" || hostname === "localhost") return true; | ||
| if(hostname === "127.0.0.1" || hostname === "localhost" || hostname === "::1") return true; | ||
|
|
||
| // allow hostname of listening adress | ||
| if(hostname === this.listenHostname) return true; | ||
|
|
||
| // also allow public hostname if provided | ||
| if(typeof this.publicHost === "string") { |
There was a problem hiding this comment.
It would be great if publicHost could be an array so we could provide an IPv6 and IPv4 address.
|
@Timer looks like there are some issues with the tests to resolve before we can merge this. Please have a look |
Timer
force-pushed
the
patch-1
branch
2 times, most recently
from
e3a42e0 to
f2cbd25
Compare
Codecov Report
@@ Coverage Diff @@
## master #907 +/- ##
==========================================
- Coverage 72.23% 72.11% -0.13%
==========================================
Files 4 4
Lines 461 459 -2
Branches 138 136 -2
==========================================
- Hits 333 331 -2
Misses 128 128
Continue to review full report at Codecov.
|
| @@ -489,7 +485,7 @@ Server.prototype.listen = function(port, hostname) { | |||
| }); | |||
| sockServer.on("connection", (conn) => { | |||
| if(!conn) return; | |||
| if(!this.checkHost(conn.headers)) { | |||
| if(!this.checkHost(conn.hostname)) { | |||
There was a problem hiding this comment.
We just have to make sure this works.
There was a problem hiding this comment.
it looks obvious as to why this was changed to hostname but what I'm wondering is if there was reason it was using headers. I think we may need tests around this to be certain.
|
ping @Timer re: tests for headers/hostname change |
|
@Timer last ping before we close this one as abandoned. please do have a look at the conflicts and adding a test around the |
What kind of change does this PR introduce?
bugfix
Did you add or update the
examples/?N/A
Summary
localhost resolves to
::1for IPv6,127.0.0.1for IPv4. We shouldn't forget about IPv6 users.Does this PR introduce a breaking change?
No
Other information