| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
1 parent 0f9332a commit d9c9b62
2 files changed
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -1154,8 +1154,14 @@ int Http2Session::OnInvalidFrame(nghttp2_session* handle, | |||
| 1154 | 1154 | // The GOAWAY frame includes an error code that indicates the type of error" | |
| 1155 | 1155 | // The GOAWAY frame is already sent by nghttp2. We emit the error | |
| 1156 | 1156 | // to liberate the Http2Session to destroy. | |
| 1157 | + // | ||
| 1158 | + // ERR_FLOW_CONTROL: A WINDOW_UPDATE on stream 0 pushed the connection-level | ||
| 1159 | + // flow control window past 2^31-1. nghttp2 sends GOAWAY internally but | ||
| 1160 | + // without propagating this error the Http2Session would never be destroyed, | ||
| 1161 | + // causing a memory leak. | ||
| 1157 | 1162 | if (nghttp2_is_fatal(lib_error_code) || | |
| 1158 | 1163 | lib_error_code == NGHTTP2_ERR_STREAM_CLOSED || | |
| 1164 | + lib_error_code == NGHTTP2_ERR_FLOW_CONTROL || | ||
| 1159 | 1165 | lib_error_code == NGHTTP2_ERR_PROTO) { | |
| 1160 | 1166 | Environment* env = session->env(); | |
| 1161 | 1167 | Isolate* isolate = env->isolate(); | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -0,0 +1,84 @@ | |||
| 1 | + 'use strict'; | ||
| 2 | + | ||
| 3 | + const common = require('../common'); | ||
| 4 | + | ||
| 5 | + if (!common.hasCrypto) | ||
| 6 | + common.skip('missing crypto'); | ||
| 7 | + | ||
| 8 | + const http2 = require('http2'); | ||
| 9 | + const net = require('net'); | ||
| 10 | + | ||
| 11 | + // Regression test: a connection-level WINDOW_UPDATE that causes the flow | ||
| 12 | + // control window to exceed 2^31-1 must destroy the Http2Session (not leak it). | ||
| 13 | + // | ||
| 14 | + // nghttp2 responds with GOAWAY(FLOW_CONTROL_ERROR) internally but previously | ||
| 15 | + // Node's OnInvalidFrame callback only propagated errors for | ||
| 16 | + // NGHTTP2_ERR_STREAM_CLOSED and NGHTTP2_ERR_PROTO. The missing | ||
| 17 | + // NGHTTP2_ERR_FLOW_CONTROL case left the session unreachable after the GOAWAY, | ||
| 18 | + // causing a memory leak. | ||
| 19 | + | ||
| 20 | + const server = http2.createServer(); | ||
| 21 | + | ||
| 22 | + server.on('session', common.mustCall((session) => { | ||
| 23 | + session.on('error', common.mustCall()); | ||
| 24 | + session.on('close', common.mustCall(() => server.close())); | ||
| 25 | + })); | ||
| 26 | + | ||
| 27 | + server.listen(0, common.mustCall(() => { | ||
| 28 | + const conn = net.connect({ | ||
| 29 | + port: server.address().port, | ||
| 30 | + allowHalfOpen: true, | ||
| 31 | + }); | ||
| 32 | + | ||
| 33 | + // HTTP/2 client connection preface. | ||
| 34 | + conn.write('PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n'); | ||
| 35 | + | ||
| 36 | + // Empty SETTINGS frame (9-byte header, 0-byte payload). | ||
| 37 | + const settingsFrame = Buffer.alloc(9); | ||
| 38 | + settingsFrame[3] = 0x04; // type: SETTINGS | ||
| 39 | + conn.write(settingsFrame); | ||
| 40 | + | ||
| 41 | + let inbuf = Buffer.alloc(0); | ||
| 42 | + let state = 'settingsHeader'; | ||
| 43 | + let settingsFrameLength; | ||
| 44 | + | ||
| 45 | + conn.on('data', (chunk) => { | ||
| 46 | + inbuf = Buffer.concat([inbuf, chunk]); | ||
| 47 | + | ||
| 48 | + switch (state) { | ||
| 49 | + case 'settingsHeader': | ||
| 50 | + if (inbuf.length < 9) return; | ||
| 51 | + settingsFrameLength = inbuf.readUIntBE(0, 3); | ||
| 52 | + inbuf = inbuf.slice(9); | ||
| 53 | + state = 'readingSettings'; | ||
| 54 | + // Fallthrough | ||
| 55 | + case 'readingSettings': { | ||
| 56 | + if (inbuf.length < settingsFrameLength) return; | ||
| 57 | + inbuf = inbuf.slice(settingsFrameLength); | ||
| 58 | + state = 'done'; | ||
| 59 | + | ||
| 60 | + // ACK the server SETTINGS. | ||
| 61 | + const ack = Buffer.alloc(9); | ||
| 62 | + ack[3] = 0x04; // type: SETTINGS | ||
| 63 | + ack[4] = 0x01; // flag: ACK | ||
| 64 | + conn.write(ack); | ||
| 65 | + | ||
| 66 | + // WINDOW_UPDATE on stream 0 (connection level) with increment 2^31-1. | ||
| 67 | + // Default connection window is 65535, so the new total would be | ||
| 68 | + // 65535 + 2147483647 = 2147549182 > 2^31-1, triggering | ||
| 69 | + // NGHTTP2_ERR_FLOW_CONTROL inside nghttp2. | ||
| 70 | + const windowUpdate = Buffer.alloc(13); | ||
| 71 | + windowUpdate.writeUIntBE(4, 0, 3); // length = 4 | ||
| 72 | + windowUpdate[3] = 0x08; // type: WINDOW_UPDATE | ||
| 73 | + windowUpdate[4] = 0x00; // flags: none | ||
| 74 | + windowUpdate.writeUIntBE(0, 5, 4); // stream id: 0 | ||
| 75 | + windowUpdate.writeUIntBE(0x7FFFFFFF, 9, 4); // increment: 2^31-1 | ||
| 76 | + conn.write(windowUpdate); | ||
| 77 | + } | ||
| 78 | + } | ||
| 79 | + }); | ||
| 80 | + | ||
| 81 | + // The server must close the connection after sending GOAWAY. | ||
| 82 | + conn.on('end', common.mustCall(() => conn.end())); | ||
| 83 | + conn.on('close', common.mustCall()); | ||
| 84 | + })); | ||
| Back | FazBrowse Home | New Git URL |
0 commit comments