http,https: align options of https's server with http's #38992
Closed
Conversation
|
We should not special-case https://nodejs.org/api/https.html#https_https_createserver_options_requestlistener claims that all “options from tls.createServer(), tls.createSecureContext() and http.createServer()” are supported – if that’s not already the case, then the goal should be to make that the case (in particular, all of the options supported in |
|
@Ayase-252 Maybe https and http could just share their code for validating the options and setting them on function storeHttpOptions(options) {
if (options.maxHeaderSize) {
validate(options.maxHeaderSize);
this.maxHeaderSize = options.maxHeaderSize;
}
// ... for the other options
}
// in http:
function Server(options) {
// ....
storeHttpOptions.call(this, options);
net.Server.call(this, options);
// ....
}
// in https:
function Server(options) {
// ....
storeHttpOptions.call(this, options);
tls.Server.call(this, options);
// ....
} |
maxHeaderSize option for https server2243db8
to
146ce9a
|
I think it is ready for review. @addaleax PTAL. |
|
Thank you :) |
|
Landed in 8954e03...8a8c7f4 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Add
maxHeaderSizeoptions for https server. The underlying logic has been implemented in #30570, but https has not adapt to that change.Fixes: #38954
Refs: #30570