http2: do not create ArrayBuffers when no DATA received

Lazily allocate `ArrayBuffer`s for the contents of DATA frames. Creating `ArrayBuffer`s is, sadly, not a cheap operation with V8. This is part of performance improvements to mitigate CVE-2019-9513. Together with the previous commit, these changes improve throughput in the adversarial case by about 100 %, and there is little more that we can do besides artificially limiting the rate of incoming metadata frames (i.e. after this patch, CPU usage is virtually exclusively in libnghttp2).
nodejs · addaleax · Aug 14, 2019 · Aug 5, 2019 · Aug 9, 2019 · Aug 9, 2019
commit d642202a68cc677608dd693d49ff04d1c8ead85d
diff --git a/src/node_http2.cc b/src/node_http2.cc
@@ -1259,7 +1259,13 @@ void Http2StreamListener::OnStreamRead(ssize_t nread, const uv_buf_t& buf) {
     return;
   }
 
-  CHECK(!session->stream_buf_ab_.IsEmpty());
+  Local<ArrayBuffer> ab;
+  if (session->stream_buf_ab_.IsEmpty()) {
+    ab = session->stream_buf_allocation_.ToArrayBuffer();
+    session->stream_buf_ab_.Reset(env->isolate(), ab);
+  } else {
+    ab = PersistentToLocal::Strong(session->stream_buf_ab_);
+  }
 
   // There is a single large array buffer for the entire data read from the
   // network; create a slice of that array buffer and emit it as the
@@ -1270,7 +1276,7 @@ void Http2StreamListener::OnStreamRead(ssize_t nread, const uv_buf_t& buf) {
   CHECK_LE(offset, session->stream_buf_.len);
   CHECK_LE(offset + buf.len, session->stream_buf_.len);
 
-  stream->CallJSOnreadMethod(nread, session->stream_buf_ab_, offset);
+  stream->CallJSOnreadMethod(nread, ab, offset);
 }
 
 
@@ -1789,6 +1795,7 @@ void Http2Session::OnStreamRead(ssize_t nread, const uv_buf_t& buf_) {
   Http2Scope h2scope(this);
   CHECK_NOT_NULL(stream_);
   Debug(this, "receiving %d bytes", nread);
+  CHECK_EQ(stream_buf_allocation_.size(), 0);
   CHECK(stream_buf_ab_.IsEmpty());
   AllocatedBuffer buf(env(), buf_);
 
@@ -1810,7 +1817,8 @@ void Http2Session::OnStreamRead(ssize_t nread, const uv_buf_t& buf_) {
     // We use `nread` instead of `buf.size()` here, because the buffer is
     // cleared as part of the `.ToArrayBuffer()` call below.
     DecrementCurrentSessionMemory(nread);
-    stream_buf_ab_ = Local<ArrayBuffer>();
+    stream_buf_ab_.Reset();
+    stream_buf_allocation_.clear();
     stream_buf_ = uv_buf_init(nullptr, 0);
   });
 
@@ -1824,9 +1832,10 @@ void Http2Session::OnStreamRead(ssize_t nread, const uv_buf_t& buf_) {
 
   Isolate* isolate = env()->isolate();
 
-  // Create an array buffer for the read data. DATA frames will be emitted
-  // as slices of this array buffer to avoid having to copy memory.
-  stream_buf_ab_ = buf.ToArrayBuffer();
+  // Store this so we can create an ArrayBuffer for read data from it.
+  // DATA frames will be emitted as slices of that ArrayBuffer to avoid having
+  // to copy memory.
+  stream_buf_allocation_ = std::move(buf);
 
   statistics_.data_received += nread;
   ssize_t ret = Write(&stream_buf_, 1);

diff --git a/src/node_http2.h b/src/node_http2.h
@@ -993,7 +993,8 @@ class Http2Session : public AsyncWrap, public StreamListener {
   uint32_t chunks_sent_since_last_write_ = 0;
 
   uv_buf_t stream_buf_ = uv_buf_init(nullptr, 0);
-  v8::Local<v8::ArrayBuffer> stream_buf_ab_;
+  v8::Global<v8::ArrayBuffer> stream_buf_ab_;
+  AllocatedBuffer stream_buf_allocation_;
 
   size_t max_outstanding_pings_ = DEFAULT_MAX_PINGS;
   std::queue<std::unique_ptr<Http2Ping>> outstanding_pings_;