HTTP module, multiple headers with same name

[ghost]

• Version: All versions from what I can tell
• Platform: All
• Subsystem: http module

The problem is pretty straight forward. There is no good way to send out multiple headers with the same name. I literally had to resort to this jank solution:

http.ServerResponse.prototype.sendRawHeader = function(name, value){
	if(!this._headerSent){
		this.connection.write(['HTTP/1.1', this.statusCode, STATUS_CODES[this.statusCode], '\r\n'].join(' '));

	}
	this.connection.write([ [name, ':'].join(''), [value, '\r\n'].join('') ].join(' '));
}

This is still bad in my personal opinion as I'm having to write headers out before content and the module should be handling the status code for me.

While everyone seems to cite that headers should be capable of being handled as comma separated values, this does not work cross-platform in ever situation. The easiest thing to cite would be sending out multiple Set-Cookie headers like so:

Set-Cookie: uid=n
Set-Cookie: token=XXX-XXXX-XXX-XXXX

There is absolutely no way to do this and this will never work cross-browser or cross-platform

Set-Cookie: uid=n, token=XXX-XXXX-XXX-XXXX

There are other situations with custom software that I've dealt with where other headers need to be sent in multiple lines in order for proper evaluation.

I propose that a function be added like http.ServerResponse.addRawHeader(string) and in the response portion of the HTTP module, a variable (array) named additionalRawHeaders be used to track these headers. When headers are finally sent, the last part would be to iterate through additionalRawHeaders and just write those out to the socket.

If no one agrees with this, if I just write it myself, what are the chances a pull request will be accepted for this?

[bnoordhuis]

You know of res.writeHead(200, [['Set-Cookie', 'one'], ['Set-Cookie', 'two']])? If yes, why doesn't that work for you? If no - well, now you do. :-)

[ghost]

I do not believe you understand how that works. That's incorrect usage in the current stable release.

Code example:

res.writeHead(200, [['X-hdr', 'one'], ['X-hdr', 'two']])

Result example:

HTTP/1.1 200 OK
Date: Thu, 13 Jul 2017 15:10:12 GMT
Server: Potato-box.v1.23.8
0: X-hdr,one
1: X-hdr,two
Content-Type: text/html; charset=UTF-8
Vary: Accept-Encoding
Content-Encoding: gzip
Content-Length: 1747
Keep-Alive: timeout=5, max=96
Connection: Keep-Alive

The entire way headers are handled in the HTTP library is just bad when you need flexibility. Also, writeHead pushes headers out. That defeats the purpose of buffering them in memory until the first http.response.write() is sent. Like, you want to be able to remove and invalidate irrelevant headers if you process some piece of data in callback hell that indicates a header is irrelevant if necessary.

[evanlucas]

That is working for me on 8.1.4

const server = http.createServer((req, res) => { 
  res.writeHead(200, [['a', 'one'], ['a', 'two']]) 
  res.end() 
}).listen(8000)

$ curl -v http://localhost:8080/
*   Trying ::1...
* TCP_NODELAY set
* Connected to localhost (::1) port 8000 (#0)
> GET / HTTP/1.1
> Host: localhost:8000
> User-Agent: curl/7.51.0
> Accept: */*
> 
< HTTP/1.1 200 OK
< a: one
< a: two
< Date: Thu, 13 Jul 2017 15:52:54 GMT
< Connection: keep-alive
< Transfer-Encoding: chunked
< 
* Curl_http_done: called premature == 0
* Connection #0 to host localhost left intact

What version are you on?

[ghost]

Interesting, I had v8.1.2 and it does not work in that version. I like doing writeHead() over directly using http.response.write(), but it still isn't quite appropriate for what the HTTP module aims to do when it comes to handling headers. http.response.writeHead() immediately sends the headers out.

Just poured over this git repo looking for how to contribute code. I'd be glad to just implement some useful functionality in addition to what exists. If you could point me in that direction I can probably have it done by the weekend.

Edit: I feel dumb... found it... here. Going to read over it and see what I got to do to just implement it and make it happen. I'm going to take the http module and modify it anyway, so I might as well contribute it if I can.

[bnoordhuis]

New APIs aren't added lightly. You are the first to bring this up so it can't be very pressing or common, and if the only use case is to work around broken clients, it's unlikely to get accepted.

Interesting, I had v8.1.2 and it does not work in that version.

The http library didn't change between 8.1.2 and 8.1.4. I tested it locally and both versions produce the same output. It's almost certainly something on your end.

[ghost]

RFC 2616 4.2 sums up the gist of flexibility necessary for anything that should be handling headers for HTTP. I've read tons of comments in other issues that are in the same realm that mention that headers "should" also work as comma-separated values, but that's just not the case. While yes, there is an RFC out there indicating combining headers into comma separated values should work, and we can implement NodeJS apps to handle that. What we can't do is enforce that on web browsers that mostly seem to overlook that particular RFC.

Anyway, long story short, the HTTP module is lacking and not flexible enough to be considered professional by any means. While people have probably spent a lot of time working on it and making it, the lack of design though to headers was just poor quality. Headers have to be flexible to be even remotely usable outside of very narrow niche pixel pusher territory. Fixing that would actually bring in more engineer level interest instead of newbie coders and early adopters.

[dougwilson]

I know there is some discussion about writeHead, but just stopping by I wondered if you knew about res.setHeader(name, array) (docs: https://nodejs.org/dist/latest-v8.x/docs/api/http.html#http_response_setheader_name_value). In the documentation there is the following example:

response.setHeader('Set-Cookie', ['type=ninja', 'language=javascript']);

And indeed, that does set two separate headers for me:

$ node -e 'require("http").createServer(function(req,res){res.setHeader("Set-Cookie",["type=ninja","language=javascript"]);res.end()}).listen(3000)' &
[1] 57700
$ curl -i http://127.0.0.1:3000/
HTTP/1.1 200 OK
Set-Cookie: type=ninja
Set-Cookie: language=javascript
Date: Thu, 13 Jul 2017 21:54:21 GMT
Connection: keep-alive
Content-Length: 0

Does that API not work or should something be changed? What would an ideal API look like? Just thought I'd try and help out :)

[bnoordhuis]

I'll be interested to know if that holds up to OP's exacting standards. I hope it does. Wouldn't want to miss out on that engineer level interest, you know.

For the sake of posterity, it's probably good to point out that .setHeader() merges Cookie header values (but not other headers) whereas .writeHead() emits two separate Cookie headers.

Cookie headers look like Cookie: a=1; b=2 though, it doesn't use commas.

[ghost]

lol sorry if I came off offensively there, @bnoordhuis, I'm just a CS guy and I'm not claiming god-like-skills or even good-skills. I'm only barking up the RFC improvement tree. Node is not a mature platform to work with yet and I wouldn't call it stable yet, though I do like the idea of CommonJS and I like the idea of only having to deal with knowing 1 programming language for a web based environment. So I'm hellbent on working with it right now.

Realistically, if you ever go surfing and watching what other people are doing with Node, you'll see a real lack of programmers or even skilled coders. This isn't to say there are none, but there are few and far between. Anyway, some of this isn't even the platform's fault, it's just a side effect of Javascript in itself being so easy to pick up with minimal understanding. Promises is the biggest joke that is currently on my vendetta list right now........ anyway I'm getting way too off topic here...........

@dougwilson That is actually good enough man! It :) I didn't think to go back and retry that with v8.1.4, as it's just 2 minors away from the version I had. It didn't work in v8.1.2 for me when I tried it a few days ago, i got the same array issue as with writeHead. Considering this works, in 8.1.4, I'm going to mark close this. You can damn near do everything surrounding the RFC for handling headers.

@bnoordhuis @dougwilson thanks for the replies guys, I really do appreciate the time and communication. If there's anything that I can do to help contribute time/effort/codewise for node, let me know. I'm down to help out. For realz <-- with a Z guys. :)