From f612a6dd5cb17d8a6bd6b6332348b94839cf2ed0 Mon Sep 17 00:00:00 2001
From: Anatoli Papirovski <apapirovski@mac.com>
Date: Wed, 30 Aug 2017 10:12:24 -0400
Subject: [PATCH] http2: handle 100-continue flow & writeContinue

Adds an implementation for writeContinue based on the h2 spec & the
existing http implementation. ClientHttp2Stream now also emits
a continue event when it receives headers with :status 100.
Includes two test cases for default server continue behaviour and
for the exposed checkContinue listener.

PR-URL: https://github.com/nodejs/node/pull/15039
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
---
 doc/api/http2.md                              | 62 +++++++++++++-
 lib/internal/http2/compat.js                  | 11 ++-
 lib/internal/http2/core.js                    |  8 ++
 ...test-http2-compat-expect-continue-check.js | 81 +++++++++++++++++++
 .../test-http2-compat-expect-continue.js      | 66 +++++++++++++++
 5 files changed, 223 insertions(+), 5 deletions(-)
 create mode 100644 test/parallel/test-http2-compat-expect-continue-check.js
 create mode 100644 test/parallel/test-http2-compat-expect-continue.js

diff --git a/doc/api/http2.md b/doc/api/http2.md
index 72ca5cc806950c..8d521705a7fa03 100755
--- a/doc/api/http2.md
+++ b/doc/api/http2.md
@@ -849,6 +849,15 @@ used exclusively on HTTP/2 Clients. `Http2Stream` instances on the client
 provide events such as `'response'` and `'push'` that are only relevant on
 the client.
 
+#### Event: 'continue'
+<!-- YAML
+added: REPLACEME
+-->
+
+Emitted when the server sends a `100 Continue` status, usually because
+the request contained `Expect: 100-continue`. This is an instruction that
+the client should send the request body.
+
 #### Event: 'headers'
 <!-- YAML
 added: v8.4.0
@@ -1306,6 +1315,28 @@ added: v8.4.0
 The `'timeout'` event is emitted when there is no activity on the Server for
 a given number of milliseconds set using `http2server.setTimeout()`.
 
+#### Event: 'checkContinue'
+<!-- YAML
+added: REPLACEME
+-->
+
+* `request` {http2.Http2ServerRequest}
+* `response` {http2.Http2ServerResponse}
+
+If a [`'request'`][] listener is registered or [`'http2.createServer()'`][] is
+supplied a callback function, the `'checkContinue'` event is emitted each time
+a request with an HTTP `Expect: 100-continue` is received. If this event is
+not listened for, the server will automatically respond with a status
+`100 Continue` as appropriate.
+
+Handling this event involves calling [`response.writeContinue()`][] if the client
+should continue to send the request body, or generating an appropriate HTTP
+response (e.g. 400 Bad Request) if the client should not continue to send the
+request body.
+
+Note that when this event is emitted and handled, the [`'request'`][] event will
+not be emitted.
+
 ### Class: Http2SecureServer
 <!-- YAML
 added: v8.4.0
@@ -1389,6 +1420,28 @@ per session. See the [Compatibility API](compatiblity-api).
 added: v8.4.0
 -->
 
+#### Event: 'checkContinue'
+<!-- YAML
+added: REPLACEME
+-->
+
+* `request` {http2.Http2ServerRequest}
+* `response` {http2.Http2ServerResponse}
+
+If a [`'request'`][] listener is registered or [`'http2.createSecureServer()'`][]
+is supplied a callback function, the `'checkContinue'` event is emitted each
+time a request with an HTTP `Expect: 100-continue` is received. If this event
+is not listened for, the server will automatically respond with a status
+`100 Continue` as appropriate.
+
+Handling this event involves calling [`response.writeContinue()`][] if the client
+should continue to send the request body, or generating an appropriate HTTP
+response (e.g. 400 Bad Request) if the client should not continue to send the
+request body.
+
+Note that when this event is emitted and handled, the [`'request'`][] event will
+not be emitted.
+
 ### http2.createServer(options[, onRequestHandler])
 <!-- YAML
 added: v8.4.0
@@ -2537,8 +2590,9 @@ buffer. Returns `false` if all or part of the data was queued in user memory.
 added: v8.4.0
 -->
 
-Throws an error as the `'continue'` flow is not current implemented. Added for
-parity with [HTTP/1]().
+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])
 <!-- YAML
@@ -2618,6 +2672,7 @@ if the stream is closed.
 [Settings Object]: #http2_settings_object
 [Using options.selectPadding]: #http2_using_options_selectpadding
 [Writable Stream]: stream.html#stream_writable_streams
+[`'checkContinue'`]: #http2_event_checkcontinue
 [`'request'`]: #http2_event_request
 [`'unknownProtocol'`]: #http2_event_unknownprotocol
 [`ClientHttp2Stream`]: #http2_class_clienthttp2stream
@@ -2628,7 +2683,9 @@ if the stream is closed.
 [`ServerRequest`]: #http2_class_server_request
 [`TypeError`]: errors.html#errors_class_typeerror
 [`http2.SecureServer`]: #http2_class_http2secureserver
+[`http2.createSecureServer()`]: #http2_createsecureserver_options_onrequesthandler
 [`http2.Server`]: #http2_class_http2server
+[`http2.createServer()`]: #http2_createserver_options_onrequesthandler
 [`net.Socket`]: net.html#net_class_net_socket
 [`request.socket.getPeerCertificate()`]: tls.html#tls_tlssocket_getpeercertificate_detailed
 [`response.end()`]: #http2_response_end_data_encoding_callback
@@ -2636,6 +2693,7 @@ if the stream is closed.
 [`response.socket`]: #http2_response_socket
 [`response.write()`]: #http2_response_write_chunk_encoding_callback
 [`response.write(data, encoding)`]: http.html#http_response_write_chunk_encoding_callback
+[`response.writeContinue()`]: #http2_response_writecontinue
 [`response.writeHead()`]: #http2_response_writehead_statuscode_statusmessage_headers
 [`stream.pushStream()`]: #http2_stream-pushstream
 [`tls.TLSSocket`]: tls.html#tls_class_tls_tlssocket
diff --git a/lib/internal/http2/compat.js b/lib/internal/http2/compat.js
index 470a765b6bec3a..e9a02c5c1bb3f1 100644
--- a/lib/internal/http2/compat.js
+++ b/lib/internal/http2/compat.js
@@ -529,9 +529,14 @@ class Http2ServerResponse extends Stream {
     this.emit('finish');
   }
 
+  // TODO doesn't support callbacks
   writeContinue() {
-    // TODO mcollina check what is the continue flow
-    throw new Error('not implemented yet');
+    const stream = this[kStream];
+    if (stream === undefined) return false;
+    this[kStream].additionalHeaders({
+      [constants.HTTP2_HEADER_STATUS]: constants.HTTP_STATUS_CONTINUE
+    });
+    return true;
   }
 }
 
@@ -556,7 +561,7 @@ function onServerStream(stream, headers, flags) {
       if (server.listenerCount('checkContinue')) {
         server.emit('checkContinue', request, response);
       } else {
-        response.sendContinue();
+        response.writeContinue();
         server.emit('request', request, response);
       }
     } else if (server.listenerCount('checkExpectation')) {
diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js
index d45acb29f1f699..492a2baa5fb77e 100755
--- a/lib/internal/http2/core.js
+++ b/lib/internal/http2/core.js
@@ -120,6 +120,7 @@ const {
   HTTP2_METHOD_HEAD,
   HTTP2_METHOD_CONNECT,
 
+  HTTP_STATUS_CONTINUE,
   HTTP_STATUS_CONTENT_RESET,
   HTTP_STATUS_OK,
   HTTP_STATUS_NO_CONTENT,
@@ -2113,10 +2114,17 @@ class ClientHttp2Stream extends Http2Stream {
     this[kState].headersSent = true;
     if (id !== undefined)
       this[kInit](id);
+    this.on('headers', handleHeaderContinue);
     debug(`[${sessionName(session[kType])}] clienthttp2stream created`);
   }
 }
 
+function handleHeaderContinue(headers) {
+  if (headers[HTTP2_HEADER_STATUS] === HTTP_STATUS_CONTINUE) {
+    this.emit('continue');
+  }
+}
+
 const setTimeout = {
   configurable: true,
   enumerable: true,
diff --git a/test/parallel/test-http2-compat-expect-continue-check.js b/test/parallel/test-http2-compat-expect-continue-check.js
new file mode 100644
index 00000000000000..a956697b5cb5a7
--- /dev/null
+++ b/test/parallel/test-http2-compat-expect-continue-check.js
@@ -0,0 +1,81 @@
+// Flags: --expose-http2
+'use strict';
+
+const common = require('../common');
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+const assert = require('assert');
+const http2 = require('http2');
+
+const testResBody = 'other stuff!\n';
+
+// Checks the full 100-continue flow from client sending 'expect: 100-continue'
+// through server receiving it, triggering 'checkContinue' custom handler,
+// writing the rest of the request to finally the client receiving to.
+
+function handler(req, res) {
+  console.error('Server sent full response');
+
+  res.writeHead(200, {
+    'content-type': 'text/plain',
+    'abcd': '1'
+  });
+  res.end(testResBody);
+}
+
+const server = http2.createServer(
+  common.mustNotCall('Full request received before 100 Continue')
+);
+
+server.on('checkContinue', common.mustCall((req, res) => {
+  console.error('Server received Expect: 100-continue');
+
+  res.writeContinue();
+
+  // timeout so that we allow the client to receive continue first
+  setTimeout(
+    common.mustCall(() => handler(req, res)),
+    common.platformTimeout(100)
+  );
+}));
+
+server.listen(0);
+
+server.on('listening', common.mustCall(() => {
+  let body = '';
+
+  const port = server.address().port;
+  const client = http2.connect(`http://localhost:${port}`);
+  const req = client.request({
+    ':method': 'POST',
+    ':path': '/world',
+    expect: '100-continue'
+  });
+  console.error('Client sent request');
+
+  let gotContinue = false;
+  req.on('continue', common.mustCall(() => {
+    console.error('Client received 100-continue');
+    gotContinue = true;
+  }));
+
+  req.on('response', common.mustCall((headers) => {
+    console.error('Client received response headers');
+
+    assert.strictEqual(gotContinue, true);
+    assert.strictEqual(headers[':status'], 200);
+    assert.strictEqual(headers['abcd'], '1');
+  }));
+
+  req.setEncoding('utf-8');
+  req.on('data', common.mustCall((chunk) => { body += chunk; }));
+
+  req.on('end', common.mustCall(() => {
+    console.error('Client received full response');
+
+    assert.strictEqual(body, testResBody);
+
+    client.destroy();
+    server.close();
+  }));
+}));
diff --git a/test/parallel/test-http2-compat-expect-continue.js b/test/parallel/test-http2-compat-expect-continue.js
new file mode 100644
index 00000000000000..291a70e0f27317
--- /dev/null
+++ b/test/parallel/test-http2-compat-expect-continue.js
@@ -0,0 +1,66 @@
+// Flags: --expose-http2
+'use strict';
+
+const common = require('../common');
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+const assert = require('assert');
+const http2 = require('http2');
+
+const testResBody = 'other stuff!\n';
+
+// Checks the full 100-continue flow from client sending 'expect: 100-continue'
+// through server receiving it, sending back :status 100, writing the rest of
+// the request to finally the client receiving to.
+
+const server = http2.createServer();
+
+let sentResponse = false;
+
+server.on('request', common.mustCall((req, res) => {
+  console.error('Server sent full response');
+
+  res.end(testResBody);
+  sentResponse = true;
+}));
+
+server.listen(0);
+
+server.on('listening', common.mustCall(() => {
+  let body = '';
+
+  const port = server.address().port;
+  const client = http2.connect(`http://localhost:${port}`);
+  const req = client.request({
+    ':method': 'POST',
+    ':path': '/world',
+    expect: '100-continue'
+  });
+  console.error('Client sent request');
+
+  let gotContinue = false;
+  req.on('continue', common.mustCall(() => {
+    console.error('Client received 100-continue');
+    gotContinue = true;
+  }));
+
+  req.on('response', common.mustCall((headers) => {
+    console.error('Client received response headers');
+
+    assert.strictEqual(gotContinue, true);
+    assert.strictEqual(sentResponse, true);
+    assert.strictEqual(headers[':status'], 200);
+  }));
+
+  req.setEncoding('utf8');
+  req.on('data', common.mustCall((chunk) => { body += chunk; }));
+
+  req.on('end', common.mustCall(() => {
+    console.error('Client received full response');
+
+    assert.strictEqual(body, testResBody);
+
+    client.destroy();
+    server.close();
+  }));
+}));