From 21516852d99a3be48e08e582329a651bf1a01471 Mon Sep 17 00:00:00 2001 From: ibolton336 Date: Thu, 20 Jul 2023 17:48:08 -0400 Subject: [PATCH 1/2] :bug: Add url validation to rules step Signed-off-by: ibolton336 --- .../app/pages/applications/analysis-wizard/schema.ts | 10 ++++------ client/src/app/utils/utils.ts | 9 +++++---- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/client/src/app/pages/applications/analysis-wizard/schema.ts b/client/src/app/pages/applications/analysis-wizard/schema.ts index 42a27db38d..465a5f332f 100644 --- a/client/src/app/pages/applications/analysis-wizard/schema.ts +++ b/client/src/app/pages/applications/analysis-wizard/schema.ts @@ -11,6 +11,7 @@ import { } from "@app/api/models"; import { useTranslation } from "react-i18next"; import { useAnalyzableApplicationsByMode } from "./utils"; +import { customURLValidation } from "@app/utils/utils"; export const ANALYSIS_MODES = [ "binary", @@ -156,13 +157,10 @@ const useCustomRulesStepSchema = (): yup.SchemaOf => { is: "repository", then: yup.mixed().required(), }), - sourceRepository: yup.mixed().when("rulesKind", { + sourceRepository: yup.string().when("rulesKind", { is: "repository", - then: yup - .string() - .required("This value is required") - .min(3, t("validation.minLength", { length: 3 })) - .max(120, t("validation.maxLength", { length: 120 })), + then: (schema) => + customURLValidation(schema).required("Enter repository url."), }), branch: yup.mixed().when("rulesKind", { is: "repository", diff --git a/client/src/app/utils/utils.ts b/client/src/app/utils/utils.ts index 3376d009e6..b736f8bec8 100644 --- a/client/src/app/utils/utils.ts +++ b/client/src/app/utils/utils.ts @@ -94,14 +94,15 @@ export const getValidatedFromError = (error: unknown | undefined) => { }; export const standardURLRegex = - /(https?:\/\/(?:www\.|(?!www))[a-zA-Z0-9][a-zA-Z0-9-]+[a-zA-Z0-9]\.[^\s]{2,}|www\.[a-zA-Z0-9][a-zA-Z0-9-]+[a-zA-Z0-9]\.[^\s]{2,}|https?:\/\/(?:www\.|(?!www))[a-zA-Z0-9]+\.[^\s]{2,}|www\.[a-zA-Z0-9]+\.[^\s]{2,})/gi; + /^(https?:\/\/(?:www\.|(?!www))[a-zA-Z0-9][a-zA-Z0-9-]+[a-zA-Z0-9]\.[^\s]{2,}|www\.[a-zA-Z0-9][a-zA-Z0-9-]+[a-zA-Z0-9]\.[^\s]{2,}|https?:\/\/(?:www\.|(?!www))[a-zA-Z0-9]+\.[^\s]{2,}|www\.[a-zA-Z0-9]+\.[^\s]{2,})$/; + +export const gitUrlRegex = + /^(?:git|ssh|https?|git@[-\w.]+):(\/\/)?(.*?)(\/?|\#[-\d\w._]+?)$/; export const standardStrictURLRegex = - /https:\/\/(www\.)?[-a-zA-Z0-9@:%._\+~#=]{2,256}\.[a-z]{2,4}\b([-a-zA-Z0-9@:%_\+.~#?&//=]*)/gi; + /https:\/\/(www\.)?[-a-zA-Z0-9@:%._\+~#=]{2,256}\.[a-z]{2,4}\b([-a-zA-Z0-9@:%_\+.~#?&//=]*)/; export const customURLValidation = (schema: StringSchema) => { - const gitUrlRegex = - /(?:git|ssh|https?|git@[-\w.]+):(\/\/)?(.*?)(\/?|\#[-\d\w._]+?)$/; const containsURL = (string: string) => gitUrlRegex.test(string) || standardURLRegex.test(string); From c0154931113a2cdfb595917abeb9d4cfcc5fd19c Mon Sep 17 00:00:00 2001 From: ibolton336 Date: Thu, 20 Jul 2023 21:28:22 -0400 Subject: [PATCH 2/2] Add unit tests for regex Signed-off-by: ibolton336 --- client/src/app/utils/utils.test.ts | 66 ++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/client/src/app/utils/utils.test.ts b/client/src/app/utils/utils.test.ts index 248c77e94a..1940677c35 100644 --- a/client/src/app/utils/utils.test.ts +++ b/client/src/app/utils/utils.test.ts @@ -1,9 +1,14 @@ +import { act, render } from "@testing-library/react"; +import { renderHook } from "@testing-library/react-hooks"; import { AxiosError } from "axios"; import { getAxiosErrorMessage, getValidatedFromError, getValidatedFromErrors, getToolbarChipKey, + customURLValidation, + gitUrlRegex, + standardURLRegex, } from "./utils"; describe("utils", () => { @@ -89,4 +94,65 @@ describe("utils", () => { const result = getToolbarChipKey({ key: "myKey", node: "myNode" }); expect(result).toBe("myKey"); }); + + //URL Regex tests + it("Regex should validate git URLs", () => { + const testGitURLs: string[] = [ + "git@github.com:konveyor/tackle2-ui", + "http://git@github.com:konveyor/tackle2-ui", + ]; + + for (const url of testGitURLs) { + const gitTestResult = gitUrlRegex.test(url); + expect(gitTestResult).toBe(true); + } + }); + + it("Regex should validate standard URLs", () => { + const testStandardURLs: string[] = [ + "http://www.foo.bar", + "www.foo.bar", + "https://www.github.com/ibolton336/tackle-testapp.git", + ]; + + for (const url of testStandardURLs) { + const standardTestResult = standardURLRegex.test(url); + expect(standardTestResult).toBe(true); + } + }); + + it("Regex should fail when validating broken standard URLs", () => { + const testBrokenURLs: string[] = [ + "", + " http://www.foo.bar ", + " http://www.foo", + " http://wrong", + "wwwfoo.bar", + "foo.bar", + "www.foo.b", + "foo.ba", + "git@github.com:konveyor/tackle2-ui", + ]; + + for (const url of testBrokenURLs) { + const testResult = standardURLRegex.test(url); + expect(testResult).toBe(false); + } + }); + + it("URL should match the same multiple times in a row", () => { + // Motivation for this test: + // https://stackoverflow.com/a/21373261/1183614 + // https://stackoverflow.com/a/1520853/1183614 + // When using the global flag on a regex, subsequent results will return a result from the last used index. + // In sum, we should never be using a global flag in combination with the .test method. + + const url = "https://github.com/ibolton336/tackle-testapp.git"; + + expect(standardURLRegex.test(url)).toBe(true); + expect(standardURLRegex.test(url)).toBe(true); + expect(standardURLRegex.test(url)).toBe(true); + expect(standardURLRegex.test(url)).toBe(true); + expect(standardURLRegex.test(url)).toBe(true); + }); });