Make clientError overridable - #4557
Conversation
Sorry, something went wrong.
`clientError` will have `http.Server`-specific behavior, and we don't want to shadow it in `tls.Server`.
Make default `clientError` behavior (close socket immediately)
overridable. With this APIs it is possible to write a custom error
handler, and to send, for example, a 400 HTTP response.
http.createServer(...).on('clientError', function(err, socket) {
socket.end('HTTP/1.1 400 Bad Request\r\n\r\n');
socket.destroy();
});
Fix: nodejs#4543
|
cc @nodejs/http |
Sorry, something went wrong.
There was a problem hiding this comment.
Perhaps: "Default behavior is to destroy the socket immediately."
Sorry, something went wrong.
There was a problem hiding this comment.
For people searching the page, mentioning "malformed request" or "invalid HTTP" would be helpful.
Sorry, something went wrong.
There was a problem hiding this comment.
Ack.
Sorry, something went wrong.
|
Should there be a test that triggers clientError with and without the override to confirm both more-or-less basically work as expected? Or is the idea that the modified TLS tests already check the override and other existing tests already check the basic behavior? |
Sorry, something went wrong.
|
@Trott yep, I will write a test. Thank you for suggestion |
Sorry, something went wrong.
Sorry, something went wrong.
|
LGTM if CI is ok with it. |
Sorry, something went wrong.
Sorry, something went wrong.
`clientError` will have `http.Server`-specific behavior, and we don't want to shadow it in `tls.Server`. PR-URL: #4557 Reviewed-By: Brian White <mscdex@mscdex.net>
Make default `clientError` behavior (close socket immediately)
overridable. With this APIs it is possible to write a custom error
handler, and to send, for example, a 400 HTTP response.
http.createServer(...).on('clientError', function(err, socket) {
socket.end('HTTP/1.1 400 Bad Request\r\n\r\n');
socket.destroy();
});
Fix: #4543
PR-URL: #4557
Reviewed-By: Brian White <mscdex@mscdex.net>
See: #4543