fixes Origin validation for IP address. gives meaning to public and allowedHosts #1622
Conversation
for both Host checking and Origin checking
carlosgeos
requested review from
alexander-akait and
michael-ciniawsky
as code owners
carlosgeos
changed the title
|
I also like #1619. If that one gets merged, I'll update this PR (to have no conflicts) |
| // so we have the pure IPv6-address in hostname. | ||
| if (ip.isV4Format(hostname) || ip.isV6Format(hostname)) { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
Why we remove this check, it can be breaking change for some developers?
There was a problem hiding this comment.
This check basically means no security (for both static server and sockjs server) (#1618). It is not breaking if people are following the docs 👍
There was a problem hiding this comment.
@carlosgeos developers are not following the docs 😄 Better avoid this removing, we can do this in next major release
There was a problem hiding this comment.
hahahah ok, then I'll close this PR
| // always allow localhost host, for convience | ||
| if (hostname === 'localhost') { | ||
| if (hostname === 'localhost' || hostname === '127.0.0.1') { |
There was a problem hiding this comment.
We need better check here, because some developers can use 192.168.0.1 or other local IPs
There was a problem hiding this comment.
If they use a local IP, developers should specify public: '192.168.0.x'. It will be covered by the public option further down in the code:
webpack-dev-server/lib/Server.js
Lines 714 to 716 in dd92f31
allowedHosts can also be used.
|
Also need rebase |
For Bugs and Features; did you add new tests?
Yes, I have adapted and improved the tests.
Motivation / Use-Case
#1618
Breaking Changes
None.
BUT it is breaking if people are using webpack-dev-server in a way it is not intended to be used. For example, without specifying
publicorallowedHoststo serve publicly.Additional Info
This fixes #1618 which are two things basically:
public,allowedHosts, etc