Skip to content

Commit

Permalink
[7.x] Apply all validation rules to empty strings (#46167) (#46598)
Browse files Browse the repository at this point in the history
  • Loading branch information
joshdover authored Sep 25, 2019
1 parent a7981fc commit e9526f5
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 12 deletions.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

24 changes: 24 additions & 0 deletions packages/kbn-config-schema/src/types/string_type.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ test('returns value is string and defined', () => {
expect(schema.string().validate('test')).toBe('test');
});

test('allows empty strings', () => {
expect(schema.string().validate('')).toBe('');
});

test('is required by default', () => {
expect(() => schema.string().validate(undefined)).toThrowErrorMatchingSnapshot();
});
Expand All @@ -41,6 +45,10 @@ describe('#minLength', () => {
test('returns error when shorter string', () => {
expect(() => schema.string({ minLength: 4 }).validate('foo')).toThrowErrorMatchingSnapshot();
});

test('returns error when empty string', () => {
expect(() => schema.string({ minLength: 2 }).validate('')).toThrowErrorMatchingSnapshot();
});
});

describe('#maxLength', () => {
Expand Down Expand Up @@ -84,6 +92,16 @@ describe('#hostname', () => {
const tooLongHostName = 'a'.repeat(256);
expect(() => hostNameSchema.validate(tooLongHostName)).toThrowErrorMatchingSnapshot();
});

test('returns error when empty string', () => {
expect(() => schema.string({ hostname: true }).validate('')).toThrowErrorMatchingSnapshot();
});

test('supports string validation rules', () => {
expect(() =>
schema.string({ hostname: true, maxLength: 3 }).validate('www.example.com')
).toThrowErrorMatchingSnapshot();
});
});

describe('#defaultValue', () => {
Expand Down Expand Up @@ -130,6 +148,12 @@ describe('#validate', () => {

expect(() => schema.string({ validate }).validate('foo')).toThrowErrorMatchingSnapshot();
});

test('throw when empty string', () => {
const validate = () => 'validator failure';

expect(() => schema.string({ validate }).validate('')).toThrowErrorMatchingSnapshot();
});
});

test('returns error when not string', () => {
Expand Down
34 changes: 22 additions & 12 deletions packages/kbn-config-schema/src/types/string_type.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,18 +29,33 @@ export type StringOptions = TypeOptions<string> & {

export class StringType extends Type<string> {
constructor(options: StringOptions = {}) {
let schema = internals.string().allow('');
// We want to allow empty strings, however calling `allow('')` casues
// Joi to whitelist the value and skip any additional validation.
// Instead, we reimplement the string validator manually except in the
// hostname case where empty strings aren't allowed anyways.
let schema =
options.hostname === true
? internals.string().hostname()
: internals.any().custom(value => {
if (typeof value !== 'string') {
return `expected value of type [string] but got [${typeDetect(value)}]`;
}
});

if (options.minLength !== undefined) {
schema = schema.min(options.minLength);
schema = schema.custom(value => {
if (value.length < options.minLength!) {
return `value is [${value}] but it must have a minimum length of [${options.minLength}].`;
}
});
}

if (options.maxLength !== undefined) {
schema = schema.max(options.maxLength);
}

if (options.hostname === true) {
schema = schema.hostname();
schema = schema.custom(value => {
if (value.length > options.maxLength!) {
return `value is [${value}] but it must have a maximum length of [${options.maxLength}].`;
}
});
}

super(schema, options);
Expand All @@ -49,12 +64,7 @@ export class StringType extends Type<string> {
protected handleError(type: string, { limit, value }: Record<string, any>) {
switch (type) {
case 'any.required':
case 'string.base':
return `expected value of type [string] but got [${typeDetect(value)}]`;
case 'string.min':
return `value is [${value}] but it must have a minimum length of [${limit}].`;
case 'string.max':
return `value is [${value}] but it must have a maximum length of [${limit}].`;
case 'string.hostname':
return `value is [${value}] but it must be a valid hostname (see RFC 1123).`;
}
Expand Down

0 comments on commit e9526f5

Please sign in to comment.