quic: minor refactor for QuicStream::Header

PR-URL: #207
Reviewed-By: #207

Author: jasnell

lib/internal/quic/core.js

@@ -534,8 +534,8 @@ function onStreamError(streamHandle, error) {
 // received for the stream. Not all QuicStreams
 // will support headers. The headers argument
 // here is an Array of name-value pairs.
-function onStreamHeaders(headers, kind) {
-  this[owner_symbol][kHeaders](headers, kind);
+function onStreamHeaders(id, headers, kind) {
+  this[owner_symbol][kHeaders](id, headers, kind);
 }
 
 function onSessionSilentClose(statelessReset, code, family) {
@@ -1491,6 +1491,14 @@ class QuicSession extends EventEmitter {
     stream.destroy();
   }
 
+  [kHeaders](id, headers, kind) {
+    const stream = this.#streams.get(id);
+    if (stream === undefined)
+      return;
+
+    stream[kHeaders](headers, kind);
+  }
+
   [kStreamReset](id, code, finalSize) {
     const stream = this.#streams.get(id);
     if (stream === undefined)

src/node_quic_http3_application.h

@@ -49,7 +49,7 @@ constexpr size_t DEFAULT_MAX_PUSHES = 65535;
 
 using Http3ConnectionPointer = DeleteFnPtr<nghttp3_conn, nghttp3_conn_del>;
 
-class Http3Header : public QuicStream::Header {
+class Http3Header : public QuicHeader {
  public:
   Http3Header(int32_t token, nghttp3_rcbuf* name, nghttp3_rcbuf* value);
   Http3Header(Http3Header&& other) noexcept :

src/node_quic_session.cc

@@ -699,6 +699,38 @@ void QuicCryptoContext::WriteHandshake(
   handshake_[level].Push(std::move(buffer));
 }
 
+void QuicApplication::StreamHeaders(
+    int64_t stream_id,
+    int kind,
+    std::vector<std::unique_ptr<QuicHeader>>* headers) {
+  HandleScope scope(env()->isolate());
+  Context::Scope context_scope(env()->context());
+  std::vector<Local<Value>> head;
+  for (const auto& header : *headers) {
+    // name and value should never be empty here, and if
+    // they are, there's an actual bug so go ahead and crash
+    Local<Value> pair[] = {
+      header->GetName(Session()->Application()).ToLocalChecked(),
+      header->GetValue(Session()->Application()).ToLocalChecked()
+    };
+    head.push_back(Array::New(env()->isolate(), pair, arraysize(pair)));
+  }
+  Local<Value> argv[] = {
+      Number::New(env()->isolate(), static_cast<double>(stream_id)),
+      Array::New(
+          env()->isolate(),
+          head.data(),
+          head.size()),
+      Integer::New(
+          env()->isolate(),
+          kind)
+  };
+  BaseObjectPtr<QuicSession> ptr(Session());
+  Session()->MakeCallback(
+      env()->quic_on_stream_headers_function(),
+      arraysize(argv), argv);
+}
+
 void QuicApplication::StreamClose(
     int64_t stream_id,
     uint64_t app_error_code) {

src/node_quic_session.h

@@ -32,6 +32,7 @@ using ConnectionPointer = DeleteFnPtr<ngtcp2_conn, ngtcp2_conn_del>;
 
 class QuicSocket;
 class QuicStream;
+class QuicHeader;
 
 enum class QlogMode {
   kDisabled,
@@ -361,6 +362,10 @@ class QuicApplication : public MemoryRetainer {
   virtual void ExtendMaxStreamData(int64_t stream_id, uint64_t max_data) {}
   virtual bool SendPendingData() = 0;
   virtual bool SendStreamData(QuicStream* stream) = 0;
+  virtual void StreamHeaders(
+      int64_t stream_id,
+      int kind,
+      std::vector<std::unique_ptr<QuicHeader>>* headers);
   virtual void StreamClose(
       int64_t stream_id,
       uint64_t app_error_code);

src/node_quic_stream.cc

@@ -387,7 +387,7 @@ void QuicStream::SetHeadersKind(QuicStreamHeadersKind kind) {
   headers_kind_ = kind;
 }
 
-bool QuicStream::AddHeader(std::unique_ptr<Header> header) {
+bool QuicStream::AddHeader(std::unique_ptr<QuicHeader> header) {
   Debug(this, "Header Added");
   headers_.emplace_back(std::move(header));
   // TODO(@jasnell): We need to limit the maximum number of headers.
@@ -400,28 +400,8 @@ void QuicStream::EndHeaders() {
   // Upon completion of a block of headers, convert the
   // vector of Header objects into an array of name+value
   // pairs, then call the on_stream_headers function.
-  std::vector<Local<Value>> headers;
-  for (const auto& header : headers_) {
-    // name and value should never be empty here, and if
-    // they are, there's an actual bug so go ahead and crash
-    Local<Value> pair[] = {
-      header->GetName(Session()->Application()).ToLocalChecked(),
-      header->GetValue(Session()->Application()).ToLocalChecked()
-    };
-    headers.push_back(Array::New(env()->isolate(), pair, arraysize(pair)));
-  }
-  Local<Value> argv[] = {
-      Array::New(
-          env()->isolate(),
-          headers.data(),
-          headers.size()),
-      Integer::New(
-          env()->isolate(),
-          headers_kind_)
-  };
+  Session()->Application()->StreamHeaders(GetID(), headers_kind_, &headers_);
   headers_.clear();
-  BaseObjectPtr<QuicStream> ptr(this);
-  MakeCallback(env()->quic_on_stream_headers_function(), arraysize(argv), argv);
 }
 
 void QuicStream::MemoryInfo(MemoryTracker* tracker) const {

src/node_quic_stream.h

@@ -37,6 +37,20 @@ enum QuicStreamHeadersKind : int {
   QUICSTREAM_HEADERS_KIND_TRAILING
 };
 
+// QuicHeader is a base class for implementing QUIC application
+// specific headers. Each type of QUIC application may have
+// different internal representations for a header name+value
+// pair. QuicApplication implementations that support headers
+// per stream must create a specialization of the Header class.
+class QuicHeader {
+  public:
+  QuicHeader() {}
+
+  virtual ~QuicHeader() {}
+  virtual v8::MaybeLocal<v8::String> GetName(QuicApplication* app) const = 0;
+  virtual v8::MaybeLocal<v8::String> GetValue(QuicApplication* app) const = 0;
+};
+
 // QuicStream's are simple data flows that, fortunately, do not
 // require much. They may be:
 //
@@ -105,20 +119,6 @@ enum QuicStreamHeadersKind : int {
 // ngtcp2 level.
 class QuicStream : public AsyncWrap, public StreamBase {
  public:
-  // Header is a base class for implementing QUIC application
-  // specific headers. Each type of QUIC application may have
-  // different internal representations for a header name+value
-  // pair. QuicApplication implementations that support headers
-  // per stream must create a specialization of the Header class.
-  class Header {
-   public:
-    Header() {}
-
-    virtual ~Header() {}
-    virtual v8::MaybeLocal<v8::String> GetName(QuicApplication* app) const = 0;
-    virtual v8::MaybeLocal<v8::String> GetValue(QuicApplication* app) const = 0;
-  };
-
   enum QuicStreamStates : uint32_t {
     // QuicStream is fully open. Readable and Writable
     QUICSTREAM_FLAG_INITIAL = 0x0,
@@ -361,7 +361,7 @@ class QuicStream : public AsyncWrap, public StreamBase {
   // Returns false if the header cannot be added. This will
   // typically only happen if a maximimum number of headers
   // has been reached.
-  bool AddHeader(std::unique_ptr<Header> header);
+  bool AddHeader(std::unique_ptr<QuicHeader> header);
   void EndHeaders();
 
   // Sets the kind of headers currently being processed.
@@ -428,7 +428,7 @@ class QuicStream : public AsyncWrap, public StreamBase {
   size_t available_outbound_length_ = 0;
   size_t inbound_consumed_data_while_paused_ = 0;
 
-  std::vector<std::unique_ptr<Header>> headers_;
+  std::vector<std::unique_ptr<QuicHeader>> headers_;
   QuicStreamHeadersKind headers_kind_;
 
   struct stream_stats {