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

http.createServer(options, app .... DOES NOT WORK #4218

Closed
cekvenich opened this issue Mar 15, 2020 · 12 comments
Closed

http.createServer(options, app .... DOES NOT WORK #4218

cekvenich opened this issue Mar 15, 2020 · 12 comments

Comments

@cekvenich
Copy link

https.createServer(options, app).listen(8080)
does not work as documented here:

  1. It does not listen
  2. It does not set maxHeaderSize.
    http.createServer({maxHeaderSize: 16000 }, appInst).listen(8080) console.warn(http.maxHeaderSize)
    outpuuts 8192.
    Above should listen and set size as documented, it does not.

I create the app, and then do above to listen.
It does not answer to a request on that port and it does not set the maxHeaderSize.

Issue is referenced:

@cekvenich
Copy link
Author

@UlisesGascon
Copy link
Member

Hi @cekvenich!

I tried to reproduce your code as follows:

// content for serve.js
const express = require('express')
const https = require('https')
const http = require('http')
const app = express()

http.createServer(app).listen(80)
https.createServer({maxHeaderSize: 16000 }, app).listen(443, () => {
    console.warn("maxHeaderSize:", http.maxHeaderSize)
})

If you execute the file directly like node server the size will be 8192 as this is the maximum value provided by HTTP Core library. See

You can skip this limitation using the flag --max-http-header-size. So the execution will be node --max-http-header-size=16000 server in this case you can see that the size is the expected one.

@cekvenich
Copy link
Author

cekvenich commented Mar 15, 2020

@UlisesGascon True you can pass the flag to node CLI.
Aside my code does not use https, only http.
But you can't set it programmatically: as documented in both express and node. And that is a bug in express, so I opened the issue.

@dougwilson
Copy link
Contributor

Hi @cekvenich sorry you are having trouble. I'm not clear where the bug would be, exactly. Are you saying there is a bug in the Express code or a bug in the Express documentation? Would you also be able to help point out where the bug is / perhaps even make a pull request to help fix?

@cekvenich
Copy link
Author

@dougwilson I can't make a PR since I see you using Object.create and I don't know how to pass args to node from that. It be nice if the code was updated to ES6 class syntax so we users can help.

Either docs should say that you can't set the options or setting the options programmatically is broken - as demonstrated by my code. (not the code that someone cut/pasted from docs).
This issue is re-open of an old issue that I linked.

@dougwilson
Copy link
Contributor

Hi @cekvenich it's fine if you cannot make a PR; I'm more asking for at least a high-level description, perhaps some links to the code would help us to even know what Object.create you are referring to in this case.

The code you provided shows the same behavior even if you remove Express completely, example:

var http = require('http')
http.createServer({maxHeaderSize: 16000 }, function () {}).listen(8080)
console.warn(http.maxHeaderSize)

So I'm unclear where Express is coming into the equation, as the above runs in Node.js without any dependency on Express, but still prints out the wrong information you're looking for it to print...

@cekvenich
Copy link
Author

cekvenich commented Mar 15, 2020

@dougwilson So maybe we need to open a bug with node?
(HTTP specs says querystrings/url can be unlimited length)

@dougwilson
Copy link
Contributor

Right, but even if I change appInst in your example to something not Express, it still doesn't set it. If I'm able to provide an example of it not working where Express is not even being used, then it would seem the bug is in Node.js itself... and that bug is just showing up when you're trying to use Express with the Node.js HTTP server, but would apply to any use of their HTTP server, Express or not I would guess.

@cekvenich
Copy link
Author

@dougwilson sorry I deleted message when I read yours wrong and reposted so messages are out of sync. We may have to open an issue w/ node. That is not the http spec. HTTP specs says querystrings/url can be unlimited length Servers is Go and Java work fine and follow the spec.

@dougwilson
Copy link
Contributor

Yea, sounds like you should open an issue with Node.js indeed :) !

@cekvenich
Copy link
Author

I opened it w/ node: nodejs/node#32290

@cekvenich
Copy link
Author

I wrote a test case and it shows that it does work correctly and that bug has to be someplace on my side.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants