Skip to content

Commit

Permalink
fix: check target tooptip (#971)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
w1kman authored Oct 21, 2024
1 parent c7494ca commit e5533fe
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 22 deletions.
4 changes: 2 additions & 2 deletions src/schemas/general/HostNameTarget.ts
Original file line number Diff line number Diff line change
@@ -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({
Expand Down
10 changes: 5 additions & 5 deletions src/validation.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {
validateDomain,
validateHostname,
validateHostAddress,
validateHostPort,
validateHttpTarget,
validateLabelName,
Expand Down Expand Up @@ -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');
});
});

Expand All @@ -100,7 +100,7 @@ describe('PING', () => {
'224.0.0.0',
];
testcases.forEach((testcase: string) => {
expect(validateHostname(testcase)).toBe(undefined);
expect(validateHostAddress(testcase)).toBe(undefined);
});
});

Expand All @@ -115,7 +115,7 @@ describe('PING', () => {
'ff00::', // multicast address
];
testcases.forEach((testcase: string) => {
expect(validateHostname(testcase)).toBe(undefined);
expect(validateHostAddress(testcase)).toBe(undefined);
});
});
});
Expand Down
27 changes: 12 additions & 15 deletions src/validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand All @@ -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 {
Expand All @@ -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);
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);
Expand All @@ -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) {
Expand All @@ -273,5 +270,5 @@ export function validateHostPort(target: string): string | undefined {
return 'Port must be greater than 0';
}

return validateHostname(host);
return validateHostAddress(host);
}

0 comments on commit e5533fe

Please sign in to comment.