doc, http: Type specification incomplete for ServerResponse.getHeader() #13825

Flarna · 2017-06-20T20:56:29Z

Version: 8.1.2 (but applicable to others)
Platform: all
Subsystem: doc, http

ServerResponse.setHeader() specifies name as string and value as string | string[].
ServerResponse.writeHead() allows an object for headers (key is header name, value is header value). All samples use string for keys and string | string[] for values - except one which uses Buffer.byteLength(body) which is a number (the sample works fine).
ServerResponse.getHeader() specifies to return a string.

Actually getHeader() returns what as passed in so it should be at least string | string[].
But as setHeader() and writeHead() actually allow any stringifyable type you get also such type back as the conversion to string happens when writing the outgoing stream not during storing the header.

Not sure here if this is worth a change; and if yes should it be in doc or code?

I would guess the same is applicable to ClientRequest.

The text was updated successfully, but these errors were encountered:

gireeshpunathil · 2018-03-29T08:34:58Z

@Flarna - if I understand your question correctly, you are saying that the headers are stored as objects before sent to the wire, so getHeader() could retrieve them as it is, as opposed to strings?

I guess they are stored as strings by stripping them from the object in writeHead method .

Flarna · 2018-03-29T13:49:06Z

It's quite a while till I submitted this. Here is a small reproducer; hope I remember correct on the actual use case...

const http = require('http')

let cnt = 0

function onRequest(req, res) {
  const body = 'hello world'
  if (cnt++ % 2) {
    res.writeHead(200, {
      'Content-Length': Buffer.byteLength(body),
      'Content-Type': 'text/plain'
    })
  } else {
    res.setHeader('Content-Length', Buffer.byteLength(body))
    res.setHeader('Content-Type', 'text/plain')
  }
  
  console.log(`typeof(res.getHeader('Content-Length')): ${typeof(res.getHeader('Content-Length'))}`)
  console.log(`typeof(res.getHeader('Content-Type')): ${typeof(res.getHeader('Content-Type'))}`)

  res.end(body)
}

http.createServer(onRequest).listen(8000)
http.get('http://localhost:8000')
http.get('http://localhost:8000')

this prints:

typeof(res.getHeader('Content-Length')): number
typeof(res.getHeader('Content-Type')): string
typeof(res.getHeader('Content-Length')): undefined
typeof(res.getHeader('Content-Type')): undefined

So if writeHead() is used then getHeader() doesn't return anything at all.
If I use setHeader() then I get exactly what I have set before (e.g. a number in case of 'Content-Length') and not a string.

The use of Buffer.byteLenght() is inspired by the sample at https://nodejs.org/dist/latest/docs/api/http.html#http_response_writehead_statuscode_statusmessage_headers

gireeshpunathil · 2018-03-29T15:58:36Z

@Flarna - I think I got your point now. Probably the setHeader() expects a series of incremental writes to the header and hence it stores the requests in an internal field outHeadersKey for potential future modifications. Whereas, if the code invoked writeHead first, the expectation is that it does in one shot, so the storing does not happen here - probably this is an optimization, based on the comments around.

Consequently, if you do a dummy setHeader first, subsequent writeHeads are stored in the internal field, and are accessible through getHeader:

#cat 13825.js

const h = require('http')
const n = require('net')

h.createServer((q, s) => {
  const bodys = 'hello world'
  const bodyw = '<html>hello</html>'
  if(process.argv[2])
    s.setHeader('Content-Length', bodys.length)
  s.writeHead(200, { 'Content-Length': bodyw.length, 'Content-Type': 'text/html' })
  
  console.log(s.getHeader('Content-Length'))
  console.log(s.getHeader('Content-Type'))
  s.end(bodyw)
}).listen(8000, () => {
  h.get('http://localhost:8000')
})

#node 13825.js
undefined
undefined
^C
#node 13825.js true
18
text/html
^C

Hope this helps. /cc @nodejs/http for expert opinion.

MoonBall · 2018-04-03T07:36:13Z

@Flarna I think the document needs to be changed. Would you like to make a PR to make the document more perfect?

Flarna · 2018-04-04T20:42:41Z

@MoonBall ok, will do so once I have some poetic moments to formulate this in a reasonable way.

http.setHeader() coerces input values. http.getHeader() returns the type as passed to setHeader(). PR-URL: #19902 Fixes: #13825 Reviewed-By: Chen Gang <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: James M Snell <[email protected]>

http.setHeader() coerces input values. http.getHeader() returns the type as passed to setHeader(). PR-URL: nodejs#19902 Fixes: nodejs#13825 Reviewed-By: Chen Gang <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: James M Snell <[email protected]>

mscdex added the http Issues or PRs related to the http subsystem. label Jun 20, 2017

micnic added the doc Issues and PRs related to the documentations. label Jun 23, 2017

Flarna mentioned this issue Apr 9, 2018

doc: improve http.setHeader and getHeader typeinfo #19902

Closed

3 tasks

gireeshpunathil mentioned this issue Apr 10, 2018

getHeader() returns undefined after using writeHead() in a certain circumstance #10354

Closed

BridgeAR closed this as completed in 50e9f8d Apr 13, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

doc, http: Type specification incomplete for ServerResponse.getHeader() #13825

doc, http: Type specification incomplete for ServerResponse.getHeader() #13825

Flarna commented Jun 20, 2017

gireeshpunathil commented Mar 29, 2018

Flarna commented Mar 29, 2018 •

edited

Loading

gireeshpunathil commented Mar 29, 2018

MoonBall commented Apr 3, 2018 •

edited

Loading

Flarna commented Apr 4, 2018

doc, http: Type specification incomplete for ServerResponse.getHeader() #13825

doc, http: Type specification incomplete for ServerResponse.getHeader() #13825

Comments

Flarna commented Jun 20, 2017

gireeshpunathil commented Mar 29, 2018

Flarna commented Mar 29, 2018 • edited Loading

gireeshpunathil commented Mar 29, 2018

MoonBall commented Apr 3, 2018 • edited Loading

Flarna commented Apr 4, 2018

Flarna commented Mar 29, 2018 •

edited

Loading

MoonBall commented Apr 3, 2018 •

edited

Loading