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

HTTP2 writehead header format can't use array of arrays for headers #24466

Closed
StephenLynx opened this issue Nov 18, 2018 · 4 comments
Closed
Labels
confirmed-bug Issues with confirmed bugs. http2 Issues or PRs related to the http2 subsystem.

Comments

@StephenLynx
Copy link

StephenLynx commented Nov 18, 2018

node 10.13
CentOs
HTTP2

With http, you can send headers like this:
[
[
"Content-Type",
"text/html; charset=utf-8"
],
[
"last-modified",
"Sun, 18 Nov 2018 17:49:47 GMT"
]
]

To send multiple values for the same header, like cookies. But with http2 the returned headers looks like this when this format is used:
https://i.imgur.com/QXZnu4v.png

@StephenLynx StephenLynx changed the title HTTP2 writehead header format can't use array of arrays HTTP2 writehead header format can't use array of arrays for headers Nov 18, 2018
@Trott
Copy link
Member

Trott commented Nov 25, 2018

@nodejs/http2

@Trott Trott added the http2 Issues or PRs related to the http2 subsystem. label Nov 25, 2018
@sebdeckers
Copy link
Contributor

I can confirm this diverging behaviour. However the documentation for http.writeHead does not describe the Array of Arrays use case. Should this be supported?

const http = require('http')
const http2 = require('http2')

const headers = [
  [ 'Content-Type', 'text/html; charset=utf-8' ],
  [ 'last-modified', 'Sun, 18 Nov 2018 17:49:47 GMT' ]
]

http.createServer((request, response) => {
  console.log(`Request via HTTP/${request.httpVersion}`)
  response.writeHead(200, headers)
  response.end()
}).listen(8001)

http2.createServer((request, response) => {
  console.log(`Request via HTTP/${request.httpVersion}`)
  response.writeHead(200, headers)
  response.end()
}).listen(8002)
$ curl -v http://localhost:8001
* Rebuilt URL to: http://localhost:8001/
*   Trying ::1...
* TCP_NODELAY set
* Connected to localhost (::1) port 8001 (#0)
> GET / HTTP/1.1
> Host: localhost:8001
> User-Agent: curl/7.54.0
> Accept: */*
> 
< HTTP/1.1 200 OK
< Content-Type: text/html; charset=utf-8
< last-modified: Sun, 18 Nov 2018 17:49:47 GMT
< Date: Mon, 26 Nov 2018 02:44:23 GMT
< Connection: keep-alive
< Transfer-Encoding: chunked
< 
* Connection #0 to host localhost left intact

$ curl -v --http2-prior-knowledge http://localhost:8002
* Rebuilt URL to: http://localhost:8002/
*   Trying ::1...
* TCP_NODELAY set
* Connected to localhost (::1) port 8002 (#0)
* Using HTTP2, server supports multi-use
* Connection state changed (HTTP/2 confirmed)
* Copying HTTP/2 data in stream buffer to connection buffer after upgrade: len=0
* Using Stream ID: 1 (easy handle 0x7f8f85000400)
> GET / HTTP/2
> Host: localhost:8002
> User-Agent: curl/7.54.0
> Accept: */*
> 
* Connection state changed (MAX_CONCURRENT_STREAMS updated)!
< HTTP/2 200 
< 0: Content-Type
< 0: text/html; charset=utf-8
< 1: last-modified
< 1: Sun, 18 Nov 2018 17:49:47 GMT
< date: Mon, 26 Nov 2018 02:32:38 GMT
< 
* Connection #0 to host localhost left intact

@jasnell
Copy link
Member

jasnell commented Nov 26, 2018

Unfortunately, we probably should support it.

@mcollina mcollina added the confirmed-bug Issues with confirmed bugs. label Nov 26, 2018
sebdeckers added a commit to sebdeckers/node that referenced this issue Nov 27, 2018
writeHead supports an array of arrays containing header name and values.
Compatibility between http2 & http1 even though this is not documented.

Fixes: nodejs#24466
@StephenLynx
Copy link
Author

A suggestion: deprecate the nested array format starting off node 11after implementing parity. You can already achieve the same functionality trough the documented means, which is what I did.

@Trott Trott closed this as completed in 5dacbf5 Nov 29, 2018
targos pushed a commit that referenced this issue Nov 29, 2018
writeHead supports an array of arrays containing header name and values.
Compatibility between http2 & http1 even though this is not documented.

Fixes: #24466

PR-URL: #24665
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
refack pushed a commit to refack/node that referenced this issue Jan 14, 2019
writeHead supports an array of arrays containing header name and values.
Compatibility between http2 & http1 even though this is not documented.

Fixes: nodejs#24466

PR-URL: nodejs#24665
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
BethGriggs pushed a commit that referenced this issue Feb 12, 2019
writeHead supports an array of arrays containing header name and values.
Compatibility between http2 & http1 even though this is not documented.

Fixes: #24466

PR-URL: #24665
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
rvagg pushed a commit that referenced this issue Feb 28, 2019
writeHead supports an array of arrays containing header name and values.
Compatibility between http2 & http1 even though this is not documented.

Fixes: #24466

PR-URL: #24665
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. http2 Issues or PRs related to the http2 subsystem.
Projects
None yet
Development

No branches or pull requests

5 participants