Fix erroneous requests due to HTTP-specific settings (#497)

# Description

The new `verb` HTTP specific option was handled by the SDK controllers, impacting all protocols, including WebSocket and MQTT

Impacted routes: `document:mGet` and `security:mGetUsers`
Those 2 routes send the `ids` argument to Kuzzle as a string, instead sending it as an array.

Fortunately, Kuzzle makes no difference between requests sent using HTTP or WebSocket or any other protocol, and it sanitizes the request in such a way that this bug is invisible to common users.

But documents aren't sanitized the same way when the API route is invoked by a plugin. And when that happens, the two routes mentioned earlier make any plugin invoking them crash.

This PR removes any specificities from the SDK controllers and, instead, makes the HTTP protocol handle the new `verb` option itself.

To do that, it simply converts any `body` object provided to a querystring if the selected API route uses the `GET` method. The conversion code was already there, it just needed a small fix when converting arrays (that conversion was never used before).

# Other changes

* Dependencies update
* Migrate the deprecate `mocha.opts` file to `.mocharc.json`
* Fix documentation deadlinks

# How to reproduce

Create a small plugin, and invoke `document:mGet` using `this.context.sdk.document.mGet`

The following error ensues: `Wrong type for argument "ids" (expected: array)`

Author: scottinet

.mocharc.json

@@ -0,0 +1,7 @@
+{
+  "reporter": "dot",
+  "recursive": true,
+  "slow": 2000,
+  "timeout": 10000,
+  "require": ["should-sinon"]
+}

doc/7/core-classes/kuzzle-error/introduction/index.md

@@ -10,4 +10,4 @@ order: 0
 
 Inherits from the standard `Error` class.
 
-The KuzzleError class represents an [error response from Kuzzle API](/core/2/api/essentials/errors).
+The KuzzleError class represents an [error response from Kuzzle API](/core/2/api/essentials/error-handling).

doc/7/essentials/error-handling/index.md

@@ -17,7 +17,7 @@ All SDK methods return a promise, that can be rejected with a `KuzzleError` valu
 | `status` | <pre>number</pre> | Status following [HTTP Standards](https://en.wikipedia.org/wiki/List_of_HTTP_status_codes) |
 | `stack`  | <pre>string</pre> | Error stacktrace (Only in development mode)                                                |
 
-You can find a detailed list of possible errors messages and statuses in the [documentation API](/core/2/api/essentials/errors).
+You can find a detailed list of possible errors messages and statuses in the [documentation API](/core/2/api/essentials/error-handling).
 
 #### Example with a promise chain
 

package-lock.json

@@ -1,6 +1,6 @@
 {
   "name": "kuzzle-sdk",
-  "version": "7.1.0",
+  "version": "7.1.1",
   "description": "Official Javascript SDK for Kuzzle",
   "author": "The Kuzzle Team <[email protected]>",
   "repository": {
@@ -39,32 +39,32 @@
   "main": "index.js",
   "license": "Apache-2.0",
   "dependencies": {
-    "@babel/core": "^7.8.4",
-    "@babel/preset-env": "^7.8.4",
+    "@babel/core": "^7.9.0",
+    "@babel/preset-env": "^7.9.0",
     "audit": "0.0.6",
-    "babel-loader": "^8.0.6",
+    "babel-loader": "^8.1.0",
     "kuzdoc": "^1.2.2",
     "min-req-promise": "^1.0.1",
-    "ora": "^3.4.0",
-    "webpack": "^4.41.6",
-    "ws": "^6.2.1"
+    "ora": "^4.0.3",
+    "webpack": "^4.42.1",
+    "ws": "^7.2.3"
   },
   "devDependencies": {
     "codecov": "^3.6.5",
-    "cucumber": "^5.1.0",
-    "eslint": "^5.16.0",
+    "cucumber": "^6.0.5",
+    "eslint": "^6.8.0",
     "eslint-friendly-formatter": "^4.0.1",
-    "eslint-loader": "^2.2.1",
-    "lolex": "^5.1.2",
-    "mocha": "6.2.0",
+    "eslint-loader": "^3.0.3",
+    "lolex": "^6.0.0",
+    "mocha": "7.1.1",
     "mock-require": "^3.0.3",
-    "nyc": "^14.1.1",
+    "nyc": "^15.0.0",
     "proxyquire": "^2.1.3",
     "retry": "^0.12.0",
-    "rewire": "^4.0.1",
+    "rewire": "^5.0.0",
     "should": "13.2.3",
     "should-sinon": "0.0.6",
-    "sinon": "^7.5.0"
+    "sinon": "^9.0.1"
   },
   "engines": {
     "node": ">= 10.13.0"

package.json

@@ -382,7 +382,8 @@ class Kuzzle extends KuzzleEventEmitter {
   }
 
   /**
-   * This is a low-level method, exposed to allow advanced SDK users to bypass high-level methods.
+   * This is a low-level method, exposed to allow advanced SDK users to bypass
+   * high-level methods.
    * Base method used to send read queries to Kuzzle
    *
    * Takes an optional argument object with the following properties:

src/Kuzzle.js

@@ -136,15 +136,10 @@ class DocumentController extends BaseController {
     const request = {
       index,
       collection,
-      action: 'mGet'
+      action: 'mGet',
+      body: {ids}
     };
 
-    if (options.verb === 'POST') {
-      request.body = {ids};
-    }
-    else {
-      request.ids = ids.join();
-    }
     return this.query(request, options)
       .then(response => response.result);
   }

src/controllers/document.js

@@ -357,15 +357,9 @@ class SecurityController extends BaseController {
 
   mGetUsers (ids, options = {}) {
     const request = {
-      action: 'mGetUsers'
+      action: 'mGetUsers',
+      body: {ids}
     };
-    
-    if (options.verb === 'POST') {
-      request.body = { ids };
-    }
-    else {
-      request.ids = ids.join();
-    }
 
     return this.query(request, options)
       .then(response => response.result.hits.map(hit => new User(this.kuzzle, hit._id, hit._source)));

src/controllers/security/index.js

@@ -136,26 +136,41 @@ class HttpWrapper extends KuzzleAbstractProtocol {
    * @returns {Promise<any>}
    */
   send (data, options = {}) {
-    const
-      payload = {
-        action: undefined,
-        body: undefined,
-        collection: undefined,
-        controller: undefined,
-        headers: {
-          'Content-Type': 'application/json'
-        },
-        index: undefined,
-        meta: undefined,
-        requestId: undefined,
+    const route = this.routes[data.controller]
+      && this.routes[data.controller][data.action];
+
+    if (! route) {
+      const error = new Error(`No URL found for "${data.controller}:${data.action}".`);
+      this.emit(data.requestId, { status: 400, error });
+      return;
+    }
+
+    const method = options.verb || route.verb;
+
+    const payload = {
+      action: undefined,
+      body: undefined,
+      collection: undefined,
+      controller: undefined,
+      headers: {
+        'Content-Type': 'application/json'
       },
-      queryArgs = {};
+      index: undefined,
+      meta: undefined,
+      requestId: undefined,
+    };
+    const queryArgs = {};
 
     for (const key of Object.keys(data)) {
       const value = data[key];
 
       if (key === 'body') {
-        payload.body = JSON.stringify(value);
+        if (method === 'GET') {
+          Object.assign(queryArgs, value);
+        }
+        else {
+          payload.body = JSON.stringify(value);
+        }
       }
       else if (key === 'jwt') {
         payload.headers.authorization = 'Bearer ' + value;
@@ -171,24 +186,10 @@ class HttpWrapper extends KuzzleAbstractProtocol {
       }
     }
 
-    const route = this.routes[payload.controller]
-        && this.routes[payload.controller][payload.action];
+    const regex = /\/:([^/]*)/;
 
-    if (! route) {
-      const error = new Error(`No URL found for "${payload.controller}:${payload.action}".`);
-
-      this.emit(payload.requestId, { status: 400, error });
-
-      return;
-    }
-
-    const
-      method = options.verb || route.verb,
-      regex = /\/:([^/]*)/;
-
-    let
-      url = route.url,
-      matches = regex.exec(url);
+    let url = route.url;
+    let matches = regex.exec(url);
 
     while (matches) {
       const urlParam = data[ matches[1] ];
@@ -214,8 +215,7 @@ class HttpWrapper extends KuzzleAbstractProtocol {
       const value = queryArgs[key];
 
       if (Array.isArray(value)) {
-        queryString.push(...value.map(v => `${key}=${v}`));
-
+        queryString.push(`${key}=${value.join()}`);
       }
       else if (typeof value === 'boolean') {
         // In Kuzzle, an optional boolean option is set to true if present in

src/protocols/http.js

@@ -256,38 +256,23 @@ describe('Document Controller', () => {
       ],
       total: 2
     };
-    it('should call document/mGet POST query and return a Promise which resolves the list of documents', () => {
+    it('should call document/mGet query and return a Promise which resolves to the list of documents', () => {
       kuzzle.query.resolves({result});
-      options.verb = 'POST';
-      return kuzzle.document.mGet('index', 'collection', ['document1', 'document2'], options)
-        .then(res => {
-          should(kuzzle.query)
-            .be.calledOnce()
-            .be.calledWith({
-              controller: 'document',
-              action: 'mGet',
-              index: 'index',
-              collection: 'collection',
-              body: {ids: ['document1', 'document2']}
-            }, options);
 
-          should(res).be.equal(result);
-        });
-    });
-    it('should call document/mGet GET query and return a Promise which resolves the list of documents', () => {
-      kuzzle.query.resolves({ result });
-      options.verb = undefined;
-      return kuzzle.document.mGet('index', 'collection', ['document1', 'document2'], options)
+      return kuzzle.document
+        .mGet('index', 'collection', ['document1', 'document2'], options)
         .then(res => {
           should(kuzzle.query)
             .be.calledOnce()
-            .be.calledWith({
-              controller: 'document',
-              action: 'mGet',
-              index: 'index',
-              collection: 'collection',
-              ids: 'document1,document2'
-            }, options);
+            .be.calledWith(
+              {
+                controller: 'document',
+                action: 'mGet',
+                index: 'index',
+                collection: 'collection',
+                body: {ids: ['document1', 'document2']}
+              },
+              options);
 
           should(res).be.equal(result);
         });