nodejs / node

Node.js JavaScript runtime ✨🐢🚀✨
https://nodejs.org
Other
106.39k stars 28.99k forks source link

HTTP/2 null pointer exceptions in insecure server. #51094

Open mctrafik opened 9 months ago

mctrafik commented 9 months ago

Version

v20.9.0

Platform

Darwin KH7JWKR9RD 23.1.0 Darwin Kernel Version 23.1.0: Mon Oct 9 21:27:24 PDT 2023; root:xnu-10002.41.9~6/RELEASE_ARM64_T6000 arm64

Subsystem

No response

What steps will reproduce the bug?

I am trying to write an insecure HTTP2 web server. However it crashes on any inputs.

Code to reproduce:

import { createServer } from 'node:http2';

import createExpressApp from 'express';
import http2ExpressBridge from 'http2-express-bridge';

// Test with: curl --insecure -v -k --http2-prior-knowledge "http://[::]:8080"
function main(): void {
  const app = http2ExpressBridge(createExpressApp);

  app.get('*', (request, response) => {
    response.setHeader('Content-Type', 'text/plain');
    response.status(200);
    response.write('Server reached');
    response.end();
  });

  const server = createServer(app);

  server.listen({ port: 8080 });

  // Triggered by CMD/CTRL + C .
  process.on('SIGINT', () => {
    server.close();
    process.exit(0);
  });
}

try {
  main();
} catch (error) {
  console.error(error);
  process.exit(1);
}

when I run the server and then query it with curl --insecure -v -k --http2-prior-knowledge "http://[::]:8080" it crashes with a short stacktrace deep within node that I'm unable to debug/troubleshoot.

How often does it reproduce? Is there a required condition?

Happens every time.

What is the expected behavior? Why is that the expected behavior?

When I add an SSL cert to this server, and run createSecureServer instead of createServer, everything work fine.

web-platform % curl --insecure -v -k --http2-prior-knowledge "http://[::]:8080"
Server reached%

What do you see instead?

Instead curl hangs, and the server crashes with the following stack trace:

> tsx src/example/http2BugRepro.ts

node:events:492
      throw er; // Unhandled 'error' event
      ^

TypeError: Cannot read properties of undefined (reading 'readable')
    at IncomingMessage._read (node:_http_incoming:214:19)
    at Readable.read (node:internal/streams/readable:547:12)
    at resume_ (node:internal/streams/readable:1048:12)
    at process.processTicksAndRejections (node:internal/process/task_queues:82:21)
Emitted 'error' event on IncomingMessage instance at:
    at emitErrorNT (node:internal/streams/destroy:151:8)
    at errorOrDestroy (node:internal/streams/destroy:214:7)
    at Readable.read (node:internal/streams/readable:549:7)
    at resume_ (node:internal/streams/readable:1048:12)
    at process.processTicksAndRejections (node:internal/process/task_queues:82:21)

Additional information

As you can see this only happens when there's no SSL and stracktrace contains no other pointer for me to latch onto.

pulkit-30 commented 9 months ago

Please confirm this issue and what should be the expected behavior. I would love to work on this issue.

mctrafik commented 9 months ago

@pulkit-30 Are you asking me to confirm something, or was your reply an internal conversation?

pulkit-30 commented 8 months ago

@pulkit-30 Are you asking me to confirm something

not you @mctrafik, to @nodejs/http2

Primexz commented 7 months ago

This appears to be an issue in the "http2-express-bridge" library. There already seems to be a PR for this: https://github.com/rahulramesha/http2-express-bridge/pull/9

mctrafik commented 7 months ago

@Primexz Great find. Thanks.

I'm not versed in this code, but is there something to be done on the node side to indicate that the exception is thrown in a lib?