quic: resolving more outstanding TODO comments

deps/nghttp3/lib/nghttp3_ringbuf.c

@@ -33,21 +33,25 @@
 
 #include "nghttp3_macro.h"
 
-#if defined(_MSC_VER) && defined(_M_ARM64)
-unsigned int __popcnt(unsigned int x) {
+#if defined(_WIN32)
+#  if defined(_M_ARM64)
+unsigned int __nghttp3_popcnt(unsigned int x) {
   unsigned int c = 0;
   for (; x; ++c) {
     x &= x - 1;
   }
   return c;
 }
+#  else
+#    define __nghttp3_popcnt __popcnt
+#  endif
 #endif
 
 int nghttp3_ringbuf_init(nghttp3_ringbuf *rb, size_t nmemb, size_t size,
                          const nghttp3_mem *mem) {
   if (nmemb) {
 #ifdef WIN32
-    assert(1 == __popcnt((unsigned int)nmemb));
+    assert(1 == __nghttp3_popcnt((unsigned int)nmemb));
 #else
     assert(1 == __builtin_popcount((unsigned int)nmemb));
 #endif
@@ -127,7 +131,7 @@ int nghttp3_ringbuf_reserve(nghttp3_ringbuf *rb, size_t nmemb) {
   }
 
 #ifdef WIN32
-  assert(1 == __popcnt((unsigned int)nmemb));
+  assert(1 == __nghttp3_popcnt((unsigned int)nmemb));
 #else
   assert(1 == __builtin_popcount((unsigned int)nmemb));
 #endif

deps/ngtcp2/lib/ngtcp2_ringbuf.c

@@ -31,20 +31,24 @@
 
 #include "ngtcp2_macro.h"
 
-#if defined(_MSC_VER) && defined(_M_ARM64)
-unsigned int __popcnt(unsigned int x) {
+#if defined(_WIN32)
+#  if defined(_M_ARM64)
+unsigned int __ngtcp2_popcnt(unsigned int x) {
   unsigned int c = 0;
   for (; x; ++c) {
     x &= x - 1;
   }
   return c;
 }
+#  else
+#    define __ngtcp2_popcnt __popcnt
+#  endif
 #endif
 
 int ngtcp2_ringbuf_init(ngtcp2_ringbuf *rb, size_t nmemb, size_t size,
                         const ngtcp2_mem *mem) {
 #ifdef WIN32
-  assert(1 == __popcnt((unsigned int)nmemb));
+  assert(1 == __ngtcp2_popcnt((unsigned int)nmemb));
 #else
   assert(1 == __builtin_popcount((unsigned int)nmemb));
 #endif

src/quic/node_quic_http3_application.cc

@@ -646,7 +646,7 @@ void Http3Application::BeginHeaders(
 // by the QuicStream until stream->EndHeaders() is called, during which
 // the collected headers are converted to an array and passed off to
 // the javascript side.
-bool Http3Application::ReceiveHeader(
+void Http3Application::ReceiveHeader(
     int64_t stream_id,
     int32_t token,
     nghttp3_rcbuf* name,
@@ -671,9 +671,9 @@ bool Http3Application::ReceiveHeader(
         name,
         value,
         flags);
-    return stream->AddHeader(std::move(header));
+    // At this level, we don't care if the header is added or not.
+    USE(stream->AddHeader(std::move(header)));
   }
-  return true;
 }
 
 // Marks the completion of a headers block.
@@ -830,10 +830,8 @@ int Http3Application::OnReceiveHeader(
     void* conn_user_data,
     void* stream_user_data) {
   Http3Application* app = static_cast<Http3Application*>(conn_user_data);
-  // TODO(@jasnell): Need to determine the appropriate response code here
-  // for when the header is not going to be accepted.
-  return app->ReceiveHeader(stream_id, token, name, value, flags) ?
-      0 : NGHTTP3_ERR_CALLBACK_FAILURE;
+  app->ReceiveHeader(stream_id, token, name, value, flags);
+  return 0;
 }
 
 int Http3Application::OnEndHeaders(
@@ -868,8 +866,7 @@ int Http3Application::OnReceivePushPromise(
     void* conn_user_data,
     void* stream_user_data) {
   Http3Application* app = static_cast<Http3Application*>(conn_user_data);
-  if (!app->ReceiveHeader(stream_id, token, name, value, flags))
-    return NGHTTP3_ERR_CALLBACK_FAILURE;
+  app->ReceiveHeader(stream_id, token, name, value, flags);
   return 0;
 }
 

src/quic/node_quic_http3_application.h

@@ -174,7 +174,7 @@ class Http3Application final :
   void BeginHeaders(
       int64_t stream_id,
       QuicStreamHeadersKind kind = QUICSTREAM_HEADERS_KIND_NONE);
-  bool ReceiveHeader(
+  void ReceiveHeader(
       int64_t stream_id,
       int32_t token,
       nghttp3_rcbuf* name,

src/quic/node_quic_session.cc

@@ -178,12 +178,8 @@ std::string QuicSession::RemoteTransportParamsDebug::ToString() const {
       out += "  Original Connection ID: N/A \n";
     }
 
-    if (params.preferred_address_present) {
-      out += "  Preferred Address Present: Yes\n";
-      // TODO(@jasnell): Serialize the IPv4 and IPv6 address options
-    } else {
-      out += "  Preferred Address Present: No\n";
-    }
+    out += "  Preferred Address Present: " +
+           params.preferred_address_present ? "Yes" : "No";
 
     if (params.stateless_reset_token_present) {
       StatelessResetToken token(params.stateless_reset_token);
@@ -3617,8 +3613,6 @@ void QuicSession::OnQlogWrite(
 BaseObjectPtr<QLogStream> QLogStream::Create(Environment* env) {
   HandleScope scope(env->isolate());
 
-  // TODO(@jasnell): There is identical code in heap_utils for the
-  // HeapSnapshotStream. We can consolidate the two.
   if (env->qlogoutputstream_constructor_template().IsEmpty()) {
     // Create FunctionTemplate for QLogStream
     Local<FunctionTemplate> os = FunctionTemplate::New(env->isolate());

src/quic/node_quic_socket.cc

@@ -542,19 +542,17 @@ void QuicSocket::OnReceive(
       // a stateless reset. The stateless reset contains a token derived
       // from the received destination connection ID.
       //
-      // TODO(@jasnell): Stateless resets are generated programmatically
-      // using HKDF with the sender provided dcid and a locally provided
-      // secret as input. It is entirely possible that a malicious
-      // peer could send multiple stateless reset eliciting packets
-      // with the specific intent of using the returned stateless
-      // reset to guess the stateless reset token secret used by
-      // the server. Once guessed, the malicious peer could use
+      // Stateless resets are generated programmatically using HKDF with
+      // the sender provided dcid and a locally provided secret as input.
+      // It is entirely possible that a malicious peer could send multiple
+      // stateless reset eliciting packets with the specific intent of using
+      // the returned stateless reset to guess the stateless reset token
+      // secret used by the server. Once guessed, the malicious peer could use
       // that secret as a DOS vector against other peers. We currently
       // implement some mitigations for this by limiting the number
       // of stateless resets that can be sent to a specific remote
-      // address but there are other possible mitigations, such as
-      // including the remote address as input in the generation of
-      // the stateless token.
+      // address and we generate a random nonce used in creation of the
+      // token.
       if (is_short_header &&
           SendStatelessReset(dcid, local_addr, remote_addr, nread)) {
         Debug(this, "Sent stateless reset");
@@ -620,7 +618,7 @@ void QuicSocket::SendVersionNegotiation(
   SendPacket(local_addr, remote_address, std::move(packet));
 }
 
-// Possible generates and sends a stateless reset packet.
+// Possibly generates and sends a stateless reset packet.
 // This is terminal for the connection. It is possible
 // that a malicious packet triggered this so we need to
 // be careful not to commit too many resources.