fix(isUrlRequest): better handle absolute urls and non standards (#134)

evilebottnawi · web-flow · commit aca43dabe807 · 2018-12-25T15:20:03.000+03:00
diff --git a/lib/isUrlRequest.js b/lib/isUrlRequest.js
@@ -1,15 +1,22 @@
 'use strict';
 
+const path = require('path');
+
 function isUrlRequest(url, root) {
   // An URL is not an request if
-  // 1. it's a Data Url
-  // 2. it's an absolute url or and protocol-relative
-  // 3. it's some kind of url for a template
-  if (
-    /^data:|^.+-extension:\/|^about:blank$|^(https?:)?\/\/|^[{}[]#*;,'§\$%&\(=?`´\^°<>]/.test(
-      url
-    )
-  ) {
+
+  // 1. It's an absolute url and it is not `windows` path like `C:\dir\file`
+  if (/^[a-z][a-z0-9+.-]*:/i.test(url) && !path.win32.isAbsolute(url)) {
+    return false;
+  }
+
+  // 2. It's a protocol-relative
+  if (/^\/\//.test(url)) {
+    return false;
+  }
+
+  // 3. It's some kind of url for a template
+  if (/^[{}[\]#*;,'§$%&(=?`´^°<>]/.test(url)) {
     return false;
   }
 
diff --git a/test/isUrlRequest.test.js b/test/isUrlRequest.test.js
@@ -14,7 +14,9 @@ describe('isUrlRequest()', () => {
     // without root
     [['//google.com'], false, 'should be negative for scheme-agnostic urls'],
     [['http://google.com'], false, 'should be negative for http urls'],
+    [['HTTP://google.com'], false, 'should be negative for http urls'],
     [['https://google.com'], false, 'should be negative for https urls'],
+    [['HTTPS://google.com'], false, 'should be negative for https urls'],
 
     [['chrome-extension://'], false, 'should be negative for nonstandard urls'],
     [['moz-extension://'], false, 'should be negative for nonstandard urls'],
@@ -26,6 +28,13 @@ describe('isUrlRequest()', () => {
     [['custom-extension://'], false, 'should be negative for nonstandard urls'],
 
     [['path/to/thing'], true, 'should be positive for implicit relative urls'],
+    [['./img.png'], true, 'should be positive for implicit relative urls'],
+    [['../img.png'], true, 'should be positive for implicit relative urls'],
+    [
+      ['./img.png?foo=bar#hash'],
+      true,
+      'should be positive for implicit relative urls',
+    ],
     [
       ['./path/to/thing'],
       true,
@@ -42,6 +51,18 @@ describe('isUrlRequest()', () => {
       true,
       'should be positive for module urls with relative path prefix',
     ],
+    [['C:/thing'], true, 'should be positive for linux path with driver'],
+    [['C:\\thing'], true, 'should be positive for windows path with driver'],
+    [
+      ['directory/things'],
+      true,
+      'should be positive for relative path (linux)',
+    ],
+    [
+      ['directory\\things'],
+      true,
+      'should be positive for relative path (windows)',
+    ],
 
     // with root (normal path)
     [
@@ -110,6 +131,48 @@ describe('isUrlRequest()', () => {
 
     // about url
     [['about:blank'], false, 'should be negative for about:blank'],
+
+    // hash
+    [['#gradient'], false, 'should be negative for hash url'],
+
+    // url
+    [['//sindresorhus.com'], false, 'should ignore noscheme url'],
+    [
+      ['//at.alicdn.com/t/font_515771_emcns5054x3whfr.eot'],
+      false,
+      'should ignore noscheme url with path',
+    ],
+    [
+      ['https://example.com/././foo'],
+      false,
+      'should ignore absolute url with relative',
+    ],
+
+    // non standard protocols
+    [
+      ['file://sindresorhus.com'],
+      false,
+      'should ignore non standard protocols (file)',
+    ],
+    [
+      ['mailto:someone@example.com'],
+      false,
+      'should ignore non standard protocols (mailto)',
+    ],
+    [
+      ['data:text/plain;base64,SGVsbG8sIFdvcmxkIQ%3D%3D'],
+      false,
+      'should ignore non standard protocols (data)',
+    ],
+    [
+      ['DATA:text/plain;base64,SGVsbG8sIFdvcmxkIQ%3D%3D'],
+      false,
+      'should ignore non standard protocols (data)',
+    ],
+
+    // root-relative url
+    [['/'], false, 'ignore root-relative url'],
+    [['//'], false, 'ignore root-relative url 1'],
   ].forEach((test) => {
     it(test[2], () => {
       const expected = test[1];
