From e5533fe9980da251d1730ce8de5112748d09fc2a Mon Sep 17 00:00:00 2001 From: Thomas Wikman Date: Mon, 21 Oct 2024 16:00:00 +0200 Subject: [PATCH] fix: check target tooptip (#971) * fix: host address validation - Change the validation error to say "host address" instead of "hostname" * chore: fix code style issues - throw of exception caught locally - redundant variable initializer - redundant if-statement - RegExp: redundant character escape --- src/schemas/general/HostNameTarget.ts | 4 ++-- src/validation.test.ts | 10 +++++----- src/validation.ts | 27 ++++++++++++--------------- 3 files changed, 19 insertions(+), 22 deletions(-) diff --git a/src/schemas/general/HostNameTarget.ts b/src/schemas/general/HostNameTarget.ts index 90aa9b198..99dda247f 100644 --- a/src/schemas/general/HostNameTarget.ts +++ b/src/schemas/general/HostNameTarget.ts @@ -1,13 +1,13 @@ import { z } from 'zod'; -import { validateHostname } from 'validation'; +import { validateHostAddress } from 'validation'; import { TargetSchema } from './Target'; export const HostNameTargetSchema = TargetSchema.and(z.string().superRefine(validate)); function validate(target: string, ctx: z.RefinementCtx) { - const message = validateHostname(target); + const message = validateHostAddress(target); if (message) { return ctx.addIssue({ diff --git a/src/validation.test.ts b/src/validation.test.ts index 7a9db63be..a06a86375 100644 --- a/src/validation.test.ts +++ b/src/validation.test.ts @@ -1,6 +1,6 @@ import { validateDomain, - validateHostname, + validateHostAddress, validateHostPort, validateHttpTarget, validateLabelName, @@ -80,12 +80,12 @@ describe('http', () => { describe('PING', () => { it('should reject hostnames without domains', async () => { - expect(validateHostname('grafana')).toBe('Target must be a valid hostname'); + expect(validateHostAddress('grafana')).toBe('Target must be a valid host address'); }); it('should reject ping targets with invalid hostnames', async () => { const testcases: string[] = ['x.', '.y', 'x=y.org']; testcases.forEach((testcase: string) => { - expect(validateHostname(testcase)).toBe('Target must be a valid hostname'); + expect(validateHostAddress(testcase)).toBe('Target must be a valid host address'); }); }); @@ -100,7 +100,7 @@ describe('PING', () => { '224.0.0.0', ]; testcases.forEach((testcase: string) => { - expect(validateHostname(testcase)).toBe(undefined); + expect(validateHostAddress(testcase)).toBe(undefined); }); }); @@ -115,7 +115,7 @@ describe('PING', () => { 'ff00::', // multicast address ]; testcases.forEach((testcase: string) => { - expect(validateHostname(testcase)).toBe(undefined); + expect(validateHostAddress(testcase)).toBe(undefined); }); }); }); diff --git a/src/validation.ts b/src/validation.ts index fea834d2a..fb9a66457 100644 --- a/src/validation.ts +++ b/src/validation.ts @@ -5,14 +5,14 @@ import validUrl from 'valid-url'; import { Label } from 'types'; export function validateHttpTarget(target: string) { - let message = 'Target must be a valid web URL'; + const message = 'Target must be a valid web URL'; try { const httpEncoded = encodeURI(target); const isValidUrl = Boolean(validUrl.isWebUri(httpEncoded)); if (!isValidUrl) { - throw new Error(message); + return message; } } catch { return message; @@ -22,8 +22,7 @@ export function validateHttpTarget(target: string) { const parsedUrl = new URL(target); if (!parsedUrl.protocol) { - message = 'Target must have a valid protocol'; - throw new Error(message); + return 'Target must have a valid protocol'; } // isWebUri will allow some invalid hostnames, so we need addional validation @@ -33,7 +32,7 @@ export function validateHttpTarget(target: string) { } const hostname = parsedUrl.hostname; - if (validateHostname(hostname)) { + if (validateHostAddress(hostname)) { return 'Target must have a valid hostname'; } } catch { @@ -44,7 +43,7 @@ export function validateHttpTarget(target: string) { } function isIpV6FromUrl(target: string) { - let isIpV6 = true; + let isIpV6; try { const address = Address6.fromURL(target); isIpV6 = Boolean(address.address); @@ -165,10 +164,8 @@ export function validateDomain(target: string): string | undefined { const filteredElements = rawElements.filter((element, index) => { const isLast = index === rawElements.length - 1; - if (isLast && element === '') { - return false; - } - return true; + + return !(isLast && element === ''); }); const errors = filteredElements @@ -196,7 +193,7 @@ function isCharacterLetter(character: string): boolean { } function isValidDomainCharacter(character: string): boolean { - const regex = new RegExp(/[-A-Za-z0-9\.]/); + const regex = new RegExp(/[-A-Za-z0-9.]/); return Boolean(!character.match(regex)?.length); } @@ -235,7 +232,7 @@ function validateDomainElement(element: string, isLast: boolean): string | undef return undefined; } -export function validateHostname(target: string): string | undefined { +export function validateHostAddress(target: string): string | undefined { const ipv4 = isIpV4(target); const ipv6 = isIpV6(target); const pc = punycode.toASCII(target); @@ -245,14 +242,14 @@ export function validateHostname(target: string): string | undefined { 'i' ); if (!pc.match(re) && !ipv4 && !ipv6) { - return 'Target must be a valid hostname'; + return 'Target must be a valid host address'; } return undefined; } export function validateHostPort(target: string): string | undefined { - const re = new RegExp(/^(?:\[([0-9a-f:.]+)\]|([^:]+)):(\d+)$/, 'i'); + const re = new RegExp(/^(?:\[([0-9a-f:.]+)]|([^:]+)):(\d+)$/, 'i'); const match = target.match(re); if (match === null) { @@ -273,5 +270,5 @@ export function validateHostPort(target: string): string | undefined { return 'Port must be greater than 0'; } - return validateHostname(host); + return validateHostAddress(host); }