url: fix canParse false value when v8 optimizes

PR-URL: #48817 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Matthew Aitken <maitken033380023@gmail.com> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
nodejs · Jul 18, 2023 · 07bfcc4 · 07bfcc4
1 parent 227e6bd
commit 07bfcc4
Show file tree

Hide file tree

Showing 6 changed files with 39 additions and 4 deletions.
diff --git a/lib/internal/url.js b/lib/internal/url.js
@@ -1046,10 +1046,10 @@ class URL {
     url = `${url}`;
 
     if (base !== undefined) {
-      base = `${base}`;
+      return bindingUrl.canParseWithBase(url, `${base}`);
     }
 
-    return bindingUrl.canParse(url, base);
+    return bindingUrl.canParse(url);
   }
 }
 

diff --git a/src/node_external_reference.h b/src/node_external_reference.h
@@ -19,8 +19,12 @@ using CFunctionCallbackWithInt64 = void (*)(v8::Local<v8::Object> receiver,
                                             int64_t);
 using CFunctionCallbackWithBool = void (*)(v8::Local<v8::Object> receiver,
                                            bool);
-using CFunctionCallbackWithStrings =
+using CFunctionCallbackWithString =
     bool (*)(v8::Local<v8::Value>, const v8::FastOneByteString& input);
+using CFunctionCallbackWithStrings =
+    bool (*)(v8::Local<v8::Value>,
+             const v8::FastOneByteString& input,
+             const v8::FastOneByteString& base);
 using CFunctionWithUint32 = uint32_t (*)(v8::Local<v8::Value>,
                                          const uint32_t input);
 
@@ -36,6 +40,7 @@ class ExternalReferenceRegistry {
   V(CFunctionCallbackReturnDouble)                                             \
   V(CFunctionCallbackWithInt64)                                                \
   V(CFunctionCallbackWithBool)                                                 \
+  V(CFunctionCallbackWithString)                                               \
   V(CFunctionCallbackWithStrings)                                              \
   V(CFunctionWithUint32)                                                       \
   V(const v8::CFunctionInfo*)                                                  \

diff --git a/src/node_url.cc b/src/node_url.cc
@@ -172,6 +172,16 @@ bool BindingData::FastCanParse(Local<Value> receiver,
 
 CFunction BindingData::fast_can_parse_(CFunction::Make(FastCanParse));
 
+bool BindingData::FastCanParseWithBase(Local<Value> receiver,
+                                       const FastOneByteString& input,
+                                       const FastOneByteString& base) {
+  auto base_view = std::string_view(base.data, base.length);
+  return ada::can_parse(std::string_view(input.data, input.length), &base_view);
+}
+
+CFunction BindingData::fast_can_parse_with_base_(
+    CFunction::Make(FastCanParseWithBase));
+
 void BindingData::Format(const FunctionCallbackInfo<Value>& args) {
   CHECK_GT(args.Length(), 4);
   CHECK(args[0]->IsString());  // url href
@@ -352,6 +362,11 @@ void BindingData::CreatePerIsolateProperties(IsolateData* isolate_data,
   SetMethod(isolate, target, "update", Update);
   SetFastMethodNoSideEffect(
       isolate, target, "canParse", CanParse, &fast_can_parse_);
+  SetFastMethodNoSideEffect(isolate,
+                            target,
+                            "canParseWithBase",
+                            CanParse,
+                            &fast_can_parse_with_base_);
 }
 
 void BindingData::CreatePerContextProperties(Local<Object> target,
@@ -373,6 +388,8 @@ void BindingData::RegisterExternalReferences(
   registry->Register(CanParse);
   registry->Register(FastCanParse);
   registry->Register(fast_can_parse_.GetTypeInfo());
+  registry->Register(FastCanParseWithBase);
+  registry->Register(fast_can_parse_with_base_.GetTypeInfo());
 }
 
 std::string FromFilePath(const std::string_view file_path) {

diff --git a/src/node_url.h b/src/node_url.h
@@ -51,6 +51,9 @@ class BindingData : public SnapshotableObject {
   static void CanParse(const v8::FunctionCallbackInfo<v8::Value>& args);
   static bool FastCanParse(v8::Local<v8::Value> receiver,
                            const v8::FastOneByteString& input);
+  static bool FastCanParseWithBase(v8::Local<v8::Value> receiver,
+                                   const v8::FastOneByteString& input,
+                                   const v8::FastOneByteString& base);
 
   static void Format(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void GetOrigin(const v8::FunctionCallbackInfo<v8::Value>& args);
@@ -73,6 +76,7 @@ class BindingData : public SnapshotableObject {
                         const ada::scheme::type type);
 
   static v8::CFunction fast_can_parse_;
+  static v8::CFunction fast_can_parse_with_base_;
 };
 
 std::string FromFilePath(const std::string_view file_path);

diff --git a/test/parallel/test-whatwg-url-canparse.js b/test/parallel/test-whatwg-url-canparse.js
@@ -19,3 +19,11 @@ const { canParse } = internalBinding('url');
 // It should not throw when called without a base string
 assert.strictEqual(URL.canParse('https://example.org'), true);
 assert.strictEqual(canParse('https://example.org'), true);
+
+// This for-loop is used to test V8 Fast API optimizations
+for (let i = 0; i < 100000; i++) {
+  // This example is used because only parsing the first parameter
+  // results in an invalid URL. They have to be used together to
+  // produce truthy value.
+  assert.strictEqual(URL.canParse('/', 'http://n'), true);
+}
diff --git a/typings/internalBinding/url.d.ts b/typings/internalBinding/url.d.ts
@@ -5,7 +5,8 @@ declare function InternalBinding(binding: 'url'): {
 
   domainToASCII(input: string): string;
   domainToUnicode(input: string): string;
-  canParse(input: string, base?: string): boolean;
+  canParse(input: string): boolean;
+  canParseWithBase(input: string, base: string): boolean;
   format(input: string, fragment?: boolean, unicode?: boolean, search?: boolean, auth?: boolean): string;
   parse(input: string, base?: string): string | false;
   update(input: string, actionType: typeof urlUpdateActions, value: string): string | false;