From 52eec8ff4b9f6aef07f97438e03df4485bea6808 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Sat, 16 Jun 2018 23:07:31 +0200
Subject: [PATCH] http2: fix memory leak when headers are not emitted
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

When headers are not emitted to JS, e.g. because of an error
before that could happen, we currently still have the vector of
previously received headers lying around, each one holding
a reference count of 1.

To fix the resulting memory leak, release them in the `Http2Stream`
destructor.

Also, clear the vector of headers once they have been emitted –
there’s not need to keep it around, wasting memory.
---
 src/node_http2.cc | 13 ++++++++-----
 src/node_http2.h  |  8 ++------
 2 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/src/node_http2.cc b/src/node_http2.cc
index 28d3de6ef0ce97..572f9e1b69de8a 100644
--- a/src/node_http2.cc
+++ b/src/node_http2.cc
@@ -1127,8 +1127,7 @@ void Http2Session::HandleHeadersFrame(const nghttp2_frame* frame) {
   if (stream->IsDestroyed())
     return;
 
-  nghttp2_header* headers = stream->headers();
-  size_t count = stream->headers_count();
+  std::vector<nghttp2_header> headers(stream->move_headers());
 
   Local<String> name_str;
   Local<String> value_str;
@@ -1145,9 +1144,9 @@ void Http2Session::HandleHeadersFrame(const nghttp2_frame* frame) {
   // this way for performance reasons (it's faster to generate and pass an
   // array than it is to generate and pass the object).
   size_t n = 0;
-  while (count > 0) {
+  while (n < headers.size()) {
     size_t j = 0;
-    while (count > 0 && j < arraysize(argv) / 2) {
+    while (n < headers.size() && j < arraysize(argv) / 2) {
       nghttp2_header item = headers[n++];
       // The header name and value are passed as external one-byte strings
       name_str =
@@ -1156,7 +1155,6 @@ void Http2Session::HandleHeadersFrame(const nghttp2_frame* frame) {
           ExternalHeader::New<false>(env(), item.value).ToLocalChecked();
       argv[j * 2] = name_str;
       argv[j * 2 + 1] = value_str;
-      count--;
       j++;
     }
     // For performance, we pass name and value pairs to array.protototype.push
@@ -1710,6 +1708,11 @@ Http2Stream::Http2Stream(
 
 
 Http2Stream::~Http2Stream() {
+  for (nghttp2_header& header : current_headers_) {
+    nghttp2_rcbuf_decref(header.name);
+    nghttp2_rcbuf_decref(header.value);
+  }
+
   if (session_ == nullptr)
     return;
   Debug(this, "tearing down stream");
diff --git a/src/node_http2.h b/src/node_http2.h
index 70c284c357f0ea..3d79ca9bf81b28 100644
--- a/src/node_http2.h
+++ b/src/node_http2.h
@@ -592,18 +592,14 @@ class Http2Stream : public AsyncWrap,
 
   bool AddHeader(nghttp2_rcbuf* name, nghttp2_rcbuf* value, uint8_t flags);
 
-  inline nghttp2_header* headers() {
-    return current_headers_.data();
+  inline std::vector<nghttp2_header> move_headers() {
+    return std::move(current_headers_);
   }
 
   inline nghttp2_headers_category headers_category() const {
     return current_headers_category_;
   }
 
-  inline size_t headers_count() const {
-    return current_headers_.size();
-  }
-
   void StartHeaders(nghttp2_headers_category category);
 
   // Required for StreamBase