koa icon indicating copy to clipboard operation
koa copied to clipboard

[fix] Request.secure returns false for HTTP2 connections after recieving RST_STREAM

Open ctusch opened this issue 2 years ago • 1 comments

Describe the bug

Node.js version: 19.4.0

OS version: Windows 10

Description: I'm using koa with http2.createSecureServer and during some requests Request.secure turns from initial true to false. I've seen that the property indirectly checks whether Request.req.socket.encrypted is truthy. This is only true if socket is of type TLSSocket which is initially the case. But when the bug occurs Http2ServerRequest.socket switches its underlying type to ServerHttp2Stream.

As far as I can tell it has to do with the client sending a RST_STREAM and node.js closing the HttpStream (aborted event is triggered on the stream). But I would argue this should either leave the secure flag true or throw an error when trying to access it. Solely relying on socket.encrypted doesn't seem to be enough for HTTP2.

I've stumbled upon this error while trying to set a secure cookie which lead to a pretty misleading error: Cannot send secure cookie over unencrypted connection.

Actual behavior

Request.secure is false.

Expected behavior

Request.secure should be true or throw an error.

Code to reproduce

const http2 = require('http2');
const fs = require('fs');
const Koa = require('koa');

const app = new Koa();

app.use(async (context) => {
  await new Promise(res => setTimeout(res, 100));
  console.log('socket: ' + context.socket.constructor.name)
  console.log('secure: ' + context.secure);
  return context.response.body = 'test';
});

const cert = fs.readFileSync('localhost.crt');
const key = fs.readFileSync('localhost.key');

http2.createSecureServer({ cert, key }, app.callback()).listen(1234);
const client = http2.connect('https://localhost:1234', { ca: cert });

const successfulRequest = client.request();
successfulRequest.on('data', (chunk) => console.log(chunk.toString()));

const cancelledRequest = client.request();
cancelledRequest.on('ready', () => cancelledRequest.destroy());

// output:
// socket: bound TLSSocket
// secure: true
// socket: bound ServerHttp2Stream
// secure: false
// test

Checklist

  • [x] I have searched through GitHub issues for similar issues.
  • [x] I have completely read through the README and documentation.
  • [x] I have tested my code with the latest version of Node.js and this package and confirmed it is still not working.

ctusch avatar Jan 12 '23 17:01 ctusch

I've added code to reproduce.

ctusch avatar Jan 16 '23 14:01 ctusch