Skip to content
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

HTTPS clients do not receive Invalid Host/Origin header errors, they just get disconnected with no data #3997

Closed
driskell opened this issue Nov 1, 2021 · 5 comments · Fixed by #3998

Comments

@driskell
Copy link
Contributor

driskell commented Nov 1, 2021

Bug report

The Invalid Host/Origin header error frame does not get sent to a client that is connecting using HTTPS.

This is due to client.terminate() call after sending the message, which I think does not allow sufficient time for the frame containing the error to get sent, presumedly slowed down by the TLS layer, so the client never receives it. (Terminate calls socket.destroy under the hood so assumes all data has been sent already.)

Changing this line from client.terminate() to client.close() resolves the issue: https://github.com/webpack/webpack-dev-server/blob/master/lib/Server.js#L1636

I would raise a PR but I'm unsure if you would want timeout handling here too to ensure it does not linger around for a bad client - but since this is a development tool that perhaps is fine. Also I think there's likely no timeout handling anywhere else so perhaps this one line change is fine?

Actual Behavior

With a non-HTTPS connection, and deliberately creating an incorrect Host/Origin (by loading index.html from a different server that then loads bundles from http://localhost:8080 as an example) - you receive the message:

[webpack-dev-server] Invalid Host/Origin header
[webpack-dev-server] Disconnected!

With a HTTPS connection, using same configuration as above, you receive only:

[webpack-dev-server] Disconnected!

Expected Behavior

Same result on both - specifically the Invalid Host/Origin header message should appear in the console.

How Do We Reproduce?

(Will look at getting a basic example together if needed - but I could reproduce this with two different large configurations so I believe would apply to any configuration, caveat maybe needing similar environment to me, as long as you make the error occur and run HTTPS.)

Please paste the results of npx webpack-cli info here, and mention other relevant information

Google Chrome Version 95.0.4638.54

System:
OS: macOS 12.0.1
CPU: (8) x64 Intel(R) Core(TM) i5-1038NG7 CPU @ 2.00GHz
Memory: 41.94 MB / 16.00 GB
Binaries:
Node: 14.18.1 - ~/.nvm/versions/node/v14.18.1/bin/node
npm: 6.14.15 - ~/.nvm/versions/node/v14.18.1/bin/npm
Browsers:
Chrome: 95.0.4638.69
Firefox: 89.0.2
Safari: 15.1
Safari Technology Preview: 15.4
Packages:
babel-loader: ^8.1.0 => 8.2.2
clean-webpack-plugin: ^4.0.0 => 4.0.0
css-loader: ^6.4.0 => 6.4.0
css-minimizer-webpack-plugin: ^3.1.1 => 3.1.1
file-loader: ^6.1.1 => 6.2.0
html-webpack-exclude-assets-plugin: 0.0.7 => 0.0.7
html-webpack-plugin: ^5.4.0 => 5.4.0
imports-loader: ^3.0.0 => 3.0.0
postcss-loader: ^6.2.0 => 6.2.0
sass-loader: ^12.2.0 => 12.2.0
script-loader: ^0.7.2 => 0.7.2
style-loader: ^3.3.0 => 3.3.0
terser-webpack-plugin: ^5.2.4 => 5.2.4
webpack: ^5.61.0 => 5.61.0
webpack-bundle-analyzer: ^4.5.0 => 4.5.0
webpack-cli: ^4.9.1 => 4.9.1
webpack-dev-server: ^4.4.0 => 4.4.0
webpack-merge: ^5.2.0 => 5.8.0

@alexander-akait
Copy link
Member

This is valid behavior, we should terminate, not close when headers are invalid

@driskell
Copy link
Contributor Author

driskell commented Nov 1, 2021

@alexander-akait Should we then remove the sendMessage? It's understandable that will never complete for HTTPS since it has no opportunity to send it before termination. I can imagine potentially depending on operating system it might not even send for HTTP either - though I do not fully understand the mechanics underneath here.

Also I guess there needs to be some message on the client side about termination with no message rather than a regular non-error message of "Disconnected!" ?

@alexander-akait
Copy link
Member

Why you need message here?

@driskell
Copy link
Contributor Author

driskell commented Nov 1, 2021

This doesn't tell you what the issue is and takes a fair bit of time to work out what's wrong:

[webpack-dev-server] Disconnected!

This tells you what the problem is so you can fix it really quick:

[webpack-dev-server] Invalid Host/Origin header
[webpack-dev-server] Disconnected!

It also is clear in the code it is SUPPOSED to be sent, but is not being given enough time for TLS. I've scoured the NodeJS sources and TLS data is encrypted and written asynchronously during a cycle and not synchronously so the terminate happens while the data to be sent is still in a buffer. I didn't get very far finding where TCP is written it's understandably extremely abstract at that point but I imagine that attempts to write synchronously and moves asynchronous for the result, thus why non-HTTPS the message is already on the (way to the) wire before it terminates.

@alexander-akait
Copy link
Member

PR welcome to fix

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 a pull request may close this issue.

2 participants