[7.x] Apply all validation rules to empty strings (#46167) (#46598)

packages/kbn-config-schema/src/types/__snapshots__/string_type.test.ts.snap

@@ -1,5 +1,7 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
+exports[`#hostname returns error when empty string 1`] = `"any.empty"`;
+
 exports[`#hostname returns error when value is not a valid hostname 1`] = `"value is [host:name] but it must be a valid hostname (see RFC 1123)."`;
 
 exports[`#hostname returns error when value is not a valid hostname 2`] = `"value is [localhost:5601] but it must be a valid hostname (see RFC 1123)."`;
@@ -10,10 +12,16 @@ exports[`#hostname returns error when value is not a valid hostname 4`] = `"valu
 
 exports[`#hostname returns error when value is not a valid hostname 5`] = `"value is [aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa] but it must be a valid hostname (see RFC 1123)."`;
 
+exports[`#hostname supports string validation rules 1`] = `"value is [www.example.com] but it must have a maximum length of [3]."`;
+
 exports[`#maxLength returns error when longer string 1`] = `"value is [foo] but it must have a maximum length of [2]."`;
 
+exports[`#minLength returns error when empty string 1`] = `"value is [] but it must have a minimum length of [2]."`;
+
 exports[`#minLength returns error when shorter string 1`] = `"value is [foo] but it must have a minimum length of [4]."`;
 
+exports[`#validate throw when empty string 1`] = `"validator failure"`;
+
 exports[`#validate throws when returns string 1`] = `"validator failure"`;
 
 exports[`includes namespace in failure 1`] = `"[foo-namespace]: expected value of type [string] but got [undefined]"`;

packages/kbn-config-schema/src/types/string_type.test.ts

@@ -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();
 });
@@ -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', () => {
@@ -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', () => {
@@ -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', () => {

packages/kbn-config-schema/src/types/string_type.ts

@@ -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);
@@ -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).`;
     }