quic: start re-enabling quic with openssl 3.5

Start working on re-enabling QUIC support with the availability
of OpenSSL 3.5. This will be a multi-step process.

Signed-off-by: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/59249
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

deps/ngtcp2/ngtcp2.gyp

@@ -48,7 +48,7 @@
       'ngtcp2/crypto/shared.c'
     ],
     'ngtcp2_sources_quictls': [
-      'ngtcp2/crypto/quictls/quictls.c'
+      #'ngtcp2/crypto/quictls/quictls.c'
     ],
     'ngtcp2_sources_boringssl': [
       'ngtcp2/crypto/boringssl/boringssl.c'

node.gyp

@@ -187,6 +187,8 @@
       'src/udp_wrap.cc',
       'src/util.cc',
       'src/uv.cc',
+      'src/quic/cid.cc',
+      'src/quic/data.cc',
       # headers to make for a more pleasant IDE experience
       'src/aliased_buffer.h',
       'src/aliased_buffer-inl.h',
@@ -323,6 +325,10 @@
       'src/udp_wrap.h',
       'src/util.h',
       'src/util-inl.h',
+      'src/quic/cid.h',
+      'src/quic/data.h',
+      'src/quic/defs.h',
+      'src/quic/guard.h',
     ],
     'node_crypto_sources': [
       'src/crypto/crypto_aes.cc',
@@ -379,8 +385,6 @@
     'node_quic_sources': [
       'src/quic/application.cc',
       'src/quic/bindingdata.cc',
-      'src/quic/cid.cc',
-      'src/quic/data.cc',
       'src/quic/endpoint.cc',
       'src/quic/http3.cc',
       'src/quic/logstream.cc',
@@ -394,8 +398,6 @@
       'src/quic/transportparams.cc',
       'src/quic/application.h',
       'src/quic/bindingdata.h',
-      'src/quic/cid.h',
-      'src/quic/data.h',
       'src/quic/endpoint.h',
       'src/quic/http3.h',
       'src/quic/logstream.h',

node.gypi

@@ -380,6 +380,8 @@
           'defines': [ 'OPENSSL_API_COMPAT=0x10100000L', ],
           'dependencies': [
             './deps/openssl/openssl.gyp:openssl',
+            './deps/ngtcp2/ngtcp2.gyp:ngtcp2',
+            './deps/ngtcp2/ngtcp2.gyp:nghttp3',
 
             # For tests
             './deps/openssl/openssl.gyp:openssl-cli',

src/quic/cid.cc

@@ -1,12 +1,14 @@
-#if HAVE_OPENSSL && NODE_OPENSSL_HAS_QUIC
+#if HAVE_OPENSSL
-#include "cid.h"
+#include "guard.h"
+#ifndef OPENSSL_NO_QUIC
 #include <crypto/crypto_util.h>
 #include <memory_tracker-inl.h>
 #include <node_mutex.h>
 #include <string_bytes.h>
+#include "cid.h"
+#include "defs.h"
 #include "nbytes.h"
 #include "ncrypto.h"
-#include "quic/defs.h"
 
 namespace node::quic {
 
@@ -77,7 +79,7 @@ std::string CID::ToString() const {
   return std::string(dest, written);
 }
 
-CID CID::kInvalid{};
+const CID CID::kInvalid{};
 
 // ============================================================================
 // CID::Hash
@@ -95,12 +97,12 @@ size_t CID::Hash::operator()(const CID& cid) const {
 // CID::Factory
 
 namespace {
-class RandomCIDFactory : public CID::Factory {
+class RandomCIDFactory final : public CID::Factory {
  public:
   RandomCIDFactory() = default;
   DISALLOW_COPY_AND_MOVE(RandomCIDFactory)
 
-  CID Generate(size_t length_hint) const override {
+  const CID Generate(size_t length_hint) const override {
     DCHECK_GE(length_hint, CID::kMinLength);
     DCHECK_LE(length_hint, CID::kMaxLength);
     Mutex::ScopedLock lock(mutex_);
@@ -110,7 +112,7 @@ class RandomCIDFactory : public CID::Factory {
     return CID(start, length_hint);
   }
 
-  CID GenerateInto(ngtcp2_cid* cid,
+  const CID GenerateInto(ngtcp2_cid* cid,
                          size_t length_hint = CID::kMaxLength) const override {
     DCHECK_GE(length_hint, CID::kMinLength);
     DCHECK_LE(length_hint, CID::kMaxLength);
@@ -135,7 +137,7 @@ class RandomCIDFactory : public CID::Factory {
     }
   }
 
-  static constexpr int kPoolSize = 4096;
+  static constexpr int kPoolSize = 1024 * 16;
   mutable int pos_ = kPoolSize;
   mutable uint8_t pool_[kPoolSize];
   mutable Mutex mutex_;
@@ -148,4 +150,5 @@ const CID::Factory& CID::Factory::random() {
 }
 
 }  // namespace node::quic
-#endif  // HAVE_OPENSSL && NODE_OPENSSL_HAS_QUIC
+#endif  // OPENSSL_NO_QUIC
+#endif  // HAVE_OPENSS

src/quic/cid.h

@@ -1,7 +1,7 @@
 #pragma once
 
 #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
-#if HAVE_OPENSSL && NODE_OPENSSL_HAS_QUIC
+
 #include <memory_tracker.h>
 #include <ngtcp2/ngtcp2.h>
 #include <string>
@@ -51,7 +51,7 @@ class CID final : public MemoryRetainer {
 
   CID(const CID& other);
   CID& operator=(const CID& other);
-  CID(CID&&) = delete;
+  DISALLOW_MOVE(CID)
 
   struct Hash final {
     size_t operator()(const CID& cid) const;
@@ -68,6 +68,8 @@ class CID final : public MemoryRetainer {
   operator bool() const;
   size_t length() const;
 
+  // Returns a hex-encoded string representation of the CID useful
+  // for debugging.
   std::string ToString() const;
 
   SET_NO_MEMORY_INFO()
@@ -75,7 +77,7 @@ class CID final : public MemoryRetainer {
   SET_SELF_SIZE(CID)
 
   template <typename T>
-  using Map = std::unordered_map<CID, T, CID::Hash>;
+  using Map = std::unordered_map<const CID, T, CID::Hash>;
 
   // A CID::Factory, as the name suggests, is used to create new CIDs.
   // Per https://datatracker.ietf.org/doc/draft-ietf-quic-load-balancers/, QUIC
@@ -85,13 +87,13 @@ class CID final : public MemoryRetainer {
   // but will allow user code to provide their own CID::Factory implementation.
   class Factory;
 
-  static CID kInvalid;
+  static const CID kInvalid;
 
   // The default constructor creates an empty, zero-length CID.
   // Zero-length CIDs are not usable. We use them as a placeholder
   // for a missing or empty CID value. This is public only because
   // it is required for the CID::Map implementation. It should not
-  // be used. Use kInvalid instead.
+  // be used directly. Use kInvalid instead.
   CID();
 
  private:
@@ -107,12 +109,12 @@ class CID::Factory {
 
   // Generate a new CID. The length_hint must be between CID::kMinLength
   // and CID::kMaxLength. The implementation can choose to ignore the length.
-  virtual CID Generate(size_t length_hint = CID::kMaxLength) const = 0;
+  virtual const CID Generate(size_t length_hint = CID::kMaxLength) const = 0;
 
   // Generate a new CID into the given ngtcp2_cid. This variation of
   // Generate should be used far less commonly.
-  virtual CID GenerateInto(ngtcp2_cid* cid,
+  virtual const CID GenerateInto(
-                           size_t length_hint = CID::kMaxLength) const = 0;
+      ngtcp2_cid* cid, size_t length_hint = CID::kMaxLength) const = 0;
 
   // The default random CID generator instance.
   static const Factory& random();
@@ -123,5 +125,4 @@ class CID::Factory {
 
 }  // namespace node::quic
 
-#endif  // HAVE_OPENSSL && NODE_OPENSSL_HAS_QUIC
 #endif  // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS

src/quic/data.cc

@@ -1,5 +1,6 @@
-#if HAVE_OPENSSL && NODE_OPENSSL_HAS_QUIC
+#if HAVE_OPENSSL
-
+#include "guard.h"
+#ifndef OPENSSL_NO_QUIC
 #include "data.h"
 #include <env-inl.h>
 #include <memory_tracker-inl.h>
@@ -13,15 +14,21 @@
 namespace node {
 
 using v8::Array;
+using v8::ArrayBuffer;
+using v8::ArrayBufferView;
+using v8::BackingStore;
 using v8::BigInt;
-using v8::Integer;
+using v8::Just;
 using v8::Local;
+using v8::Maybe;
 using v8::MaybeLocal;
+using v8::Nothing;
 using v8::Uint8Array;
 using v8::Undefined;
 using v8::Value;
 
 namespace quic {
+int DebugIndentScope::indent_ = 0;
 
 Path::Path(const SocketAddress& local, const SocketAddress& remote) {
   ngtcp2_addr_init(&this->local, local.data(), local.length());
@@ -50,9 +57,6 @@ std::string Path::ToString() const {
 PathStorage::PathStorage() {
   Reset();
 }
-PathStorage::operator ngtcp2_path() {
-  return path;
-}
 
 void PathStorage::Reset() {
   ngtcp2_path_storage_zero(this);
@@ -72,44 +76,54 @@ bool PathStorage::operator!=(const PathStorage& other) const {
 
 // ============================================================================
 
-Store::Store(std::shared_ptr<v8::BackingStore> store,
+Store::Store(std::shared_ptr<BackingStore> store, size_t length, size_t offset)
-             size_t length,
-             size_t offset)
     : store_(std::move(store)), length_(length), offset_(offset) {
   CHECK_LE(offset_, store_->ByteLength());
   CHECK_LE(length_, store_->ByteLength() - offset_);
 }
 
-Store::Store(std::unique_ptr<v8::BackingStore> store,
+Store::Store(std::unique_ptr<BackingStore> store, size_t length, size_t offset)
-             size_t length,
-             size_t offset)
     : store_(std::move(store)), length_(length), offset_(offset) {
   CHECK_LE(offset_, store_->ByteLength());
   CHECK_LE(length_, store_->ByteLength() - offset_);
 }
 
-Store::Store(Local<v8::ArrayBuffer> buffer, Option option)
+Maybe<Store> Store::From(
-    : Store(buffer->GetBackingStore(), buffer->ByteLength()) {
+    Local<ArrayBuffer> buffer,
-  if (option == Option::DETACH) {
+    Local<Value> detach_key) {
-    USE(buffer->Detach(Local<Value>()));
+  if (!buffer->IsDetachable()) {
+    return Nothing<Store>();
   }
+  bool res;
+  auto backing = buffer->GetBackingStore();
+  auto length = buffer->ByteLength();
+  if (!buffer->Detach(detach_key).To(&res) || !res) {
+    return Nothing<Store>();
+  }
+  return Just(Store(std::move(backing), length, 0));
 }
 
-Store::Store(Local<v8::ArrayBufferView> view, Option option)
+Maybe<Store> Store::From(
-    : Store(view->Buffer()->GetBackingStore(),
+    Local<ArrayBufferView> view,
-            view->ByteLength(),
+    Local<Value> detach_key) {
-            view->ByteOffset()) {
+  if (!view->Buffer()->IsDetachable()) {
-  if (option == Option::DETACH) {
+    return Nothing<Store>();
-    USE(view->Buffer()->Detach(Local<Value>()));
   }
+  bool res;
+  auto backing = view->Buffer()->GetBackingStore();
+  auto length = view->ByteLength();
+  auto offset = view->ByteOffset();
+  if (!view->Buffer()->Detach(detach_key).To(&res) || !res) {
+    return Nothing<Store>();
+  }
+  return Just(Store(std::move(backing), length, offset));
 }
 
 Local<Uint8Array> Store::ToUint8Array(Environment* env) const {
   return !store_
-             ? Uint8Array::New(v8::ArrayBuffer::New(env->isolate(), 0), 0, 0)
+             ? Uint8Array::New(ArrayBuffer::New(env->isolate(), 0), 0, 0)
-             : Uint8Array::New(v8::ArrayBuffer::New(env->isolate(), store_),
+             : Uint8Array::New(
-                               offset_,
+                   ArrayBuffer::New(env->isolate(), store_), offset_, length_);
-                               length_);
 }
 
 Store::operator bool() const {
@@ -119,25 +133,31 @@ size_t Store::length() const {
   return length_;
 }
 
-template <typename T, typename t>
+size_t Store::total_length() const {
+  return store_ ? store_->ByteLength() : 0;
+}
+
+template <typename T, OneByteType N>
 T Store::convert() const {
+  // We can only safely convert to T if we have a valid store.
+  CHECK(store_);
   T buf;
   buf.base =
-      store_ != nullptr ? static_cast<t*>(store_->Data()) + offset_ : nullptr;
+      store_ != nullptr ? static_cast<N*>(store_->Data()) + offset_ : nullptr;
   buf.len = length_;
   return buf;
 }
 
 Store::operator uv_buf_t() const {
-  return convert<uv_buf_t, char>();
+  return convert<uv_buf_t, typeof(*uv_buf_t::base)>();
 }
 
 Store::operator ngtcp2_vec() const {
-  return convert<ngtcp2_vec, uint8_t>();
+  return convert<ngtcp2_vec, typeof(*ngtcp2_vec::base)>();
 }
 
 Store::operator nghttp3_vec() const {
-  return convert<nghttp3_vec, uint8_t>();
+  return convert<nghttp3_vec, typeof(*ngtcp2_vec::base)>();
 }
 
 void Store::MemoryInfo(MemoryTracker* tracker) const {
@@ -147,18 +167,23 @@ void Store::MemoryInfo(MemoryTracker* tracker) const {
 // ============================================================================
 
 namespace {
-std::string TypeName(QuicError::Type type) {
+constexpr std::string_view TypeName(QuicError::Type type) {
   switch (type) {
     case QuicError::Type::APPLICATION:
-      return "APPLICATION";
+      return "application";
     case QuicError::Type::TRANSPORT:
-      return "TRANSPORT";
+      return "transport";
     case QuicError::Type::VERSION_NEGOTIATION:
-      return "VERSION_NEGOTIATION";
+      return "version_negotiation";
     case QuicError::Type::IDLE_CLOSE:
-      return "IDLE_CLOSE";
+      return "idle_close";
+    case QuicError::Type::DROP_CONNECTION:
+      return "drop_connection";
+    case QuicError::Type::RETRY:
+      return "retry";
+    default:
+      return "<unknown>";
   }
-  UNREACHABLE();
 }
 }  // namespace
 
@@ -167,6 +192,8 @@ QuicError::QuicError(const std::string& reason)
   ngtcp2_ccerr_default(&error_);
 }
 
+// Keep in mind that reason_ in each of the constructors here will copy
+// the string from the ngtcp2_ccerr input.
 QuicError::QuicError(const ngtcp2_ccerr* ptr)
     : reason_(reinterpret_cast<const char*>(ptr->reason), ptr->reasonlen),
       error_(),
@@ -177,14 +204,6 @@ QuicError::QuicError(const ngtcp2_ccerr& error)
       error_(error),
       ptr_(&error_) {}
 
-QuicError::operator bool() const {
-  if ((code() == QUIC_NO_ERROR && type() == Type::TRANSPORT) ||
-      ((code() == QUIC_APP_NO_ERROR && type() == Type::APPLICATION))) {
-    return false;
-  }
-  return true;
-}
-
 const uint8_t* QuicError::reason_c_str() const {
   return reinterpret_cast<const uint8_t*>(reason_.c_str());
 }
@@ -247,31 +266,45 @@ error_code QuicError::h3_liberr_to_code(int liberr) {
   return nghttp3_err_infer_quic_app_error_code(liberr);
 }
 
-bool QuicError::is_crypto() const {
+bool QuicError::is_crypto_error() const {
   return code() & NGTCP2_CRYPTO_ERROR;
 }
 
-std::optional<int> QuicError::crypto_error() const {
+std::optional<int> QuicError::get_crypto_error() const {
-  if (!is_crypto()) return std::nullopt;
+  if (!is_crypto_error()) return std::nullopt;
   return code() & ~NGTCP2_CRYPTO_ERROR;
 }
 
 MaybeLocal<Value> QuicError::ToV8Value(Environment* env) const {
   if ((type() == Type::TRANSPORT && code() == NGTCP2_NO_ERROR) ||
-      (type() == Type::APPLICATION && code() == NGTCP2_APP_NOERROR) ||
       (type() == Type::APPLICATION && code() == NGHTTP3_H3_NO_ERROR)) {
+    // Note that we only return undefined for *known* no-error application
+    // codes. It is possible that other application types use other specific
+    // no-error codes, but since we don't know which application is being used,
+    // we'll just return the error code value for those below.
     return Undefined(env->isolate());
   }
 
+  Local<Value> type_str;
+  if (!node::ToV8Value(env->context(), TypeName(type())).ToLocal(&type_str)) {
+    return {};
+  }
+
   Local<Value> argv[] = {
-      Integer::New(env->isolate(), static_cast<int>(type())),
+      type_str,
       BigInt::NewFromUnsigned(env->isolate(), code()),
       Undefined(env->isolate()),
   };
 
+  // Note that per the QUIC specification, the reason, if present, is
+  // expected to be UTF-8 encoded. The spec uses the term "SHOULD" here,
+  // which means that is is entirely possible that some QUIC implementation
+  // could choose a different encoding, in which case the conversion here
+  // will produce garbage. That's ok though, we're going to use the default
+  // assumption that the impl is following the guidelines.
   if (reason_.length() > 0 &&
       !node::ToV8Value(env->context(), reason()).ToLocal(&argv[2])) {
-    return MaybeLocal<Value>();
+    return {};
   }
 
   return Array::New(env->isolate(), argv, arraysize(argv)).As<Value>();
@@ -279,7 +312,8 @@ MaybeLocal<Value> QuicError::ToV8Value(Environment* env) const {
 
 std::string QuicError::ToString() const {
   std::string str = "QuicError(";
-  str += TypeName(type()) + ") ";
+  str += TypeName(type());
+  str += ") ";
   str += std::to_string(code());
   if (!reason_.empty()) str += ": " + reason_;
   return str;
@@ -289,53 +323,78 @@ void QuicError::MemoryInfo(MemoryTracker* tracker) const {
   tracker->TrackField("reason", reason_.length());
 }
 
-QuicError QuicError::ForTransport(error_code code, std::string reason) {
+const QuicError QuicError::ForTransport(TransportError code,
+                                        std::string reason) {
+  return ForTransport(static_cast<error_code>(code), std::move(reason));
+}
+
+const QuicError QuicError::ForTransport(error_code code, std::string reason) {
   QuicError error(std::move(reason));
   ngtcp2_ccerr_set_transport_error(
       &error.error_, code, error.reason_c_str(), error.reason().length());
   return error;
 }
 
-QuicError QuicError::ForApplication(error_code code, std::string reason) {
+const QuicError QuicError::ForApplication(Http3Error code, std::string reason) {
+  return ForApplication(static_cast<error_code>(code), std::move(reason));
+}
+
+const QuicError QuicError::ForApplication(error_code code, std::string reason) {
   QuicError error(std::move(reason));
   ngtcp2_ccerr_set_application_error(
       &error.error_, code, error.reason_c_str(), error.reason().length());
   return error;
 }
 
-QuicError QuicError::ForVersionNegotiation(std::string reason) {
+const QuicError QuicError::ForVersionNegotiation(std::string reason) {
   return ForNgtcp2Error(NGTCP2_ERR_RECV_VERSION_NEGOTIATION, std::move(reason));
 }
 
-QuicError QuicError::ForIdleClose(std::string reason) {
+const QuicError QuicError::ForIdleClose(std::string reason) {
   return ForNgtcp2Error(NGTCP2_ERR_IDLE_CLOSE, std::move(reason));
 }
 
-QuicError QuicError::ForNgtcp2Error(int code, std::string reason) {
+const QuicError QuicError::ForDropConnection(std::string reason) {
+  return ForNgtcp2Error(NGTCP2_ERR_DROP_CONN, std::move(reason));
+}
+
+const QuicError QuicError::ForRetry(std::string reason) {
+  return ForNgtcp2Error(NGTCP2_ERR_RETRY, std::move(reason));
+}
+
+const QuicError QuicError::ForNgtcp2Error(int code, std::string reason) {
   QuicError error(std::move(reason));
   ngtcp2_ccerr_set_liberr(
       &error.error_, code, error.reason_c_str(), error.reason().length());
   return error;
 }
 
-QuicError QuicError::ForTlsAlert(int code, std::string reason) {
+const QuicError QuicError::ForTlsAlert(int code, std::string reason) {
   QuicError error(std::move(reason));
   ngtcp2_ccerr_set_tls_alert(
       &error.error_, code, error.reason_c_str(), error.reason().length());
   return error;
 }
 
-QuicError QuicError::FromConnectionClose(ngtcp2_conn* session) {
+const QuicError QuicError::FromConnectionClose(ngtcp2_conn* session) {
   return QuicError(ngtcp2_conn_get_ccerr(session));
 }
 
-QuicError QuicError::TRANSPORT_NO_ERROR = ForTransport(QUIC_NO_ERROR);
+#define V(name)                                                                \
-QuicError QuicError::APPLICATION_NO_ERROR = ForApplication(QUIC_APP_NO_ERROR);
+  const QuicError QuicError::TRANSPORT_##name =                                \
-QuicError QuicError::VERSION_NEGOTIATION = ForVersionNegotiation();
+      ForTransport(TransportError::name);
-QuicError QuicError::IDLE_CLOSE = ForIdleClose();
+QUIC_TRANSPORT_ERRORS(V)
-QuicError QuicError::INTERNAL_ERROR = ForNgtcp2Error(NGTCP2_ERR_INTERNAL);
+#undef V
+
+const QuicError QuicError::HTTP3_NO_ERROR = ForApplication(NGHTTP3_H3_NO_ERROR);
+const QuicError QuicError::VERSION_NEGOTIATION = ForVersionNegotiation();
+const QuicError QuicError::IDLE_CLOSE = ForIdleClose();
+const QuicError QuicError::DROP_CONNECTION = ForDropConnection();
+const QuicError QuicError::RETRY = ForRetry();
+const QuicError QuicError::INTERNAL_ERROR = ForNgtcp2Error(NGTCP2_ERR_INTERNAL);
 
 }  // namespace quic
 }  // namespace node
 
-#endif  // HAVE_OPENSSL && NODE_OPENSSL_HAS_QUIC
+#endif  // OPENSSL_NO_QUIC
+#endif  // HAVE_OPENSSL

src/quic/data.h

@@ -1,29 +1,37 @@
 #pragma once
 
 #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
-#if HAVE_OPENSSL && NODE_OPENSSL_HAS_QUIC
 
 #include <env.h>
 #include <memory_tracker.h>
 #include <nghttp3/nghttp3.h>
 #include <ngtcp2/ngtcp2.h>
 #include <node_internals.h>
+#include <node_realm.h>
 #include <node_sockaddr.h>
 #include <v8.h>
+#include <concepts>
 #include <string>
 #include "defs.h"
 
 namespace node::quic {
 
+template <typename T>
+concept OneByteType = sizeof(T) == 1;
+
 struct Path final : public ngtcp2_path {
-  Path(const SocketAddress& local, const SocketAddress& remote);
+  explicit Path(const SocketAddress& local, const SocketAddress& remote);
-  inline operator ngtcp2_path*() { return this; }
+  Path(Path&& other) noexcept = default;
+  Path& operator=(Path&& other) noexcept = default;
+  DISALLOW_COPY(Path)
   std::string ToString() const;
 };
 
 struct PathStorage final : public ngtcp2_path_storage {
-  PathStorage();
+  explicit PathStorage();
-  operator ngtcp2_path();
+  PathStorage(PathStorage&& other) noexcept = default;
+  PathStorage& operator=(PathStorage&& other) noexcept = default;
+  DISALLOW_COPY(PathStorage)
 
   void Reset();
   void CopyTo(PathStorage* path) const;
@@ -32,6 +40,9 @@ struct PathStorage final : public ngtcp2_path_storage {
   bool operator!=(const PathStorage& other) const;
 };
 
+// Store acts as a wrapper around a v8::BackingStore, providing a convenient
+// abstraction to map it to various buffer types used in QUIC, HTTP/3, and
+// libuv, taking care of the necessary adjustments for length and offset.
 class Store final : public MemoryRetainer {
  public:
   Store() = default;
@@ -42,16 +53,30 @@ class Store final : public MemoryRetainer {
   Store(std::unique_ptr<v8::BackingStore> store,
         size_t length,
         size_t offset = 0);
+  Store(Store&& other) noexcept = default;
+  Store& operator=(Store&& other) noexcept = default;
+  DISALLOW_COPY(Store)
 
-  enum class Option {
+  // Creates a Store from the contents of an ArrayBuffer, always detaching
-    NONE,
+  // it in the process. An empty Maybe will be returned if the ArrayBuffer
-    DETACH,
+  // is not detachable or detaching failed (likely due to a detach key
-  };
+  // mismatch).
+  static v8::Maybe<Store> From(
+    v8::Local<v8::ArrayBuffer> buffer,
+    v8::Local<v8::Value> detach_key = v8::Local<v8::Value>());
 
-  Store(v8::Local<v8::ArrayBuffer> buffer, Option option = Option::NONE);
+  // Creates a Store from the contents of an ArrayBufferView, always detaching
-  Store(v8::Local<v8::ArrayBufferView> view, Option option = Option::NONE);
+  // it in the process. An empty Maybe will be returned if the ArrayBuffer
+  // is not detachable or detaching failed (likely due to a detach key
+  // mismatch).
+  static v8::Maybe<Store> From(
+    v8::Local<v8::ArrayBufferView> view,
+    v8::Local<v8::Value> detach_key = v8::Local<v8::Value>());
 
   v8::Local<v8::Uint8Array> ToUint8Array(Environment* env) const;
+  inline v8::Local<v8::Uint8Array> ToUint8Array(Realm* realm) const {
+    return ToUint8Array(realm->env());
+  }
 
   operator uv_buf_t() const;
   operator ngtcp2_vec() const;
@@ -59,33 +84,124 @@ class Store final : public MemoryRetainer {
   operator bool() const;
   size_t length() const;
 
+  // Returns the total length of the underlying store, not just the
+  // length of the view. This is useful for memory accounting.
+  size_t total_length() const;
+
   void MemoryInfo(MemoryTracker* tracker) const override;
   SET_MEMORY_INFO_NAME(Store)
   SET_SELF_SIZE(Store)
 
  private:
-  template <typename T, typename t>
+  template <typename T, OneByteType N>
   T convert() const;
   std::shared_ptr<v8::BackingStore> store_;
   size_t length_ = 0;
   size_t offset_ = 0;
+
+  // Because Store holds the v8::BackingStore and not the v8::ArrayBuffer,
+  // etc, the memory held by the Store is not directly, automatically
+  // associated with a v8::Isolate, and is therefore not accounted for
+  // as external memory. It is the responsibility of the owner of the
+  // Store instance to ensure the memory remains accounted for.
 };
 
+// Periodically, these need to be updated to match the latest ngtcp2 defs.
+#define QUIC_TRANSPORT_ERRORS(V)                                               \
+  V(NO_ERROR)                                                                  \
+  V(INTERNAL_ERROR)                                                            \
+  V(CONNECTION_REFUSED)                                                        \
+  V(FLOW_CONTROL_ERROR)                                                        \
+  V(STREAM_LIMIT_ERROR)                                                        \
+  V(STREAM_STATE_ERROR)                                                        \
+  V(FINAL_SIZE_ERROR)                                                          \
+  V(FRAME_ENCODING_ERROR)                                                      \
+  V(TRANSPORT_PARAMETER_ERROR)                                                 \
+  V(CONNECTION_ID_LIMIT_ERROR)                                                 \
+  V(PROTOCOL_VIOLATION)                                                        \
+  V(INVALID_TOKEN)                                                             \
+  V(APPLICATION_ERROR)                                                         \
+  V(CRYPTO_BUFFER_EXCEEDED)                                                    \
+  V(KEY_UPDATE_ERROR)                                                          \
+  V(AEAD_LIMIT_REACHED)                                                        \
+  V(NO_VIABLE_PATH)                                                            \
+  V(CRYPTO_ERROR)                                                              \
+  V(VERSION_NEGOTIATION_ERROR)
+
+// Periodically, these need to be updated to match the latest nghttp3 defs.
+#define HTTP3_APPLICATION_ERRORS(V)                                            \
+  V(H3_NO_ERROR)                                                               \
+  V(H3_GENERAL_PROTOCOL_ERROR)                                                 \
+  V(H3_INTERNAL_ERROR)                                                         \
+  V(H3_STREAM_CREATION_ERROR)                                                  \
+  V(H3_CLOSED_CRITICAL_STREAM)                                                 \
+  V(H3_FRAME_UNEXPECTED)                                                       \
+  V(H3_FRAME_ERROR)                                                            \
+  V(H3_EXCESSIVE_LOAD)                                                         \
+  V(H3_ID_ERROR)                                                               \
+  V(H3_SETTINGS_ERROR)                                                         \
+  V(H3_MISSING_SETTINGS)                                                       \
+  V(H3_REQUEST_REJECTED)                                                       \
+  V(H3_REQUEST_CANCELLED)                                                      \
+  V(H3_REQUEST_INCOMPLETE)                                                     \
+  V(H3_MESSAGE_ERROR)                                                          \
+  V(H3_CONNECT_ERROR)                                                          \
+  V(H3_VERSION_FALLBACK)                                                       \
+  V(QPACK_DECOMPRESSION_FAILED)                                                \
+  V(QPACK_ENCODER_STREAM_ERROR)                                                \
+  V(QPACK_DECODER_STREAM_ERROR)
+
 class QuicError final : public MemoryRetainer {
  public:
-  static constexpr error_code QUIC_NO_ERROR = NGTCP2_NO_ERROR;
+  // The known error codes for the transport namespace.
-  static constexpr error_code QUIC_APP_NO_ERROR = 65280;
+  enum class TransportError : error_code {
+#define V(name) name = NGTCP2_##name,
+    QUIC_TRANSPORT_ERRORS(V)
+#undef V
+  };
 
+  // Every QUIC application defines its own error codes in the application
+  // namespace. These are managed independently of each other and may overlap
+  // with other applications and even the transport namespace. The only way
+  // to correctly interpret an application error code is to know which
+  // application is being used. For convenience, we define constants for the
+  // known HTTP/3 application error codes here.
+  enum class Http3Error : error_code {
+#define V(name) name = NGHTTP3_##name,
+    HTTP3_APPLICATION_ERRORS(V)
+#undef V
+  };
+
+  static constexpr error_code QUIC_NO_ERROR = NGTCP2_NO_ERROR;
+  static constexpr error_code HTTP3_NO_ERROR_CODE = NGHTTP3_H3_NO_ERROR;
+
+  // In QUIC, Errors are represented as namespaced 64-bit error codes.
+  // The error code only has meaning within the context of a specific
+  // namespace. The QuicError::Type enum defines the available namespaces.
+  // There are essentially two namespaces: transport and application, with
+  // a few additional special-cases that are variants of the transport
+  // namespace.
   enum class Type {
     TRANSPORT = NGTCP2_CCERR_TYPE_TRANSPORT,
     APPLICATION = NGTCP2_CCERR_TYPE_APPLICATION,
+
+    // These are special cases of transport errors.
     VERSION_NEGOTIATION = NGTCP2_CCERR_TYPE_VERSION_NEGOTIATION,
     IDLE_CLOSE = NGTCP2_CCERR_TYPE_IDLE_CLOSE,
+    DROP_CONNECTION = NGTCP2_CCERR_TYPE_DROP_CONN,
+    RETRY = NGTCP2_CCERR_TYPE_RETRY,
   };
 
+  // Do not use the constructors directly in regular use. Use the static
+  // factory methods instead. Those will ensure that the underlying
+  // ngtcp2_ccerr is initialized correctly based on the type of error
+  // being created.
   explicit QuicError(const std::string& reason = "");
   explicit QuicError(const ngtcp2_ccerr* ptr);
   explicit QuicError(const ngtcp2_ccerr& error);
+  QuicError(QuicError&& other) noexcept = default;
+  QuicError& operator=(QuicError&& other) noexcept = default;
+  DISALLOW_COPY(QuicError)
 
   Type type() const;
   error_code code() const;
@@ -95,13 +211,16 @@ class QuicError final : public MemoryRetainer {
   operator const ngtcp2_ccerr&() const;
   operator const ngtcp2_ccerr*() const;
 
-  // Returns false if the QuicError uses a no_error code with type
+  // Crypto errors are a subset of transport errors. The error code includes
-  // transport or application.
+  // the TLS alert code embedded within it.
-  operator bool() const;
+  bool is_crypto_error() const;
-
+  std::optional<int> get_crypto_error() const;
-  bool is_crypto() const;
-  std::optional<int> crypto_error() const;
 
+  // Note that since application errors are application-specific and we
+  // don't know which application is being used here, it is possible that
+  // the comparing two different QuicError instances from different applications
+  // will return true even if they are not semantically equivalent. This should
+  // not be a problem in practice.
   bool operator==(const QuicError& other) const;
   bool operator!=(const QuicError& other) const;
 
@@ -110,29 +229,59 @@ class QuicError final : public MemoryRetainer {
   SET_SELF_SIZE(QuicError)
 
   std::string ToString() const;
-  v8::MaybeLocal<v8::Value> ToV8Value(Environment* env) const;
 
+  // Returns an array containing [type, code, reason], where type is a string
+  // representation of the error type, code is a bigint representation of the
+  // error code, and reason is a string representation of the error reason, or
+  // undefined if the reason is the empty string. This is expected to be used
+  // to construct a JS error object from the information in JS.
+  v8::MaybeLocal<v8::Value> ToV8Value(Environment* env) const;
+  inline v8::MaybeLocal<v8::Value> ToV8Value(Realm* realm) const {
+    return ToV8Value(realm->env());
+  }
+
+  // Utility functions for getting the default error reason strings for
+  // internal error codes returned by the underlying ngtcp2/nghttp3 libraries.
   static std::string reason_for_liberr(int liberr);
   static std::string reason_for_h3_liberr(int liberr);
+
+  // Utility functions for checking if the given internal error codes are
+  // considered to be fatal by the underlying ngtcp2/nghttp3 libraries.
   static bool is_fatal_liberror(int liberr);
   static bool is_fatal_h3_liberror(int liberr);
+
+  // Utility functions for converting between ngtcp2/nghttp3 internal error
+  // codes to the corresponding QUIC error codes.
   static error_code liberr_to_code(int liberr);
   static error_code h3_liberr_to_code(int liberr);
 
-  static QuicError ForTransport(error_code code, std::string reason = "");
+  // Utility functions for creating QuicError instances.
-  static QuicError ForApplication(error_code code, std::string reason = "");
+  // The reason is expected to always be UTF-8 encoded.
-  static QuicError ForVersionNegotiation(std::string reason = "");
+  static const QuicError ForTransport(TransportError code,
-  static QuicError ForIdleClose(std::string reason = "");
+                                      std::string reason = "");
-  static QuicError ForNgtcp2Error(int code, std::string reason = "");
+  static const QuicError ForTransport(error_code code, std::string reason = "");
-  static QuicError ForTlsAlert(int code, std::string reason = "");
+  static const QuicError ForApplication(Http3Error code,
+                                        std::string reason = "");
+  static const QuicError ForApplication(error_code code,
+                                        std::string reason = "");
+  static const QuicError ForVersionNegotiation(std::string reason = "");
+  static const QuicError ForIdleClose(std::string reason = "");
+  static const QuicError ForDropConnection(std::string reason = "");
+  static const QuicError ForRetry(std::string reason = "");
+  static const QuicError ForNgtcp2Error(int code, std::string reason = "");
+  static const QuicError ForTlsAlert(int code, std::string reason = "");
 
-  static QuicError FromConnectionClose(ngtcp2_conn* session);
+  static const QuicError FromConnectionClose(ngtcp2_conn* session);
 
-  static QuicError TRANSPORT_NO_ERROR;
+#define V(name) static const QuicError TRANSPORT_##name;
-  static QuicError APPLICATION_NO_ERROR;
+  QUIC_TRANSPORT_ERRORS(V)
-  static QuicError VERSION_NEGOTIATION;
+#undef V
-  static QuicError IDLE_CLOSE;
+  static const QuicError HTTP3_NO_ERROR;
-  static QuicError INTERNAL_ERROR;
+  static const QuicError VERSION_NEGOTIATION;
+  static const QuicError IDLE_CLOSE;
+  static const QuicError DROP_CONNECTION;
+  static const QuicError RETRY;
+  static const QuicError INTERNAL_ERROR;
 
  private:
   const uint8_t* reason_c_str() const;
@@ -142,7 +291,17 @@ class QuicError final : public MemoryRetainer {
   const ngtcp2_ccerr* ptr_ = nullptr;
 };
 
+// Marked maybe_unused because these are used in the tests but not in the
+// production code.
+[[maybe_unused]] static bool operator==(const QuicError::TransportError& lhs,
+                                        error_code rhs) {
+  return static_cast<error_code>(lhs) == rhs;
+}
+[[maybe_unused]] static bool operator==(const QuicError::Http3Error& lhs,
+                                        error_code rhs) {
+  return static_cast<error_code>(lhs) == rhs;
+}
+
 }  // namespace node::quic
 
-#endif  // HAVE_OPENSSL && NODE_OPENSSL_HAS_QUIC
 #endif  // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS

src/quic/defs.h

@@ -221,7 +221,6 @@ enum class DatagramStatus : uint8_t {
 CC_ALGOS(V)
 #undef V
 
-constexpr uint64_t NGTCP2_APP_NOERROR = 65280;
 constexpr size_t kDefaultMaxPacketLength = NGTCP2_MAX_UDP_PAYLOAD_SIZE;
 constexpr size_t kMaxSizeT = std::numeric_limits<size_t>::max();
 constexpr uint64_t kMaxSafeJsInteger = 9007199254740991;
@@ -230,7 +229,7 @@ constexpr size_t kMaxVectorCount = 16;
 
 using error_code = uint64_t;
 
-class DebugIndentScope {
+class DebugIndentScope final {
  public:
   inline DebugIndentScope() { ++indent_; }
   DISALLOW_COPY_AND_MOVE(DebugIndentScope)

src/quic/guard.h

@@ -0,0 +1,13 @@
+#pragma once
+
+#if HAVE_OPENSSL
+#include <openssl/opensslv.h>
+// QUIC is only available in Openssl 3.5.x and later. It was not introduced in
+// Node.js until 3.5.1... prior to that we will not compile any of the QUIC
+// related code.
+#if OPENSSL_VERSION_NUMBER < 0x30500010
+#define OPENSSL_NO_QUIC = 1
+#endif
+#else
+#define OPENSSL_NO_QUIC = 1
+#endif

src/quic/quic.cc

@@ -20,8 +20,6 @@ using v8::Value;
 
 namespace quic {
 
-int DebugIndentScope::indent_ = 0;
-
 void CreatePerIsolateProperties(IsolateData* isolate_data,
                                 Local<ObjectTemplate> target) {
   Endpoint::InitPerIsolate(isolate_data, target);

test/cctest/test_quic_cid.cc

@@ -1,4 +1,6 @@
-#if HAVE_OPENSSL && NODE_OPENSSL_HAS_QUIC
+#if HAVE_OPENSSL
+#include "quic/guard.h"
+#ifndef OPENSSL_NO_QUIC
 #include <env-inl.h>
 #include <gtest/gtest.h>
 #include <ngtcp2/ngtcp2.h>
@@ -12,42 +14,51 @@ using node::quic::CID;
 TEST(CID, Basic) {
   auto& random = CID::Factory::random();
   {
-    auto cid = random.Generate();
+    // Check that the kInvalid CID is indeed invalid.
+    CHECK(!CID::kInvalid);
+  }
+  {
+    const auto cid = random.Generate();
     CHECK_EQ(cid.length(), CID::kMaxLength);
     CHECK(cid);
     CHECK_EQ(cid, cid);
   }
   {
-    auto cid = random.Generate(5);
+    const auto cid = random.Generate(5);
     CHECK_EQ(cid.length(), 5);
     CHECK(cid);
   }
   {
-    auto cid1 = random.Generate();
+    const auto cid1 = random.Generate();
-    auto cid2 = random.Generate();
+    const auto cid2 = random.Generate();
     CHECK_NE(cid1, cid2);
   }
   {
-    auto cid1 = random.Generate(5);
+    const auto cid1 = random.Generate(5);
-    auto cid2 = random.Generate();
+    const auto cid2 = random.Generate();
     CHECK_NE(cid1, cid2);
   }
   {
-    auto cid1 = random.Generate();
+    const auto cid1 = random.Generate();
-    auto cid2 = random.Generate(5);
+    const auto cid2 = random.Generate(5);
     CHECK_NE(cid1, cid2);
   }
   {
-    auto cid = CID::kInvalid;
     // They are copy constructible...
-    auto cid2 = cid;
+    CID cid = CID::kInvalid;
     CHECK(!cid);
     CHECK_EQ(cid.length(), 0);
-    CHECK_EQ(cid, cid2);
+    CHECK_EQ(CID::kInvalid, cid);
+
+    const auto cid2 = random.Generate();
+    const auto cid3 = cid2;
+    CHECK_EQ(cid2, cid3);
   }
   {
-    auto cid1 = random.Generate();
+    const auto cid1 = random.Generate();
-    auto cid2 = random.Generate();
+    const auto cid2 = random.Generate();
+    const auto cid3 = random.Generate();
+
     CID::Map<std::string> map;
     map[cid1] = "hello";
     map[cid2] = "there";
@@ -55,12 +66,23 @@ TEST(CID, Basic) {
     CHECK_EQ(map[cid2], "there");
     CHECK_NE(map[cid2], "hello");
     CHECK_NE(map[cid1], "there");
+
+    // Remove the two CIDs.
+    CHECK_EQ(map.erase(cid1), 1);
+    CHECK_EQ(map.erase(cid2), 1);
+
+    // Make sure they were removed.
+    CHECK_EQ(map.find(cid1), map.end());
+    CHECK_EQ(map.find(cid2), map.end());
+
+    // The cid3 is not in the map, so it should not be found.
+    CHECK_EQ(map.find(cid3), map.end());
   }
   {
     ngtcp2_cid cid_;
     uint8_t data[] = {1, 2, 3, 4, 5};
     ngtcp2_cid_init(&cid_, data, 5);
-    auto cid = CID(cid_);
+    const CID cid(cid_);
     // This variation of the constructor copies the cid_, so if we
     // modify the original data it doesn't change in the CID.
     cid_.data[0] = 9;
@@ -71,26 +93,28 @@ TEST(CID, Basic) {
     ngtcp2_cid cid_;
     uint8_t data[] = {1, 2, 3, 4, 5};
     ngtcp2_cid_init(&cid_, data, 5);
-    auto cid = CID(&cid_);
+    const CID cid(&cid_);
     // This variation of the constructor wraps the cid_, so if we
-    // modify the original data it does change in the CID.
+    // modify the original data it does change in the CID... not
+    // that this is something we would normally do.
     cid_.data[0] = 9;
     CHECK_EQ(cid.length(), 5);
     CHECK_EQ(cid.ToString(), "0902030405");
   }
   {
     // Generate a bunch to ensure that the pool is regenerated.
-    for (int n = 0; n < 1000; n++) {
+    for (int n = 0; n < 5000; n++) {
-      random.Generate();
+      CHECK(random.Generate());
     }
   }
   {
     ngtcp2_cid cid_;
     // Generate a bunch to ensure that the pool is regenerated.
-    for (int n = 0; n < 1000; n++) {
+    for (int n = 0; n < 5000; n++) {
-      random.GenerateInto(&cid_, 10);
+      CHECK(random.GenerateInto(&cid_, 10));
       CHECK_EQ(cid_.datalen, 10);
     }
   }
 }
-#endif  // HAVE_OPENSSL && NODE_OPENSSL_HAS_QUIC
+#endif  // OPENSSL_NO_QUIC
+#endif  // HAVE_OPENSSL

test/cctest/test_quic_error.cc

@@ -1,4 +1,6 @@
-#if HAVE_OPENSSL && NODE_OPENSSL_HAS_QUIC
+#if HAVE_OPENSSL
+#include "quic/guard.h"
+#ifndef OPENSSL_NO_QUIC
 #include <env-inl.h>
 #include <gtest/gtest.h>
 #include <quic/data.h>
@@ -13,7 +15,9 @@ TEST(QuicError, NoError) {
   CHECK_EQ(err.type(), QuicError::Type::TRANSPORT);
   CHECK_EQ(err.reason(), "");
   CHECK_EQ(err, QuicError::TRANSPORT_NO_ERROR);
-  CHECK(!err);
+
+  CHECK_EQ(QuicError::TransportError::NO_ERROR, QuicError::QUIC_NO_ERROR);
+  CHECK_EQ(QuicError::Http3Error::H3_NO_ERROR, QuicError::HTTP3_NO_ERROR_CODE);
 
   QuicError err2("a reason");
   CHECK_EQ(err2.code(), QuicError::QUIC_NO_ERROR);
@@ -29,17 +33,13 @@ TEST(QuicError, NoError) {
   CHECK_EQ(err3.reason(), "");
   CHECK_EQ(err3, QuicError::TRANSPORT_NO_ERROR);
 
-  // QuicError's are copy assignable
-  auto err4 = err3;
-  CHECK_EQ(err4, err3);
-
   // QuicError's are movable
-  auto err5 = std::move(err4);
+  auto err5 = std::move(err3);
   CHECK_EQ(err5, err3);
 
   // Equality check ignores the reason
   CHECK(err5 == err2);
-  CHECK(err5 != QuicError::APPLICATION_NO_ERROR);
+  CHECK(err5 != QuicError::HTTP3_NO_ERROR);
 
   const ngtcp2_ccerr& ccerr = err5;
   CHECK_EQ(ccerr.error_code, NGTCP2_NO_ERROR);
@@ -57,8 +57,8 @@ TEST(QuicError, NoError) {
   QuicError err7(ccerr2);
   CHECK_EQ(err6, err7);
 
-  CHECK_EQ(err.ToString(), "QuicError(TRANSPORT) 0");
+  CHECK_EQ(err.ToString(), "QuicError(transport) 0");
-  CHECK_EQ(err2.ToString(), "QuicError(TRANSPORT) 0: a reason");
+  CHECK_EQ(err2.ToString(), "QuicError(transport) 0: a reason");
 
   ngtcp2_ccerr ccerr3;
   ngtcp2_ccerr_default(&ccerr3);
@@ -68,18 +68,16 @@ TEST(QuicError, NoError) {
 }
 
 TEST(QuicError, ApplicationNoError) {
-  CHECK_EQ(QuicError::APPLICATION_NO_ERROR.code(),
+  CHECK_EQ(QuicError::HTTP3_NO_ERROR.code(), QuicError::HTTP3_NO_ERROR_CODE);
-           QuicError::QUIC_APP_NO_ERROR);
+  CHECK_EQ(QuicError::HTTP3_NO_ERROR.type(), QuicError::Type::APPLICATION);
-  CHECK_EQ(QuicError::APPLICATION_NO_ERROR.type(),
+  CHECK_EQ(QuicError::HTTP3_NO_ERROR.reason(), "");
-           QuicError::Type::APPLICATION);
-  CHECK_EQ(QuicError::APPLICATION_NO_ERROR.reason(), "");
 
   auto err =
-      QuicError::ForApplication(QuicError::QUIC_APP_NO_ERROR, "a reason");
+      QuicError::ForApplication(QuicError::HTTP3_NO_ERROR_CODE, "a reason");
-  CHECK_EQ(err.code(), QuicError::QUIC_APP_NO_ERROR);
+  CHECK_EQ(err.code(), QuicError::HTTP3_NO_ERROR_CODE);
   CHECK_EQ(err.type(), QuicError::Type::APPLICATION);
   CHECK_EQ(err.reason(), "a reason");
-  CHECK_EQ(err.ToString(), "QuicError(APPLICATION) 65280: a reason");
+  CHECK_EQ(err.ToString(), "QuicError(application) 256: a reason");
 }
 
 TEST(QuicError, VersionNegotiation) {
@@ -92,7 +90,7 @@ TEST(QuicError, VersionNegotiation) {
   CHECK_EQ(err.code(), 0);
   CHECK_EQ(err.type(), QuicError::Type::VERSION_NEGOTIATION);
   CHECK_EQ(err.reason(), "a reason");
-  CHECK_EQ(err.ToString(), "QuicError(VERSION_NEGOTIATION) 0: a reason");
+  CHECK_EQ(err.ToString(), "QuicError(version_negotiation) 0: a reason");
 }
 
 TEST(QuicError, IdleClose) {
@@ -104,7 +102,7 @@ TEST(QuicError, IdleClose) {
   CHECK_EQ(err.code(), 0);
   CHECK_EQ(err.type(), QuicError::Type::IDLE_CLOSE);
   CHECK_EQ(err.reason(), "a reason");
-  CHECK_EQ(err.ToString(), "QuicError(IDLE_CLOSE) 0: a reason");
+  CHECK_EQ(err.ToString(), "QuicError(idle_close) 0: a reason");
 
   CHECK_EQ(QuicError::IDLE_CLOSE, err);
 }
@@ -114,7 +112,7 @@ TEST(QuicError, InternalError) {
   CHECK_EQ(err.code(), NGTCP2_INTERNAL_ERROR);
   CHECK_EQ(err.type(), QuicError::Type::TRANSPORT);
   CHECK_EQ(err.reason(), "a reason");
-  CHECK_EQ(err.ToString(), "QuicError(TRANSPORT) 1: a reason");
+  CHECK_EQ(err.ToString(), "QuicError(transport) 1: a reason");
 
   printf("%s\n", QuicError::INTERNAL_ERROR.ToString().c_str());
   CHECK_EQ(err, QuicError::INTERNAL_ERROR);
@@ -125,8 +123,9 @@ TEST(QuicError, TlsAlert) {
   CHECK_EQ(err.code(), 257);
   CHECK_EQ(err.type(), QuicError::Type::TRANSPORT);
   CHECK_EQ(err.reason(), "a reason");
-  CHECK(err.is_crypto());
+  CHECK(err.is_crypto_error());
-  CHECK_EQ(err.crypto_error(), 1);
+  CHECK_EQ(err.get_crypto_error(), 1);
 }
 
-#endif  // HAVE_OPENSSL && NODE_OPENSSL_HAS_QUIC
+#endif  // OPENSSL_NO_QUIC
+#endif  // HAVE_OPENSSL