Repository navigation
getHeader() returns undefined after using writeHead() in a certain circumstance #10354
Description
- Version: 6.9.1
- Platform: Linux 4.4.0-21-generic logo ideas #37-Ubuntu SMP x86_64 x86_64 x86_64 GNU/Linux
- Subsystem: http
In short, calling writeHead() with a ServerResponse object which has no headers already set causes getHeader() to return undefined even if the header was set in the writeHead() function call. I am unfamiliar with the project, so I'm unsure if this is defined behavior or not.
//Case 1: using setHeader, getHeader works as intended
res.setHeader("Location", "index.html");
res.getHeader("Location"); //index.html
//Case 2: using setHeader then writeHead, getHeader works as intended
res.setHeader("Set-Cookie", "hello=world");
res.writeHead(303, {Location: "index.html"});
res.getHeader("Location"); //index.html
//Case 3: using writeHead when res._headers is null, getHeader does not work as intended
res.writeHead(303, {Location: "index.html"});
res.getHeader("Location"); //undefined
Activity
Fishrock123 commentedon Dec 20, 2016
Oh haha, one of these "bug"s. Related to #1873
IIRC there are a number of performance reason this is difficult to fix because? Implicit headers... stuff?
Maybe when it's null we can just assign it to that object...
AndrewDoesNotCompute commentedon Dec 21, 2016
Seems when
req._headersis null thatwriteHead()skips callingsetHeader()a bunch of times and instead goes straight to writing the final header. Unfortunately, I don't think we could assign the passed object toreq._headersbecause of the way keys are handled insetHeader().Fishrock123 commentedon Dec 22, 2016
@DevAndrewGeorge I think (but I have not checked) that it should be possible to invoke the same mechanism as
setHeader()uses on each of the keys in the object passed towriteHead()ifreq._headersisnull. It could have a performance impact though. :/However it should be mentioned that you are still missing implicit headers and... I think you can still get poorly defined behavior when mixing
writeHead()andsetHeader(). Until the API is re-written (probably via HTTP/2) headers is just always going to have loopholes and strange behavior. :(That being said I'd review a PR if you are willing to make one that covers what I've mentioned in the first paragraph. :D
Trott commentedon Jul 15, 2017
@nodejs/http
gireeshpunathil commentedon Apr 10, 2018
A reasonable explanation to this behavior is optimization as opposed to a bug. This block explains the logic.
setHeaderis for a progressive supply of header fields (iteratively set the headers as the program advances) andwriteHeadis for a one shot write (all the header values available to write at a single program point) in which case you don't need to reflect on the written data and hence the storing of headers for future retrieval throughgetHeaderis short-circuited.The same is also explained in #13825 (comment) and a PR is being worked upon here : #19902
gireeshpunathil commentedon May 14, 2018
@Fishrock123 - can we have a consensus on this as to:
My assertion is that use
setHeaderfor progressive supply of headers for future modification potential (and hence can be retrieved throughgetHeader) and usewriteHeadfor committing fields that are confirmed to be final (cannot be retrieved throughgetHeader), and a doc to this effect would be good enough.bnoordhuis commentedon May 14, 2018
Not a bug, an optimization. Could maybe use a note in the docs.
If we want uniform behavior, we could set
this[outHeadersKey] = nullin writeHead(). There's no reason to hang on to them after the headers have been sent and it saves some memory.gireeshpunathil commentedon May 20, 2018
thanks. Added
good first issueto attract contributions.AndrewDoesNotCompute commentedon May 21, 2018
I should actually block away a few hours and actually get this done. It's been literal years.... :P
doc: explain HTTP writeHead()'s fast path behavior
doc: explain HTTP writeHead()'s fast path behavior