[pull] main from nodejs:main

doc/api/tls.md

@@ -741,12 +741,9 @@ server. If `tlsSocket.authorized` is `false`, then `socket.authorizationError`
 is set to describe how authorization failed. Depending on the settings
 of the TLS server, unauthorized connections may still be accepted.
 
-The `tlsSocket.alpnProtocol` property is a string that contains the selected
-ALPN protocol. When ALPN has no selected protocol because the client or the
-server did not send an ALPN extension, `tlsSocket.alpnProtocol` equals `false`.
-
-The `tlsSocket.servername` property is a string containing the server name
-requested via SNI.
+The [`tls.TLSSocket.servername`][] and [`tls.TLSSocket.alpnProtocol`][]
+properties can be used to check which server name was requested, and which
+protocol was negotiated.
 
 ### Event: `'tlsClientError'`
 
@@ -1043,6 +1040,18 @@ Returns the bound `address`, the address `family` name, and `port` of the
 underlying socket as reported by the operating system:
 `{ port: 12346, family: 'IPv4', address: '127.0.0.1' }`.
 
+### `tlsSocket.alpnProtocol`
+
+<!-- YAML
+added: v6.0.0
+-->
+
+* Type: {string|boolean|null}
+
+The negotiated ALPN protocol. This is `null` before the handshake completes.
+Once the handshake completes, it settles as either the negotiated protocol
+name, or `false` if the peers did not negotiate an ALPN protocol.
+
 ### `tlsSocket.authorizationError`
 
 <!-- YAML
@@ -1570,6 +1579,18 @@ When running as the server, the socket will be destroyed with an error after
 For TLSv1.3, renegotiation cannot be initiated, it is not supported by the
 protocol.
 
+### `tlsSocket.servername`
+
+<!-- YAML
+added: v0.11.3
+-->
+
+* Type: {string|boolean|null}
+
+The SNI (Server Name Indication) host name associated with the socket. This is
+`null` before the handshake completes. Once the handshake completes it settles
+as either the host name string, or `false` if SNI was not used.
+
 ### `tlsSocket.setKeyCert(context)`
 
 <!-- YAML
@@ -2553,11 +2574,13 @@ added: v0.11.3
 [`tls.DEFAULT_MAX_VERSION`]: #tlsdefault_max_version
 [`tls.DEFAULT_MIN_VERSION`]: #tlsdefault_min_version
 [`tls.Server`]: #class-tlsserver
+[`tls.TLSSocket.alpnProtocol`]: #tlssocketalpnprotocol
 [`tls.TLSSocket.enableTrace()`]: #tlssocketenabletrace
 [`tls.TLSSocket.getPeerCertificate()`]: #tlssocketgetpeercertificatedetailed
 [`tls.TLSSocket.getProtocol()`]: #tlssocketgetprotocol
 [`tls.TLSSocket.getSession()`]: #tlssocketgetsession
 [`tls.TLSSocket.getTLSTicket()`]: #tlssocketgettlsticket
+[`tls.TLSSocket.servername`]: #tlssocketservername
 [`tls.TLSSocket`]: #class-tlstlssocket
 [`tls.connect()`]: #tlsconnectoptions-callback
 [`tls.createSecureContext()`]: #tlscreatesecurecontextoptions

doc/contributing/first-contributions.md

@@ -21,7 +21,7 @@ If your first contribution is motivated by an issue you already encounter, but y
 is complex, introduces a new feature, breaks backward compatibility, or needs a significant refactor,
 it can be useful to open an issue to discuss whether the change or its design is acceptable before
 working out the implementation details. The issue can be opened before you send the pull request,
-or along side your pull request (in which case, it's better to keep the pull request in draft mode).
+or alongside your pull request (in which case, it's better to keep the pull request in draft mode).
 
 Straightforward or non-controversial changes, such as an obvious bug fix or a small refactor, don't
 usually benefit from discussions in a separate issue, and can be sent directly as a pull request.

doc/contributing/pull-requests.md

@@ -459,7 +459,7 @@ commit.
 
 If a pull request has been inactive for more than six months, add the `stalled` label
 to it. That will trigger an automation that adds a comment explaining the pull request
-may be close for inactivity, giving a heads-up to the contributor before actually
+may be closed for inactivity, giving a heads-up to the contributor before actually
 closing it if it remains inactive.
 
 ### Approving a change

doc/contributing/writing-tests.md

@@ -345,7 +345,7 @@ when troubleshooting tests that timeout in CI. With no log statements, we have
 no idea where the test got hung up.
 
 There have been cases where tests fail without `console.log()`, and then pass
-when its added, so be cautious about its use, particularly in tests of the I/O
+when it's added, so be cautious about its use, particularly in tests of the I/O
 and streaming APIs.
 
 Excessive use of console output is discouraged as it can overwhelm the display,

lib/internal/dns/callback_resolver.js

@@ -3,7 +3,6 @@
 const {
   ArrayPrototypeMap,
   FunctionPrototypeCall,
-  ObjectDefineProperty,
   Symbol,
 } = primordials;
 
@@ -33,6 +32,7 @@ const {
   startPerf,
   stopPerf,
 } = require('internal/perf/observe');
+const { assignFunctionName } = require('internal/util');
 
 const kPerfHooksDnsLookupResolveContext = Symbol('kPerfHooksDnsLookupResolveContext');
 
@@ -82,8 +82,7 @@ function resolver(bindingName) {
     }
     return req;
   }
-  ObjectDefineProperty(query, 'name', { __proto__: null, value: bindingName });
-  return query;
+  return assignFunctionName(bindingName, query);
 }
 
 // This is the callback-based resolver. There is another similar

lib/internal/quic/stats.js

@@ -870,7 +870,7 @@ class QuicSessionStats {
   [kFinishClose]() {
     const view = TypedArrayPrototypeSubarray(this.#handle,
                                              this.#offset,
-                                             this.#offset + IDX_STATS_STREAM_COUNT);
+                                             this.#offset + IDX_STATS_SESSION_COUNT);
     this.#handle = new BigUint64Array(view);
     this.#offset = 0;
     this.#disconnected = true;

lib/internal/util/inspector.js

@@ -4,7 +4,6 @@ const {
   ArrayPrototypePushApply,
   ArrayPrototypeSome,
   FunctionPrototypeBind,
-  ObjectDefineProperty,
   ObjectKeys,
   ObjectPrototypeHasOwnProperty,
   RegExpPrototypeExec,
@@ -13,6 +12,7 @@ const {
 
 const { validatePort } = require('internal/validators');
 const permission = require('internal/process/permission');
+const { assignFunctionName } = require('internal/util');
 
 const kMinPort = 1024;
 const kMaxPort = 65535;
@@ -88,16 +88,12 @@ function wrapConsole(consoleFromNode) {
     // then wrap these two methods into one. Native wrapper will preserve
     // the original stack.
     if (ObjectPrototypeHasOwnProperty(consoleFromNode, key)) {
-      consoleFromNode[key] = FunctionPrototypeBind(
+      consoleFromNode[key] = assignFunctionName(key, FunctionPrototypeBind(
         consoleCall,
         consoleFromNode,
         consoleFromVM[key],
         consoleFromNode[key],
-      );
-      ObjectDefineProperty(consoleFromNode[key], 'name', {
-        __proto__: null,
-        value: key,
-      });
+      ));
     } else {
       // Add additional console APIs from the inspector
       consoleFromNode[key] = consoleFromVM[key];

src/node_builtins.cc

@@ -562,7 +562,7 @@ bool BuiltinLoader::CompileAllBuiltinsAndCopyCodeCache(
       std::unordered_set(eager_builtins.begin(), eager_builtins.end());
 
   for (const auto& id : ids) {
-    // Eagerly compile primordials/boostrap/main scripts during code cache
+    // Eagerly compile primordials/bootstrap/main scripts during code cache
     // generation.
     if (id.starts_with(primordial_prefix) || id.starts_with(bootstrap_prefix) ||
         id.starts_with(main_prefix)) {

src/node_builtins.h

@@ -207,7 +207,7 @@ class NODE_EXTERN_PRIVATE BuiltinLoader {
 
   const UnionBytes config_;
 
-  // If any bulitins should be eagerly compiled i.e. with inner functions
+  // If any builtins should be eagerly compiled i.e. with inner functions
   // compiled too, either use should_eager_compile_ to compile all builtins
   // eagerly, or use to_eager_compile_ to compile specific builtins eagerly.
   // Currently we set should_eager_compile_ to true when compiling primordials,

src/node_http2.cc

@@ -815,6 +815,21 @@ void Http2Session::Close(uint32_t code, bool socket_closed) {
     return;
   set_closing();
 
+  // Do not flush GOAWAY from inside nghttp2_session_mem_recv() callbacks.
+  // ConsumeHTTP2Data() finishes the close once mem_recv returns.
+  if (is_receiving()) {
+    set_close_pending();
+    pending_close_code_ = code;
+    pending_close_socket_closed_ = socket_closed;
+    return;
+  }
+
+  FinishClose(code, socket_closed);
+}
+
+void Http2Session::FinishClose(uint32_t code, bool socket_closed) {
+  CHECK(is_closing());
+
   // Stop reading on the i/o stream
   if (stream_ != nullptr) {
     set_reading_stopped();
@@ -864,6 +879,12 @@ void Http2Session::Close(uint32_t code, bool socket_closed) {
   EmitStatistics();
 }
 
+void Http2Session::MaybeFinishPendingClose() {
+  if (!is_close_pending() || is_destroyed()) return;
+  set_close_pending(false);
+  FinishClose(pending_close_code_, pending_close_socket_closed_);
+}
+
 // Locates an existing known stream by ID. nghttp2 has a similar method
 // but this is faster and does not fail if the stream is not found.
 BaseObjectPtr<Http2Stream> Http2Session::FindStream(int32_t id) {
@@ -958,11 +979,13 @@ void Http2Session::ConsumeHTTP2Data() {
         nghttp2_session_want_read(session_.get()));
   set_receive_paused(false);
   custom_recv_error_code_ = nullptr;
+  set_receiving();
   ssize_t ret =
     nghttp2_session_mem_recv(session_.get(),
                              reinterpret_cast<uint8_t*>(stream_buf_.base) +
                                  stream_buf_offset_,
                              read_len);
+  set_receiving(false);
   CHECK_NE(ret, NGHTTP2_ERR_NOMEM);
   CHECK_IMPLIES(custom_recv_error_code_ != nullptr, ret < 0);
 
@@ -976,6 +999,10 @@ void Http2Session::ConsumeHTTP2Data() {
     // Even if all bytes were received, a paused stream may delay the
     // nghttp2_on_frame_recv_callback which may have an END_STREAM flag.
     stream_buf_offset_ += ret;
+    // Still complete a Close() deferred during mem_recv; do not fall through
+    // to SendPendingData() here (paused receives historically skip that flush
+    // because a write may already be in progress).
+    MaybeFinishPendingClose();
     goto done;
   }
 
@@ -986,12 +1013,23 @@ void Http2Session::ConsumeHTTP2Data() {
   stream_buf_allocation_.reset();
   stream_buf_ = uv_buf_init(nullptr, 0);
 
+  // Finish a Close() deferred during mem_recv before flushing, so GOAWAY is
+  // not written after pending RST_STREAM frames.
+  MaybeFinishPendingClose();
+
+done:
+  // Finish a Close() deferred above before flushing, so GOAWAY is not written
+  // after pending RST_STREAM frames.
+  if (is_close_pending() && !is_destroyed()) {
+    set_close_pending(false);
+    FinishClose(pending_close_code_, pending_close_socket_closed_);
+  }
+
   // Send any data that was queued up while processing the received data.
   if (ret >= 0 && !is_destroyed()) {
     SendPendingData();
   }
 
-done:
   if (ret < 0) [[unlikely]] {
     Isolate* isolate = env()->isolate();
     Debug(this,
@@ -1418,6 +1456,9 @@ int Http2Session::OnDataChunkReceived(nghttp2_session* handle,
     len -= avail;
     stream->EmitRead(avail, buf);
 
+    // JS may have destroyed the stream from inside onread; stop delivering.
+    if (stream->is_destroyed()) break;
+
     // If the stream owner (e.g. the JS Http2Stream) wants more data, just
     // tell nghttp2 that all data has been consumed. Otherwise, defer until
     // more data is being requested.
@@ -1975,6 +2016,12 @@ uint8_t Http2Session::SendPendingData() {
   // SendPendingData should not be called recursively.
   if (is_sending())
     return 1;
+
+  // Do not call `nghttp2_session_mem_send()` while nghttp2 is processing
+  // incoming data. Sending may close the stream and free nghttp2 state
+  // that is still in use by `nghttp2_session_mem_recv()`.
+  if (is_receiving()) return 1;
+
   // This is cleared by ClearOutgoing().
   set_sending();
 
@@ -2387,10 +2434,48 @@ void Http2Stream::Destroy() {
   // Do nothing if this stream instance is already destroyed
   if (is_destroyed())
     return;
-  if (session_->has_pending_rststream(id_))
-    FlushRstStream();
+
+  // Session may already be gone if destroy was deferred across a session
+  // teardown.
+  if (!session_) {
+    set_destroyed();
+    Detach();
+    return;
+  }
+
+  // Mark destroyed immediately so OnDataChunkReceived stops EmitRead into an
+  // already-destroyed JS stream (which would treat the byte count as errno).
   set_destroyed();
 
+  // While mem_recv is active, do not FlushRstStream or RemoveStream yet:
+  // - FlushRstStream would close the nghttp2 stream before queued response
+  //   DATA can be mem_send'd after receive returns.
+  // - RemoveStream would make OnSendData/Provider::OnRead fail to FindStream.
+  // Pending RSTs stay in pending_rst_streams_ and are flushed from
+  // ClearOutgoing after the post-receive SendPendingData.
+  if (session_->is_receiving()) {
+    BaseObjectPtr<Http2Stream> strong_ref{this};
+    env()->SetImmediate(
+        [this, strong_ref](Environment*) { CompleteDestroyCleanup(); });
+    return;
+  }
+
+  if (session_->has_pending_rststream(id_)) FlushRstStream();
+
+  CompleteDestroyCleanup();
+}
+
+void Http2Stream::CompleteDestroyCleanup() {
+  if (!session_) {
+    Detach();
+    return;
+  }
+
+  // Destroy() always set_destroyed() before scheduling or calling this.
+  CHECK(is_destroyed());
+
+  if (session_->has_pending_rststream(id_)) FlushRstStream();
+
   Debug(this, "destroying stream");
 
   // Wait until the start of the next loop to delete because there
@@ -2427,7 +2512,6 @@ void Http2Stream::Destroy() {
   EmitStatistics();
 }
 
-
 // Initiates a response on the Http2Stream using data provided via the
 // StreamBase Streams API.
 int Http2Stream::SubmitResponse(const Http2Headers& headers, int options) {
@@ -2559,6 +2643,18 @@ void Http2Stream::SubmitRstStream(const uint32_t code) {
     return code == NGHTTP2_CANCEL;
   };
 
+  // Do not call `nghttp2_session_mem_send()` while nghttp2 is processing
+  // incoming data. Sending may close the stream and free nghttp2 state
+  // that is still in use by `nghttp2_session_mem_recv()`.
+  if (session_->is_receiving() && available_outbound_length_ == 0) {
+    if (is_stream_cancel(code)) {
+      session_->AddPendingRstStream(id_);
+      return;
+    }
+    FlushRstStream();
+    return;
+  }
+
   // If RST_STREAM frame is received with error code NGHTTP2_CANCEL,
   // add it to the pending list and don't force purge the data. It is
   // to avoids the double free error due to unwanted behavior of nghttp2.
@@ -2586,8 +2682,8 @@ void Http2Stream::SubmitRstStream(const uint32_t code) {
 }
 
 void Http2Stream::FlushRstStream() {
-  if (is_destroyed())
-    return;
+  if (!session_) return;
+  session_->RemovePendingRstStream(id_);
   Http2Scope h2scope(this);
   CHECK_EQ(nghttp2_submit_rst_stream(
       session_->session(),