Skip to content

node 8.12 net,http module emit connection bug. TypeError: ParserIncomingMessage is not a constructor . #22857

Description

@whtiehack
  • Version: v8.12.0
  • Platform: linux ,windows
  • Subsystem:

Here is the code that can be reproduced:

const http = require('http');
const net = require('net');


const httpserver = http.createServer((req,res)=>{
    console.log('~~ http request');
    res.end('aaa');
})

httpserver.on('connection',(socket)=>{
    console.log('httpsever connection');
})

const netserver = net.createServer((socket)=>{
    console.log('netserver connection');
    httpserver.emit('connection',socket);
    // This listener is not important.
    socket.once('data',(data)=>{
        console.log('netserver data',data.toString());
        // This line of code is not important.
        socket.emit('data',data);
    });
}).listen(30332);

The same code run in v10.9.0 and v8.10.0 has been tested without issue.

This is a bug ,because of http_event_connection.

Note: This event can also be explicitly emitted by users to inject connections into the HTTP server. In that case, any Duplex stream can be passed.

Activity

  1. changed the title [-]node 8.12 net,http module bug. TypeError: ParserIncomingMessage is not a constructor .[/-] [+]node 8.12 net,http module emit `connection` bug. TypeError: ParserIncomingMessage is not a constructor .[/+] on Sep 14, 2018
  2. whtiehack commented on Sep 14, 2018

    @whtiehack
    Author

    Temporary solution socket.server = null;

    
    const netserver = net.createServer((socket)=>{
        console.log('netserver connection');
        socket.server = null;
        httpserver.emit('connection',socket);
    
    }).listen(30332);
    
    
  3. added
    httpIssues and PRs related to the http subsystem.
    on Sep 14, 2018
  4. tonyjansan commented on Sep 15, 2018

    @tonyjansan

    [2018-09-15 13:10:24.637] [ERROR] console - Caught exception: TypeError: ParserIncomingMessage is not a constructor
    at HTTPParser.parserOnHeadersComplete (_http_common.js:81:21)
    at socketOnData (_http_server.js:470:20)
    at emitOne (events.js:116:13)
    at Socket.emit (events.js:211:7)

    like some problem, seems critical issue!!!

  5. AyushG3112 commented on Sep 15, 2018

    @AyushG3112
    Contributor

    cc @nodejs/http

    Offending line:

    https://github.com/nodejs/node/blob/master/lib/_http_common.js#L86

    Looks like it should be

    const incoming = parser.incoming = ParserIncomingMessage

    But I'm not 100% sure

  6. added a commit that references this issue on Sep 15, 2018
  7. apapirovski commented on Sep 15, 2018

    @apapirovski
    Contributor

    The problem is simply one of backporting #20029 to v8.x. I'll take care of it later today if no one gets there first. (Surprised that one slipped by.)

  8. alexjeffburke commented on Sep 23, 2018

    @alexjeffburke
    Contributor

    Hi, sorry to nudge but is there any progress on getting a fix in - I assume that #22880 is the one to watch? Since this was a regression introduced in the active LTS release, are there plans to do an 8.12.1 release? Thanks.

  9. lpinca commented on Sep 25, 2018

    @lpinca
    Member

    cc: @nodejs/lts

  10. MylesBorins commented on Sep 25, 2018

    @MylesBorins
    Contributor

    We don't currently have a timeline for the next 8.x release but it could be escalated if this problem is serious enough. We could potentially get a smaller release out in the next 2 - 3 unless this is deemed to be a severe bug, in which case we could escalate a hotfix

  11. papandreou commented on Sep 25, 2018

    @papandreou
    Contributor

    2 - 3 days or weeks? :)

  12. MylesBorins commented on Sep 25, 2018

    @MylesBorins
    Contributor

    sorry for missing a word

    We generally do a 2 week r.c. process for new LTS releases, and we have not yet backported stuff. So unless this is deemed a severe bug which would escalate a release we wouldn't see a release for at least 2 - 3 weeks.

  13. whtiehack commented on Sep 26, 2018

    @whtiehack
    Author

    @MylesBorins

    This problem is not a serious problem, just using the latest version of node, many libraries can not be used. For example pomelo, pinus and mitm. Therefore, using these libraries requires a rollback of the node version.


    This problem has been repeated many times before and after, I think we should look for reasons to avoid recurring in the future.

    Thanks.

  14. MylesBorins commented on Sep 26, 2018

    @MylesBorins
    Contributor

    @whtiehack we can add them to CITGM so we don't hit this edge case again

  15. dougwilson commented on Sep 26, 2018

    @dougwilson
    Member

    FWIW this is a documented feature:

    https://nodejs.org/dist/latest-v8.x/docs/api/http.html#http_event_connection

    Note: This event can also be explicitly emitted by users to inject connections into the HTTP server. In that case, any Duplex stream can be passed.

    It would seem like the Node.js test suite itself should catch the issue instead of relying on CITGM to catch regressions in documented features.

    Not saying that it's not worth adding to CIGTM, but maybe someone wants to contribute a test directly.

  16. lpinca commented on Feb 10, 2019

    @lpinca
    Member

    Fixed by e3bddeec18. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    httpIssues and PRs related to the http subsystem.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions