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 insecure server crashes every time. #5347

Open
mctrafik opened this issue Dec 8, 2023 · 9 comments
Open

HTTP2 insecure server crashes every time. #5347

mctrafik opened this issue Dec 8, 2023 · 9 comments

Comments

@mctrafik
Copy link

mctrafik commented Dec 8, 2023

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

I need an insecure HTTP2 server. Here's my code:

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);
}

I'm testing using curl --insecure -v -k --http2-prior-knowledge "http://[::]:8080"

Every time I execute it, the server crashes with:

> 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)

When I add an SSL certificate and use createSecureServer everything works fine. But when I use createServer it crashes.

I think it's a bug in express or http2-express-bridge because without specifying the app, and handling the stream directly produces no errors.

My info:

OS: MacOS
Node: 20.9.0
Express: I tried v5.0.0-beta and v4.18.2
http2-express-bridge: 1.0.7

Thank you.

@mctrafik
Copy link
Author

mctrafik commented Dec 8, 2023

Small correction: This fails for secure servers in express 5 as well. Different stack though:

 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) 
TypeError: Cannot set properties of undefined (setting 'content-security-policy')
    at ServerResponse.setHeader (node:_http_outgoing:661:45)
    at Immediate.write (/somepath/web-platform/node_modules/finalhandler/index.js:285:9)
    at process.processImmediate (node:internal/timers:480:21) 

@UlisesGascon
Copy link
Member

I will suggest to open an issue on http2-express-bridge, It seems that the issue might be related to the library.

@LBYPatrick
Copy link

Because Express.JS cannot work with HTTP/2 natively, and http2-express-bridge is no longer maintained. If you really need the extra efficiency provided by HTTP/2, then switching to something like fastify makes more sense.

@mikegwhit
Copy link

i would not mind contributing to create a separate version of express that works with HTTP2, backwards compatibility be damned.

i would also not mind authoring the change if we can get about $10k-20k into a kickstarter for it. seems suitable and would likely deliver a lot more back to the community.

@LBYPatrick
Copy link

It would be awesome if HTTP/2 ever gets added to express.js! Though, if you are asking for money from kickstarters to kick off the process, I guess:

  1. Others (Springboot, Fastify, even nginx!) have done it and are available to the public for free. You might need a convincing story to get this money, good luck!
  2. Actually, ask IBM and StrangeLoop to see if they will offer the money or simply a position at their companies! This will definitely catch their attentions!

@mikegwhit
Copy link

mikegwhit commented Jan 15, 2024

Yeah, well, a startup I consulted for ran out of cash in December and I need a job :)

But no, I also want to see this happen. The last company I worked at informally sponsored Matteo you could say to continue working on Fastify. I could go to them, but this seems like it would stir some pots. If not them, a similar organization.

I am sitting on a large monorepository solution without a safe way to go to market without getting ripped off by Vercel, the powers and IP thiefs at large. This seems like a reasonable way to gain some notoriety and while I seriously would just sit down for 3-6 months and do this, I burned out my bank account.

I authored hot reloading in express without doing a server reload on top, and helping with HTTP2 could build trust to push this and similar changes into Express v5 as a PR(s). I do more stuff that allows route repositioning and declarative route ordering across a monorepository (where construction of route order is not always known in advance). Basically I have done a lot of additional work to make route positioning more dynamic that might be interesting to propose into Express.

This is me, inventing a job for myself. Scratching at the food line. This is tech.

@mikegwhit
Copy link

See PR : #3730

I can pick up where this one left off (6 years ago!).. speed in Express is important to me.

@QuantGeekDev
Copy link

See PR : #3730

I can pick up where this one left off (6 years ago!).. speed in Express is important to me.

Hey! Can I join you? I'd love to work on this with you

@mikegwhit
Copy link

Yes let's chat @QuantGeekDev i added you in LI

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

No branches or pull requests

5 participants