http2 crash on h2spec check

[songdongsheng]

• Version:

PS C:\> node --version
v14.5.0

• Platform:

Microsoft Windows [Version 10.0.17763.1294]

What steps will reproduce the bug?

1. Just run a demo http2 server

const fs = require('fs');
const server = require('http2').createSecureServer({
  cert: fs.readFileSync('localhost-cert.pem'),
  key: fs.readFileSync('localhost-privkey.pem')
});

server.on('error', (err) => console.error(err));

server.on('stream', (stream, headers) => {
  stream.respond({
    'content-type': 'application/json',
    ':status': 200
  });
  stream.end('{"hello":"world"}');
});

server.listen(8181);

2. run h2spec

• https:/summerwind/h2spec/releases/tag/v2.5.0

PS C:\> h2spec --version
Version: 2.5.0 (8b58a5e5cc9590ac7a470dc1c01065ae575bbf25)

PS C:\> h2spec.exe -kt -p 8181
Generic tests for HTTP/2 server
  1. Starting HTTP/2
    ✔ 1: Sends a client connection preface

  2. Streams and Multiplexing
    ✔ 1: Sends a PRIORITY frame on idle stream
    ✔ 2: Sends a WINDOW_UPDATE frame on half-closed (remote) stream
    ✔ 3: Sends a PRIORITY frame on half-closed (remote) stream
    ✔ 4: Sends a RST_STREAM frame on half-closed (remote) stream
    ✔ 5: Sends a PRIORITY frame on closed stream

  3. Frame Definitions
    3.1. DATA
      ✔ 1: Sends a DATA frame
      ✔ 2: Sends multiple DATA frames
      ✔ 3: Sends a DATA frame with padding

    3.2. HEADERS
      ✔ 1: Sends a HEADERS frame
      ✔ 2: Sends a HEADERS frame with padding
      ✔ 3: Sends a HEADERS frame with priority

    3.3. PRIORITY
      ✔ 1: Sends a PRIORITY frame with priority 1
      ✔ 2: Sends a PRIORITY frame with priority 256
      ✔ 3: Sends a PRIORITY frame with stream dependency
      ✔ 4: Sends a PRIORITY frame with exclusive
      ✔ 5: Sends a PRIORITY frame for an idle stream, then send a HEADER frame for a lower stream
 ID

    3.4. RST_STREAM
      ✔ 1: Sends a RST_STREAM frame

    3.5. SETTINGS
      ✔ 1: Sends a SETTINGS frame

    3.7. PING
      ✔ 1: Sends a PING frame

    3.8. GOAWAY
      ✔ 1: Sends a GOAWAY frame

    3.9. WINDOW_UPDATE
      ✔ 1: Sends a WINDOW_UPDATE frame with stream ID 0
      ✔ 2: Sends a WINDOW_UPDATE frame with stream ID 1

    3.10. CONTINUATION
      ✔ 1: Sends a CONTINUATION frame
      ✔ 2: Sends multiple CONTINUATION frames

  4. HTTP Message Exchanges
    ✔ 1: Sends a GET request
    × 2: Sends a HEAD request
      -> The endpoint MUST respond to the request.
         Expected: HEADERS Frame (stream_id:1)
           Actual: Connection closed
    × 3: Sends a POST request

Error: dial tcp 127.0.0.1:8181: connectex: No connection could be made because the target machine actively refused it.

PS C:\> h2spec --version
Version: 2.5.0 (8b58a5e5cc9590ac7a470dc1c01065ae575bbf25)

3. nodejs crashed

PS C:\> node .\index.js

events.js:291
      throw er; // Unhandled 'error' event
      ^

Error [ERR_STREAM_WRITE_AFTER_END]: write after end
    at ServerHttp2Stream.Writable.write (_stream_writable.js:292:11)
    at ServerHttp2Stream.Writable.end (_stream_writable.js:555:10)
    at Http2SecureServer.<anonymous> (C:\var\vcs\git\misc\scratch\nodejs\node-h2-demo\index.js:18:10)
    at Http2SecureServer.emit (events.js:314:20)
    at ServerHttp2Session.sessionOnStream (internal/http2/core.js:2769:19)
    at ServerHttp2Session.emit (events.js:314:20)
    at emit (internal/http2/core.js:291:8)
    at processTicksAndRejections (internal/process/task_queues.js:83:22)
Emitted 'error' event on ServerHttp2Stream instance at:
    at emitErrorNT (internal/streams/destroy.js:100:8)
    at processTicksAndRejections (internal/process/task_queues.js:80:21) {
  code: 'ERR_STREAM_WRITE_AFTER_END'
}

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

It crashed every time.

What is the expected behavior?

h2spec should be passed all tests.

At least the node js process should not crash.

What do you see instead?

node js process crashed every time.

[mcollina]

cc @nodejs/http2

[bzoz]

Also reproduces on Linux.

[qballer]

I'm going to start working on this tonight. Will update once I have more information.

[qballer]

Just an update report:

I have build and test running of the node project. I'm working on reconstructing the bug on my own machine.
Also reviewing rfc7540 to make sure I understand the details of the protocol before I fix something in the implementation (interesting stuff).

Few questions as it is my first attempt to contribute to node:

• Should the patch go to branch v14.x or master? both?
• Is there a document that explains how to debug node? I see in the makefile how to provide debug symbols.

[mcollina]

It's probably better if you ask those question on a separate issue in this repo. The folks that are maintaining the project tooling are not likely to watch this issue.

[qballer]

I've currently paused work on this issue if someone else wants to pick it up. It's important to note that h2spec is able to fail the HTTP2 module in several ways. issues also occur if you apply the following commands:

h2spec -kt -p 8181 http2/5 
h2spec -kt -p 8181 http2/8

change the last digit in command to run corresponding section tests of rfc7540

[szmarczak]

The write after end error happens because Node.js automatically responds ends on HEAD.

node/lib/internal/http2/core.js

Lines 367 to 372 in 306c7a5

	if (obj[HTTP2_HEADER_METHOD] === HTTP2_METHOD_HEAD) {
	// For head requests, there must not be a body...
	// end the writable side immediately.
	stream.end();
	stream[kState].flags |= STREAM_FLAGS_HEAD_REQUEST;
	}