lib: reduce URL invocations on http2 origins

PR-URL: #48338 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Paolo Insogna <paolo@cowtech.it> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
nodejs · Jul 3, 2023 · 159ab66 · 159ab66
1 parent 4ec2d92
commit 159ab66
Show file tree

Hide file tree

Showing 5 changed files with 43 additions and 4 deletions.
diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js
@@ -45,7 +45,7 @@ const EventEmitter = require('events');
 const fs = require('fs');
 const http = require('http');
 const { readUInt16BE, readUInt32BE } = require('internal/buffer');
-const { URL } = require('internal/url');
+const { URL, getURLOrigin } = require('internal/url');
 const net = require('net');
 const { Duplex } = require('stream');
 const tls = require('tls');
@@ -636,7 +636,7 @@ function initOriginSet(session) {
       // We have to ensure that it is a properly serialized
       // ASCII origin string. The socket.servername might not
       // be properly ASCII encoded.
-      originSet.add((new URL(originString)).origin);
+      originSet.add(getURLOrigin(originString));
     }
   }
   return originSet;
@@ -1641,7 +1641,7 @@ class ServerHttp2Session extends Http2Session {
     let origin;
 
     if (typeof originOrStream === 'string') {
-      origin = (new URL(originOrStream)).origin;
+      origin = getURLOrigin(originOrStream);
       if (origin === 'null')
         throw new ERR_HTTP2_ALTSVC_INVALID_ORIGIN();
     } else if (typeof originOrStream === 'number') {
@@ -1693,7 +1693,7 @@ class ServerHttp2Session extends Http2Session {
     for (let i = 0; i < count; i++) {
       let origin = origins[i];
       if (typeof origin === 'string') {
-        origin = (new URL(origin)).origin;
+        origin = getURLOrigin(origin);
       } else if (origin != null && typeof origin === 'object') {
         origin = origin.origin;
       }

diff --git a/lib/internal/url.js b/lib/internal/url.js
@@ -1436,6 +1436,16 @@ function toPathIfFileURL(fileURLOrPath) {
   return fileURLToPath(fileURLOrPath);
 }
 
+/**
+ * This util takes a string containing a URL and return the URL origin,
+ * its meant to avoid calls to `new URL` constructor.
+ * @param {string} url
+ * @returns {URL['origin']}
+ */
+function getURLOrigin(url) {
+  return bindingUrl.getOrigin(url);
+}
+
 module.exports = {
   toUSVString,
   fileURLToPath,
@@ -1451,4 +1461,5 @@ module.exports = {
   isURL,
 
   urlUpdateActions: updateActions,
+  getURLOrigin,
 };
diff --git a/src/node_errors.h b/src/node_errors.h
@@ -74,6 +74,7 @@ void AppendExceptionLine(Environment* env,
   V(ERR_INVALID_STATE, Error)                                                  \
   V(ERR_INVALID_THIS, TypeError)                                               \
   V(ERR_INVALID_TRANSFER_OBJECT, TypeError)                                    \
+  V(ERR_INVALID_URL, TypeError)                                                \
   V(ERR_MEMORY_ALLOCATION_FAILED, Error)                                       \
   V(ERR_MESSAGE_TARGET_CONTEXT_UNAVAILABLE, Error)                             \
   V(ERR_MISSING_ARGS, TypeError)                                               \

diff --git a/src/node_url.cc b/src/node_url.cc
@@ -117,6 +117,30 @@ void BindingData::DomainToUnicode(const FunctionCallbackInfo<Value>& args) {
                                 .ToLocalChecked());
 }
 
+void BindingData::GetOrigin(const v8::FunctionCallbackInfo<Value>& args) {
+  CHECK_GE(args.Length(), 1);
+  CHECK(args[0]->IsString());  // input
+
+  Environment* env = Environment::GetCurrent(args);
+  HandleScope handle_scope(env->isolate());
+
+  Utf8Value input(env->isolate(), args[0]);
+  std::string_view input_view = input.ToStringView();
+  auto out = ada::parse<ada::url_aggregator>(input_view);
+
+  if (!out) {
+    THROW_ERR_INVALID_URL(env, "Invalid URL");
+    return;
+  }
+
+  std::string origin = out->get_origin();
+  args.GetReturnValue().Set(String::NewFromUtf8(env->isolate(),
+                                                origin.data(),
+                                                NewStringType::kNormal,
+                                                origin.length())
+                                .ToLocalChecked());
+}
+
 void BindingData::CanParse(const FunctionCallbackInfo<Value>& args) {
   CHECK_GE(args.Length(), 1);
   CHECK(args[0]->IsString());  // input
@@ -322,6 +346,7 @@ void BindingData::CreatePerIsolateProperties(IsolateData* isolate_data,
   SetMethodNoSideEffect(isolate, target, "domainToASCII", DomainToASCII);
   SetMethodNoSideEffect(isolate, target, "domainToUnicode", DomainToUnicode);
   SetMethodNoSideEffect(isolate, target, "format", Format);
+  SetMethodNoSideEffect(isolate, target, "getOrigin", GetOrigin);
   SetMethod(isolate, target, "parse", Parse);
   SetMethod(isolate, target, "update", Update);
   SetFastMethodNoSideEffect(
@@ -341,6 +366,7 @@ void BindingData::RegisterExternalReferences(
   registry->Register(DomainToASCII);
   registry->Register(DomainToUnicode);
   registry->Register(Format);
+  registry->Register(GetOrigin);
   registry->Register(Parse);
   registry->Register(Update);
   registry->Register(CanParse);

diff --git a/src/node_url.h b/src/node_url.h
@@ -53,6 +53,7 @@ class BindingData : public SnapshotableObject {
                            const v8::FastOneByteString& input);
 
   static void Format(const v8::FunctionCallbackInfo<v8::Value>& args);
+  static void GetOrigin(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void Parse(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void Update(const v8::FunctionCallbackInfo<v8::Value>& args);