http2: near full http1 compatibility, add tests

Extensive re-work of http1 compatibility layer based on tests in
express, on-finished and finalhandler. Fix handling of HEAD
method to match http1. Adjust write, end, etc. to call writeHead
as in http1 and as expected by user-land modules. Add socket
proxy that instead uses the Http2Stream for the vast majority of
socket interactions. Add and change tests to closer represent
http1 behaviour.

Refs: #15633
Refs: https://github.com/expressjs/express/tree/master/test
Refs: https://github.com/jshttp/on-finished/blob/master/test/test.js
Refs: https://github.com/pillarjs/finalhandler/blob/master/test/test.js
PR-URL: #15702
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>

Author: apapirovski

doc/api/errors.md

@@ -722,6 +722,12 @@ SETTINGS. By default, a maximum number of un-acknowledged `SETTINGS` frame may
 be sent at any given time. This error code is used when that limit has been
 reached.
 
+<a id="ERR_HTTP2_NO_SOCKET_MANIPULATION"></a>
+### ERR_HTTP2_NO_SOCKET_MANIPULATION
+
+Used when attempting to read, write, pause, and/or resume a socket attached to
+an `Http2Session`.
+
 <a id="ERR_HTTP2_OUT_OF_STREAMS"></a>
 ### ERR_HTTP2_OUT_OF_STREAMS
 

doc/api/http2.md

@@ -2049,7 +2049,7 @@ console.log(request.headers);
 
 See [Headers Object][].
 
-### request.httpVersion
+#### request.httpVersion
 <!-- YAML
 added: v8.4.0
 -->
@@ -2120,7 +2120,14 @@ added: v8.4.0
 * `msecs` {number}
 * `callback` {Function}
 
-Calls `request.connection.setTimeout(msecs, callback)`.
+Sets the [`Http2Stream`]()'s timeout value to `msecs`.  If a callback is
+provided, then it is added as a listener on the `'timeout'` event on
+the response object.
+
+If no `'timeout'` listener is added to the request, the response, or
+the server, then [`Http2Stream`]()s are destroyed when they time out. If a
+handler is assigned to the request, the response, or the server's `'timeout'`
+events, timed out sockets must be handled explicitly.
 
 Returns `request`.
 
@@ -2131,13 +2138,24 @@ added: v8.4.0
 
 * {net.Socket}
 
-The [`net.Socket`][] object associated with the connection.
+Returns a Proxy object that acts as a `net.Socket` but applies getters,
+setters and methods based on HTTP/2 logic.
+
+`destroyed`, `readable`, and `writable` properties will be retrieved from and
+set on `request.stream`.
 
-With TLS support, use [`request.socket.getPeerCertificate()`][] to obtain the
-client's authentication details.
+`destroy`, `emit`, `end`, `on` and `once` methods will be called on
+`request.stream`.
 
-*Note*: do not use this socket object to send or receive any data. All
-data transfers are managed by HTTP/2 and data might be lost.
+`setTimeout` method will be called on `request.stream.session`.
+
+`pause`, `read`, `resume`, and `write` will throw an error with code
+`ERR_HTTP2_NO_SOCKET_MANIPULATION`. See [`Http2Session and Sockets`][] for
+more information.
+
+All other interactions will be routed directly to the socket. With TLS support,
+use [`request.socket.getPeerCertificate()`][] to obtain the client's
+authentication details.
 
 #### request.stream
 <!-- YAML
@@ -2235,15 +2253,15 @@ passed as the second parameter to the [`'request'`][] event.
 The response implements, but does not inherit from, the [Writable Stream][]
 interface. This is an [`EventEmitter`][] with the following events:
 
-### Event: 'close'
+#### Event: 'close'
 <!-- YAML
 added: v8.4.0
 -->
 
 Indicates that the underlying [`Http2Stream`]() was terminated before
 [`response.end()`][] was called or able to flush.
 
-### Event: 'finish'
+#### Event: 'finish'
 <!-- YAML
 added: v8.4.0
 -->
@@ -2255,7 +2273,7 @@ does not imply that the client has received anything yet.
 
 After this event, no more events will be emitted on the response object.
 
-### response.addTrailers(headers)
+#### response.addTrailers(headers)
 <!-- YAML
 added: v8.4.0
 -->
@@ -2268,7 +2286,7 @@ message) to the response.
 Attempting to set a header field name or value that contains invalid characters
 will result in a [`TypeError`][] being thrown.
 
-### response.connection
+#### response.connection
 <!-- YAML
 added: v8.4.0
 -->
@@ -2277,7 +2295,7 @@ added: v8.4.0
 
 See [`response.socket`][].
 
-### response.end([data][, encoding][, callback])
+#### response.end([data][, encoding][, callback])
 <!-- YAML
 added: v8.4.0
 -->
@@ -2296,7 +2314,7 @@ If `data` is specified, it is equivalent to calling
 If `callback` is specified, it will be called when the response stream
 is finished.
 
-### response.finished
+#### response.finished
 <!-- YAML
 added: v8.4.0
 -->
@@ -2306,7 +2324,7 @@ added: v8.4.0
 Boolean value that indicates whether the response has completed. Starts
 as `false`. After [`response.end()`][] executes, the value will be `true`.
 
-### response.getHeader(name)
+#### response.getHeader(name)
 <!-- YAML
 added: v8.4.0
 -->
@@ -2323,7 +2341,7 @@ Example:
 const contentType = response.getHeader('content-type');
 ```
 
-### response.getHeaderNames()
+#### response.getHeaderNames()
 <!-- YAML
 added: v8.4.0
 -->
@@ -2343,7 +2361,7 @@ const headerNames = response.getHeaderNames();
 // headerNames === ['foo', 'set-cookie']
 ```
 
-### response.getHeaders()
+#### response.getHeaders()
 <!-- YAML
 added: v8.4.0
 -->
@@ -2371,7 +2389,7 @@ const headers = response.getHeaders();
 // headers === { foo: 'bar', 'set-cookie': ['foo=bar', 'bar=baz'] }
 ```
 
-### response.hasHeader(name)
+#### response.hasHeader(name)
 <!-- YAML
 added: v8.4.0
 -->
@@ -2388,7 +2406,7 @@ Example:
 const hasContentType = response.hasHeader('content-type');
 ```
 
-### response.headersSent
+#### response.headersSent
 <!-- YAML
 added: v8.4.0
 -->
@@ -2397,7 +2415,7 @@ added: v8.4.0
 
 Boolean (read-only). True if headers were sent, false otherwise.
 
-### response.removeHeader(name)
+#### response.removeHeader(name)
 <!-- YAML
 added: v8.4.0
 -->
@@ -2412,7 +2430,7 @@ Example:
 response.removeHeader('Content-Encoding');
 ```
 
-### response.sendDate
+#### response.sendDate
 <!-- YAML
 added: v8.4.0
 -->
@@ -2425,7 +2443,7 @@ the response if it is not already present in the headers. Defaults to true.
 This should only be disabled for testing; HTTP requires the Date header
 in responses.
 
-### response.setHeader(name, value)
+#### response.setHeader(name, value)
 <!-- YAML
 added: v8.4.0
 -->
@@ -2466,7 +2484,7 @@ const server = http2.createServer((req, res) => {
 });
 ```
 
-### response.setTimeout(msecs[, callback])
+#### response.setTimeout(msecs[, callback])
 <!-- YAML
 added: v8.4.0
 -->
@@ -2485,18 +2503,29 @@ events, timed out sockets must be handled explicitly.
 
 Returns `response`.
 
-### response.socket
+#### response.socket
 <!-- YAML
 added: v8.4.0
 -->
 
 * {net.Socket}
 
-Reference to the underlying socket. Usually users will not want to access
-this property. In particular, the socket will not emit `'readable'` events
-because of how the protocol parser attaches to the socket. After
-`response.end()`, the property is nulled. The `socket` may also be accessed
-via `response.connection`.
+Returns a Proxy object that acts as a `net.Socket` but applies getters,
+setters and methods based on HTTP/2 logic.
+
+`destroyed`, `readable`, and `writable` properties will be retrieved from and
+set on `response.stream`.
+
+`destroy`, `emit`, `end`, `on` and `once` methods will be called on
+`response.stream`.
+
+`setTimeout` method will be called on `response.stream.session`.
+
+`pause`, `read`, `resume`, and `write` will throw an error with code
+`ERR_HTTP2_NO_SOCKET_MANIPULATION`. See [`Http2Session and Sockets`][] for
+more information.
+
+All other interactions will be routed directly to the socket.
 
 Example:
 
@@ -2509,7 +2538,7 @@ const server = http2.createServer((req, res) => {
 }).listen(3000);
 ```
 
-### response.statusCode
+#### response.statusCode
 <!-- YAML
 added: v8.4.0
 -->
@@ -2529,7 +2558,7 @@ response.statusCode = 404;
 After response header was sent to the client, this property indicates the
 status code which was sent out.
 
-### response.statusMessage
+#### response.statusMessage
 <!-- YAML
 added: v8.4.0
 -->
@@ -2548,7 +2577,7 @@ added: v8.4.0
 
 The [`Http2Stream`][] object backing the response.
 
-### response.write(chunk[, encoding][, callback])
+#### response.write(chunk[, encoding][, callback])
 <!-- YAML
 added: v8.4.0
 -->
@@ -2586,7 +2615,7 @@ Returns `true` if the entire data was flushed successfully to the kernel
 buffer. Returns `false` if all or part of the data was queued in user memory.
 `'drain'` will be emitted when the buffer is free again.
 
-### response.writeContinue()
+#### response.writeContinue()
 <!-- YAML
 added: v8.4.0
 -->
@@ -2595,7 +2624,7 @@ Sends a status `100 Continue` to the client, indicating that the request body
 should be sent. See the [`'checkContinue'`][] event on `Http2Server` and
 `Http2SecureServer`.
 
-### response.writeHead(statusCode[, statusMessage][, headers])
+#### response.writeHead(statusCode[, statusMessage][, headers])
 <!-- YAML
 added: v8.4.0
 -->
@@ -2651,7 +2680,7 @@ const server = http2.createServer((req, res) => {
 Attempting to set a header field name or value that contains invalid characters
 will result in a [`TypeError`][] being thrown.
 
-### response.createPushResponse(headers, callback)
+#### response.createPushResponse(headers, callback)
 <!-- YAML
 added: v8.4.0
 -->

lib/internal/errors.js

@@ -173,6 +173,9 @@ E('ERR_HTTP2_INVALID_SETTING_VALUE',
 E('ERR_HTTP2_INVALID_STREAM', 'The stream has been destroyed');
 E('ERR_HTTP2_MAX_PENDING_SETTINGS_ACK',
   (max) => `Maximum number of pending settings acknowledgements (${max})`);
+E('ERR_HTTP2_NO_SOCKET_MANIPULATION',
+  'HTTP/2 sockets should not be directly read from, written to, ' +
+  'paused and/or resumed.');
 E('ERR_HTTP2_OUT_OF_STREAMS',
   'No stream ID is available because maximum stream ID has been reached');
 E('ERR_HTTP2_PAYLOAD_FORBIDDEN',