From 0bc324b8baa36052805fa06674d62a25da58ed83 Mon Sep 17 00:00:00 2001 From: arunvjn <32433245+arunvjn@users.noreply.github.com> Date: Mon, 3 Apr 2023 23:23:10 +0530 Subject: [PATCH] fix: fixed broken file downloads for urls containing "(" or ")" (#21752) ## Description - fixed broken file downloads for urls containing "(" or ")" - Modified regex that checks to see if a string is a URL. - Added unit tests for isURL utility method. Fixes #13915 ## Type of change - Bug fix (non-breaking change which fixes an issue) ## How Has This Been Tested? - Manual - Jest ### Test Plan > Add Testsmith test cases links that relate to this PR ### Issues raised during DP testing > Link issues raised during DP testing for better visiblity and tracking (copy link from comments dropped on this PR) ## Checklist: ### Dev activity - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [ ] PR is being merged under a feature flag ### QA activity: - [ ] Test plan has been approved by relevant developers - [ ] Test plan has been peer reviewed by QA - [x] Cypress test cases have been added and approved by either SDET or manual QA - [ ] Organized project review call with relevant stakeholders after Round 1/2 of QA - [x] Added Test Plan Approved label after reveiwing all Cypress test --- app/client/src/utils/AppsmithUtils.test.ts | 27 ++++++++++++++++++++++ app/client/src/utils/TypeHelpers.ts | 4 ++-- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/app/client/src/utils/AppsmithUtils.test.ts b/app/client/src/utils/AppsmithUtils.test.ts index 865aa3eb39..cd4dbcb2a8 100644 --- a/app/client/src/utils/AppsmithUtils.test.ts +++ b/app/client/src/utils/AppsmithUtils.test.ts @@ -1,4 +1,5 @@ import { areArraysEqual, getCamelCaseString } from "utils/AppsmithUtils"; +import { isURL } from "./TypeHelpers"; describe("getCamelCaseString", () => { it("Should return a string in camelCase", () => { @@ -29,3 +30,29 @@ describe("test areArraysEqual", () => { expect(areArraysEqual(OGArray, testArray)).toBe(true); }); }); + +describe("isURL", () => { + test("returns true for valid URLs", () => { + expect(isURL("http://example.com")).toBe(true); + expect(isURL("https://www.google.com/search?q=javascript")).toBe(true); + expect(isURL("https://en.wikipedia.org/wiki/Regular_expression")).toBe( + true, + ); + expect( + isURL("https://www.example.com/path(withparentheses)/file.html"), + ).toBe(true); + expect( + isURL("https://www.example.com/path[withparentheses]/file_(1)[2].html"), + ).toBe(true); + }); + + test("returns false for invalid URLs", () => { + expect(isURL("http://localhost:3000")).toBe(false); + expect(isURL("not a URL")).toBe(false); + expect(isURL("ftp:/example.com")).toBe(false); + expect(isURL("http://example.")).toBe(false); + expect(isURL("http://localhost:port")).toBe(false); + expect(isURL("notAURL")).toBe(false); + expect(isURL("httpsnotAURL")).toBe(false); + }); +}); diff --git a/app/client/src/utils/TypeHelpers.ts b/app/client/src/utils/TypeHelpers.ts index 39f1aff515..ab99fa606f 100644 --- a/app/client/src/utils/TypeHelpers.ts +++ b/app/client/src/utils/TypeHelpers.ts @@ -27,10 +27,10 @@ export const getType = (value: unknown) => { export function isURL(str: string) { const pattern = new RegExp( - "^((blob:)?https?:\\/\\/)?" + // protocol + "^((blob:)?https?:\\/\\/)?" + //protocol "((([a-z\\d]([a-z\\d-]*[a-z\\d])*)\\.)+[a-z]{2,}|" + // domain name "((\\d{1,3}\\.){3}\\d{1,3}))" + // OR ip (v4) address - "(\\:\\d+)?(\\/[-a-z\\d%_.~+]*)*" + // port and path + "(\\:\\d+)?(/[^?#]*)?" + // port and path "(\\?[;&a-z\\d%_.~+=-]*)?" + // query string "(\\#[-a-z\\d_]*)?$", "i",