fix(isURL): Correct the patch and apply feedback

Author: theofidry

src/lib/isURL.js

@@ -34,6 +34,7 @@ max_allowed_length - if set, isURL will not allow URLs longer than the specified
 
 */
 
+
 const default_url_options = {
   protocols: ['http', 'https', 'ftp'],
   require_tld: true,
@@ -70,10 +71,7 @@ export default function isURL(url, options) {
     return false;
   }
 
-  if (
-    !options.allow_query_components &&
-    (includes(url, '?') || includes(url, '&'))
-  ) {
+  if (!options.allow_query_components && (includes(url, '?') || includes(url, '&'))) {
     return false;
   }
 
@@ -87,30 +85,91 @@ export default function isURL(url, options) {
 
   // Replaced the 'split("://")' logic with a regex to match the protocol.
   // This correctly identifies schemes like `javascript:` which don't use `//`.
+  // However, we need to be careful not to confuse authentication credentials (user:password@host)
+  // with protocols. A colon before an @ symbol might be part of auth, not a protocol separator.
   const protocol_match = url.match(/^([a-z][a-z0-9+\-.]*):/i);
-  const hadExplicitProtocol = !!protocol_match;
+  let had_explicit_protocol = false;
+
+  const cleanUpProtocol = (potential_protocol) => {
+    had_explicit_protocol = true;
+    protocol = potential_protocol.toLowerCase();
+
+    if (options.require_valid_protocol && options.protocols.indexOf(protocol) === -1) {
+      // The identified protocol is not in the allowed list.
+      return false;
+    }
+
+    // Remove the protocol from the URL string.
+    return url.substring(protocol_match[0].length);
+  };
 
   if (protocol_match) {
-    protocol = protocol_match[1].toLowerCase();
-    if (
-      options.require_valid_protocol &&
-      options.protocols.indexOf(protocol) === -1
-    ) {
-      return false; // The identified protocol is not in the allowed list.
+    const potential_protocol = protocol_match[1];
+    const after_colon = url.substring(protocol_match[0].length);
+
+    // Check if what follows looks like authentication credentials (user:password@host)
+    // rather than a protocol. This happens when:
+    // 1. There's no `//` after the colon (protocols like `http://` have this)
+    // 2. There's an `@` symbol before any `/`
+    // 3. The part before `@` contains only valid auth characters (alphanumeric, -, _, ., %, :)
+    const starts_with_slashes = after_colon.slice(0, 2) === '//';
+
+    if (!starts_with_slashes) {
+      const first_slash_position = after_colon.indexOf('/');
+      const before_slash = first_slash_position === -1
+        ? after_colon
+        : after_colon.substring(0, first_slash_position);
+      const at_position = before_slash.indexOf('@');
+
+      if (at_position !== -1) {
+        const before_at = before_slash.substring(0, at_position);
+        const valid_auth_regex = /^[a-zA-Z0-9\-_.%:]*$/;
+        const is_valid_auth = valid_auth_regex.test(before_at);
+
+        if (is_valid_auth) {
+          // This looks like authentication (e.g., user:password@host), not a protocol
+          if (options.require_protocol) {
+            return false;
+          }
+
+          // Don't consume the colon; let the auth parsing handle it later
+        } else {
+          // This looks like a malicious protocol (e.g., javascript:alert();@host)
+          url = cleanUpProtocol(potential_protocol);
+
+          if (url === false) {
+            return false;
+          }
+        }
+      } else {
+        // No @ symbol, this is definitely a protocol
+        url = cleanUpProtocol(potential_protocol);
+
+        if (url === false) {
+          return false;
+        }
+      }
+    } else {
+      // Starts with '//', this is definitely a protocol like http://
+      url = cleanUpProtocol(potential_protocol);
+
+      if (url === false) {
+        return false;
+      }
     }
-    url = url.substring(protocol_match[0].length); // Remove the protocol from the URL string.
   } else if (options.require_protocol) {
-    return false; // A protocol was required but not found.
+    return false;
   }
 
   // Handle leading '//' only as protocol-relative when there was NO explicit protocol.
   // If there was an explicit protocol, '//' is the normal separator
   // and should be stripped unconditionally.
   if (url.slice(0, 2) === '//') {
-    if (!hadExplicitProtocol && !options.allow_protocol_relative_urls) {
+    if (!had_explicit_protocol && !options.allow_protocol_relative_urls) {
       return false;
     }
-    url = url.slice(2); // Remove the '//' from the URL string.
+
+    url = url.slice(2);
   }
 
   if (url === '') {

test/validators.test.js

@@ -792,12 +792,11 @@ describe('Validators', () => {
         host_whitelist: DOMAIN_WHITELIST,
         require_host: false,
       }],
-      valid: [
-        // TODO: the expected result is **INVALID**.
+      valid: [],
+      invalid: [
         // eslint-disable-next-line no-script-url
         "javascript:alert(1);a=';@example.com/alert(1)",
       ],
-      invalid: [],
     });
   });