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

v9.x ParserIncomingMessage is not a constructor #20241

Closed
coolaj86 opened this issue Apr 24, 2018 · 2 comments
Closed

v9.x ParserIncomingMessage is not a constructor #20241

coolaj86 opened this issue Apr 24, 2018 · 2 comments
Labels
duplicate Issues and PRs that are duplicates of other issues or PRs.

Comments

@coolaj86
Copy link
Contributor

coolaj86 commented Apr 24, 2018

  • Version: v9.11.1
  • Platform: Linux
  • Subsystem: http

Works in v8.11.1

The same code run in v8.11.1 has been tested without issue

Broken in v9.11.1

This is the error message I get. I tried back to v9.9.0 with the same error.

TypeError: ParserIncomingMessage is not a constructor
    at HTTPParser.parserOnHeadersComplete (_http_common.js:81:21)
    at socketOnData (_http_server.js:472:20)
    at Socket.emit (events.js:180:13)
    at addChunk (_stream_readable.js:274:12)
    at readableAddChunk (_stream_readable.js:249:11)
    at Socket.Readable.unshift (_stream_readable.js:223:10)
    at /root/tunnel-server.js/wstunneld.js:502:14
    at process._tickCallback (internal/process/next_tick.js:176:11)

The surrounding internal node code looks like this:

_http_common.js:81
  parser.incoming = new ParserIncomingMessage(parser.socket);
                    ^

The reason this rarely used API call was run into was due to code that inspects a tcp connection before manually pushing it through either a TLS or HTTP connection:

  function onTcpConnection(conn) {
    conn.once('data', function (firstChunk) {

      // defer after return (instead of being in many places)
      process.nextTick(function () {


        ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
        ////  The offending line: 
        ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
        conn.unshift(firstChunk);
      });

      // https://github.com/mscdex/httpolyglot/issues/3#issuecomment-173680155
      if (22 === firstChunk[0]) {
        // TLS
        service = 'https';
        servername = (sni(firstChunk)||'').toLowerCase();
        console.log("tls hello servername:", servername);
        tryTls();
        return;
      }

      if (firstChunk[0] > 32 && firstChunk[0] < 127) {
        str = firstChunk.toString();
        m = str.match(/(?:^|[\r\n])Host: ([^\r\n]+)[\r\n]*/im);
        servername = (m && m[1].toLowerCase() || '').split(':')[0];
        console.log('servername', servername);
        if (/HTTP\//i.test(str)) {
          service = 'http';
          // TODO disallow http entirely
          // /^\/\.well-known\/acme-challenge\//.test(str)
          if (/well-known/.test(str)) {
            // HTTP
            if (Devices.exist(deviceLists, servername)) {
              pipeWs(servername, service, conn, Devices.next(deviceLists, servername));
              return;
            }
            copts.handleHttp(servername, conn);
          }
          else {
            // redirect to https
            copts.handleInsecureHttp(servername, conn);
          }
          return;
        }
      }

    });
  }
@apapirovski
Copy link
Member

Hi @coolaj86. See #20029 for a fix which will be in the next release.

Duplicate of #19231

@apapirovski apapirovski added the duplicate Issues and PRs that are duplicates of other issues or PRs. label Apr 24, 2018
@sagitsofan
Copy link
Contributor

Duplicate #22857

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate Issues and PRs that are duplicates of other issues or PRs.
Projects
None yet
Development

No branches or pull requests

3 participants