Improving the urls to not break protocols and adding tests (#3995)

* Improving the urls utils to not break dynamic protocols and testing the utils

Signed-off-by: iifawzi <iifawzie@gmail.com>

* Adding a list of blocked protocols

Signed-off-by: iifawzi <iifawzie@gmail.com>

* Update the way of sanitizing blocked protocols

Signed-off-by: iifawzi <iifawzie@gmail.com>

* Update shared/utils/urls.ts

Javascript pseudo protocol does not require the //

Co-authored-by: Tom Moor <tom.moor@gmail.com>

* updating the javascript protocol sanitizing tests

Signed-off-by: iifawzi <iifawzie@gmail.com>

* Update shared/utils/urls.test.ts

Co-authored-by: Apoorv Mishra <apoorvmishra101092@gmail.com>

* Update shared/utils/urls.ts

Co-authored-by: Apoorv Mishra <apoorvmishra101092@gmail.com>

* Using toBe instead of toEqual in tests

Signed-off-by: iifawzi <iifawzie@gmail.com>

* Sanitizing data: and vbscript:

Signed-off-by: iifawzi <iifawzie@gmail.com>

* Using toBeUndefined instead of toEqual in tests

Signed-off-by: iifawzi <iifawzie@gmail.com>

* Using URL to check the protocols

Signed-off-by: iifawzi <iifawzie@gmail.com>

* Allowing sms, fax, and tel protocols

Signed-off-by: iifawzi <iifawzie@gmail.com>

* Update shared/utils/urls.ts

inlining the protocols in the same file

Co-authored-by: Tom Moor <tom.moor@gmail.com>

* removing unused protocols constant

Signed-off-by: iifawzi <iifawzie@gmail.com>

Signed-off-by: iifawzi <iifawzie@gmail.com>
Co-authored-by: Tom Moor <tom.moor@gmail.com>
Co-authored-by: Apoorv Mishra <apoorvmishra101092@gmail.com>

shared/utils/urls.test.ts

@@ -1,5 +1,100 @@
+import * as urlsUtils from "./urls";
 import { urlRegex } from "./urls";
 
+describe("IsUrl Method", () => {
+  describe("invalid urls", () => {
+    it("should return false", () => {
+      expect(urlsUtils.isUrl("")).toBe(false);
+      expect(urlsUtils.isUrl("#invalidurl")).toBe(false);
+      expect(urlsUtils.isUrl("mailto:")).toBe(false);
+      expect(urlsUtils.isUrl("://")).toBe(false);
+    });
+  });
+});
+
+describe("isInternalUrl Method", () => {
+  it("should return false if empty string", () => {
+    expect(urlsUtils.isInternalUrl("")).toBe(false);
+  });
+
+  it("should return true if starting with relative path", () => {
+    expect(urlsUtils.isInternalUrl("/drafts")).toEqual(true);
+  });
+});
+
+describe("isExternalUrl Method", () => {
+  it("should return false if empty url", () => {
+    expect(urlsUtils.isExternalUrl("")).toBe(false);
+  });
+
+  it("should return false if internal url", () => {
+    expect(urlsUtils.isExternalUrl("/drafts")).toBe(false);
+  });
+});
+
+describe("sanitizeUrl Method", () => {
+  it("should return undefined if not url", () => {
+    expect(urlsUtils.sanitizeUrl(undefined)).toBeUndefined();
+    expect(urlsUtils.sanitizeUrl(null)).toBeUndefined();
+    expect(urlsUtils.sanitizeUrl("")).toBeUndefined();
+  });
+
+  it("should append https:// to non-special urls", () => {
+    expect(urlsUtils.sanitizeUrl("www.google.com")).toEqual(
+      "https://www.google.com"
+    );
+  });
+
+  describe("special urls", () => {
+    it("should return the url as it's if starting with /", () => {
+      expect(urlsUtils.sanitizeUrl("/drafts")).toEqual("/drafts");
+    });
+    it("should return the url as it's if starting with #", () => {
+      expect(urlsUtils.sanitizeUrl("#home")).toEqual("#home");
+    });
+    it("should return the url as it's if it's mailto:", () => {
+      expect(urlsUtils.sanitizeUrl("mailto:outline@getoutline.com")).toEqual(
+        "mailto:outline@getoutline.com"
+      );
+    });
+    it("should return the url as it's if it's mailto:", () => {
+      expect(urlsUtils.sanitizeUrl("mailto:outline@getoutline.com")).toEqual(
+        "mailto:outline@getoutline.com"
+      );
+    });
+    it("should return the url as it's if it's sms:, fax:, tel:", () => {
+      expect(urlsUtils.sanitizeUrl("mailto:outline@getoutline.com")).toEqual(
+        "mailto:outline@getoutline.com"
+      );
+      expect(urlsUtils.sanitizeUrl("tel:0123456789")).toEqual("tel:0123456789");
+      expect(urlsUtils.sanitizeUrl("fax:0123456789")).toEqual("fax:0123456789");
+      expect(urlsUtils.sanitizeUrl("sms:0123456789")).toEqual("sms:0123456789");
+    });
+    it("should return the url as it's if it's a special domain", () => {
+      expect(urlsUtils.sanitizeUrl("mqtt://getoutline.com")).toEqual(
+        "mqtt://getoutline.com"
+      );
+    });
+  });
+
+  describe("Blocked protocols", () => {
+    it("should be sanitized", () => {
+      expect(urlsUtils.sanitizeUrl("file://localhost.com/outline.txt")).toEqual(
+        "https://file://localhost.com/outline.txt"
+      );
+      expect(urlsUtils.sanitizeUrl("javascript:whatever")).toEqual(
+        "https://javascript:whatever"
+      );
+      expect(
+        urlsUtils.sanitizeUrl("data:text/html,<script>alert('hi');</script>")
+      ).toEqual("https://data:text/html,<script>alert('hi');</script>");
+      expect(urlsUtils.sanitizeUrl("vbscript:whatever")).toEqual(
+        "https://vbscript:whatever"
+      );
+    });
+  });
+});
+
 describe("#urlRegex", () => {
   it("should return undefined for invalid urls", () => {
     expect(urlRegex(undefined)).toBeUndefined();

shared/utils/urls.ts

@@ -55,7 +55,9 @@ export function isUrl(text: string) {
 
   try {
     const url = new URL(text);
-    return url.hostname !== "";
+    const blockedProtocols = ["javascript:", "file:", "vbscript:", "data:"];
+
+    return url.hostname !== "" && !blockedProtocols.includes(url.protocol);
   } catch (err) {
     return false;
   }
@@ -68,7 +70,7 @@ export function isUrl(text: string) {
  * @returns True if the url is external, false otherwise.
  */
 export function isExternalUrl(url: string) {
-  return !isInternalUrl(url);
+  return !!url && !isInternalUrl(url);
 }
 
 /**
@@ -87,7 +89,10 @@ export function sanitizeUrl(url: string | null | undefined) {
     !isUrl(url) &&
     !url.startsWith("/") &&
     !url.startsWith("#") &&
-    !url.startsWith("mailto:")
+    !url.startsWith("mailto:") &&
+    !url.startsWith("sms:") &&
+    !url.startsWith("fax:") &&
+    !url.startsWith("tel:")
   ) {
     return `https://${url}`;
   }