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

error http request at headers is array #8235

Closed
magicode opened this issue Aug 23, 2016 · 7 comments · Fixed by #8238
Closed

error http request at headers is array #8235

magicode opened this issue Aug 23, 2016 · 7 comments · Fixed by #8238
Labels
http Issues or PRs related to the http subsystem.

Comments

@magicode
Copy link

magicode commented Aug 23, 2016

  • Version: v6.2.1
  • Platform: ubuntu 14

test case

var http = require("http");

var headersArray = [
        ['Host','echo.websocket.org'],
        ['Connection','Upgrade'],
        ['Upgrade','websocket'],
        ['Origin','http://www.websocket.org'],
    ];
var headersObj = {
        'Host':'echo.websocket.org',
        'Connection':'Upgrade',
        'Upgrade':'websocket',
        'Origin':'http://www.websocket.org'
};

var req = http.request({
    method: "GET",
    host: "echo.websocket.org" ,
    path: "ws://echo.websocket.org/?encoding=text",
    headers: headersArray
});
req.on('response',function(res){
    console.log('response' , res.headers);
});
req.on('upgrade',function(res){
    console.log('upgrade',res.headers);
});
req.end();

error output

_http_common.js:92
      (parser.outgoing._headers.upgrade === undefined ||
                               ^

TypeError: Cannot read property 'upgrade' of null
    at HTTPParser.parserOnHeadersComplete (_http_common.js:92:32)
    at Socket.socketOnData (_http_client.js:359:20)
    at emitOne (events.js:96:13)
    at Socket.emit (events.js:188:7)
    at readableAddChunk (_stream_readable.js:172:18)
    at Socket.Readable.push (_stream_readable.js:130:10)
    at TCP.onread (net.js:542:20)
@mscdex mscdex added the http Issues or PRs related to the http subsystem. label Aug 23, 2016
@disharmonized
Copy link

From docs:

headers: An object containing request headers.

You should use headersObj.

@magicode
Copy link
Author

magicode commented Aug 23, 2016

@disharmonized
Why has this line ?
https://github.com/nodejs/node/blob/v6.4.0/lib/_http_client.js#L124

this code work well

var http = require("http");

var headersArray = [
        ['Host','echo.websocket.org'],
        ['Connection','Upgrade'],
//        ['Upgrade','websocket'],
        ['Origin','http://www.websocket.org'],
    ];

var req = http.request({
    method: "GET",
    host: "echo.websocket.org" ,
    path: "ws://echo.websocket.org/?encoding=text",
    headers: headersArray
});
req.on('response',function(res){
    console.log('response' , res.headers);
});
req.on('upgrade',function(res){
    console.log('upgrade',res.headers);
});
req.end();

@disharmonized
Copy link

Hmm, interesting...

@mscdex
Copy link
Contributor

mscdex commented Aug 23, 2016

You really should use the documented API for headers (objects, not arrays). Nonetheless, I have a fix for this in #8238.

@magicode
Copy link
Author

Array is only way to send duplicate headers.
without nodejs merge them.

@mscdex
Copy link
Contributor

mscdex commented Aug 23, 2016

What server requires duplicate HTTP headers?

@magicode
Copy link
Author

For proxy server.
There are many browsers and software and servers that not meet the standards.

mscdex added a commit to mscdex/io.js that referenced this issue Aug 26, 2016
This commit fixes connection upgrade checks, specifically when headers
are passed as an array instead of a plain object to http.request()

Fixes: nodejs#8235
PR-URL: nodejs#8238
Reviewed-By: James M Snell <[email protected]>
Fishrock123 pushed a commit to Fishrock123/node that referenced this issue Sep 8, 2016
This commit fixes connection upgrade checks, specifically when headers
are passed as an array instead of a plain object to http.request()

Fixes: nodejs#8235
PR-URL: nodejs#8238
Reviewed-By: James M Snell <[email protected]>
Fishrock123 pushed a commit that referenced this issue Sep 9, 2016
This commit fixes connection upgrade checks, specifically when headers
are passed as an array instead of a plain object to http.request()

Fixes: #8235
PR-URL: #8238
Reviewed-By: James M Snell <[email protected]>
mscdex added a commit to mscdex/io.js that referenced this issue Nov 18, 2016
This commit fixes connection upgrade checks, specifically when headers
are passed as an array instead of a plain object to http.request()

Fixes: nodejs#8235
PR-URL: nodejs#8238
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this issue Nov 22, 2016
This commit fixes connection upgrade checks, specifically when headers
are passed as an array instead of a plain object to http.request()

Fixes: #8235
PR-URL: #8238
Reviewed-By: James M Snell <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
http Issues or PRs related to the http subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants