Cleanup for #1487.

hapijs · Jun 29, 2018 · 2391f72 · 2391f72
1 parent 7aa0df0
commit 2391f72
Show file tree

Hide file tree

Showing 5 changed files with 44 additions and 8 deletions.
diff --git a/API.md b/API.md
@@ -2038,6 +2038,7 @@ Requires the string value to be a valid [RFC 3986](http://tools.ietf.org/html/rf
     - `scheme` - Specifies one or more acceptable Schemes, should only include the scheme name. Can be an Array or String (strings are automatically escaped for use in a Regular Expression).
     - `allowRelative` - Allow relative URIs. Defaults to `false`.
     - `relativeOnly` - Restrict only relative URIs.  Defaults to `false`.
+    - `allowQuerySquareBrackets` - Allows unencoded square brackets inside the query string. This is **NOT** RFC 3986 compliant but query strings like `abc[]=123&abc[]=456` are very common these days. Defaults to `false`.
 
 ```js
 // Accept git or git http/https

diff --git a/lib/types/string/index.js b/lib/types/string/index.js
@@ -276,11 +276,15 @@ internals.String = class extends Any {
         let customScheme = '';
         let allowRelative = false;
         let relativeOnly = false;
+        let allowQuerySquareBrackets = false;
         let regex = internals.uriRegex;
 
         if (uriOptions) {
             Hoek.assert(typeof uriOptions === 'object', 'options must be an object');
 
+            const unknownOptions = Object.keys(uriOptions).filter((key) => !['scheme', 'allowRelative', 'relativeOnly', 'allowQuerySquareBrackets'].includes(key));
+            Hoek.assert(unknownOptions.length === 0, `options contain unknown keys: ${unknownOptions}`);
+
             if (uriOptions.scheme) {
                 Hoek.assert(uriOptions.scheme instanceof RegExp || typeof uriOptions.scheme === 'string' || Array.isArray(uriOptions.scheme), 'scheme must be a RegExp, String, or Array');
 
@@ -316,10 +320,14 @@ internals.String = class extends Any {
             if (uriOptions.relativeOnly) {
                 relativeOnly = true;
             }
+
+            if (uriOptions.allowQuerySquareBrackets) {
+                allowQuerySquareBrackets = true;
+            }
         }
 
-        if (customScheme || allowRelative || relativeOnly) {
-            regex = Uri.createUriRegex(customScheme, allowRelative, relativeOnly);
+        if (customScheme || allowRelative || relativeOnly || allowQuerySquareBrackets) {
+            regex = Uri.createUriRegex(customScheme, allowRelative, relativeOnly, allowQuerySquareBrackets);
         }
 
         return this._test('uri', uriOptions, function (value, state, options) {

diff --git a/lib/types/string/rfc3986.js b/lib/types/string/rfc3986.js
@@ -79,9 +79,9 @@ internals.generate = function () {
     const pcharOnly = '[' + pchar + ']';
 
     /**
-     * squareBraces example: []
+     * squareBrackets example: []
      */
-    const squareBraces = '\\[\\]';
+    const squareBrackets = '\\[\\]';
 
     /**
      * dec-octet   = DIGIT                 ; 0-9
@@ -200,7 +200,12 @@ internals.generate = function () {
     /**
      * query = *( pchar / "/" / "?" )
      */
-    internals.rfc3986.query = '[' + pchar + squareBraces + '\\/\\?]*(?=#|$)'; //Finish matching either at the fragment part or end of the line.
+    internals.rfc3986.query = '[' + pchar + '\\/\\?]*(?=#|$)'; //Finish matching either at the fragment part or end of the line.
+
+    /**
+     * query = *( pchar / "[" / "]" / "/" / "?" )
+     */
+    internals.rfc3986.queryWithSquareBrackets = '[' + pchar + squareBrackets + '\\/\\?]*(?=#|$)'; //Finish matching either at the fragment part or end of the line.
 
     /**
      * fragment = *( pchar / "/" / "?" )

diff --git a/lib/types/string/uri.js b/lib/types/string/uri.js
@@ -9,7 +9,7 @@ const RFC3986 = require('./rfc3986');
 
 const internals = {
     Uri: {
-        createUriRegex: function (optionalScheme, allowRelative, relativeOnly) {
+        createUriRegex: function (optionalScheme, allowRelative, relativeOnly, allowQuerySquareBrackets) {
 
             let scheme = RFC3986.scheme;
             let prefix;
@@ -37,7 +37,7 @@ const internals = {
              *
              * relative-ref = relative-part [ "?" query ] [ "#" fragment ]
              */
-            return new RegExp('^' + prefix + '(?:\\?' + RFC3986.query + ')?' + '(?:#' + RFC3986.fragment + ')?$');
+            return new RegExp('^' + prefix + '(?:\\?' + (allowQuerySquareBrackets ? RFC3986.queryWithSquareBrackets : RFC3986.query) + ')?' + '(?:#' + RFC3986.fragment + ')?$');
         }
     }
 };

diff --git a/test/types/string.js b/test/types/string.js
@@ -2523,7 +2523,15 @@ describe('string', () => {
 
             Helper.validate(schema, [
                 ['foo://example.com:8042/over/there?name=ferret#nose', true],
-                ['https://example.com?abc[]=123&abc[]=456', true],
+                ['https://example.com?abc[]=123&abc[]=456', false, null, {
+                    message:'"value" must be a valid uri',
+                    details: [{
+                        message: '"value" must be a valid uri',
+                        path: [],
+                        type: 'string.uri',
+                        context: { value: 'https://example.com?abc[]=123&abc[]=456', label: 'value', key: undefined }
+                    }]
+                }],
                 ['urn:example:animal:ferret:nose', true],
                 ['ftp://ftp.is.co.za/rfc/rfc1808.txt', true],
                 ['http://www.ietf.org/rfc/rfc2396.txt', true],
@@ -4379,6 +4387,20 @@ describe('string', () => {
                 ['/absolute', true]
             ]);
         });
+
+        it('validates uri with square brackets allowed', () => {
+
+            const schema = Joi.string().uri({ allowQuerySquareBrackets: true });
+
+            Helper.validate(schema, [
+                ['https://example.com?abc[]=123&abc[]=456', true]
+            ]);
+        });
+
+        it('warns about unknown options', () => {
+
+            expect(() => Joi.string().uri({ foo: 'bar', baz: 'qux' })).to.throw('options contain unknown keys: foo,baz');
+        });
     });
 
     describe('truncate()', () => {