Query does not return an error if `body` is invalid JSON

[chrillewoodz]

I've done some digging in the source and found this:

     if( fbutil.isString(queryData.body) ) {
       queryData.body = this._getJSON(queryData.body);
       if( queryData.body === null ) {
         this._replyError(key, 'Search body was a string but did not contain a valid JSON object. It must be an object or a JSON parsable string.');
       }
       return null;
     }

When you've stringified the body so it looks like this:

body: "{"query":{"bool":{"must":[{"nested":{"path":"guestlist","query":{"bool":{"must":[{"path":"list","query":{"bool":{"must":[{"match_phrase_prefix":{"list.name":"c"}}]}}}]}}}}]}}}"

This snippet below just returns null, causing the actual search function not to run at all since query is null here:

if( query === null ) { return; }

     this.esc.search(query, function(error, response) {
       if (error) {
         this._replyError(key, error);
       } else {
         this._reply(key, response);
       }
     }.bind(this));

It seems to me like the first snippet shouldn't return null, but rather something else. Such as queryData.body, however I tried to change it to this but then I got an error for my query. I am not sure if it's down to it being an invalid query but here it is:

      body: {
        query: {
          bool: {
            must: [
              {
                nested: {
                  path: 'guestlist',
                  query: {
                    bool: {
                      must: [
                        {
                          path: 'list',
                          query: {
                            bool: {
                              must: [
                                {
                                  match_phrase_prefix: {
                                    'list.name': query
                                  }
                                }
                              ]
                            }
                          }
                        }
                      ]
                    }
                  }
                }
              }
            ]
          }
        }
      }

And the error it spits out is:

{ Error: [illegal_argument_exception] request [/_search] contains unrecognized parameter: [query]
  status: 400,
  displayName: 'BadRequest',
  message: '[illegal_argument_exception] request [/_search] contains unrecognized parameter: [query]',
  path: '/_search',
  query: { query: { bool: [Object] } },
  body: undefined,
  statusCode: 400,
  response: '{"error":{"root_cause":[{"type":"illegal_argument_exception","reason":"request [/_search] contains unrecognized parameter: [query]"}],"type":"illegal_argument_exception","reason":"request [/_search] contains unrecognized parameter: [query]"},"status":400}',
  toString: [Function],
  toJSON: [Function] } { error:
   { root_cause: [ [Object] ],
     type: 'illegal_argument_exception',
     reason: 'request [/_search] contains unrecognized parameter: [query]' },
  status: 400 }

I hope you can investigate this.

[chrillewoodz]

I tried removing the return altogether and then a stringed query works as expected. I am unsure as to if this will cause other things to break however.

[katowulf]

If you look at the _buildQuery method, you'll see that the only way you can get a null returned there is one of the following:

1. You did not include type or index code
2. You did not include query (deprecated/legacy), body (object|string) or q (string) code
3. The body attribute was a string but did not contain valid JSON code

If you've verified that it's returning null when the body is a string, then it means it's not a valid JSON string, and there would be a warning in the logs.

It does seem like, rather than returning null in the _process method, we could return an error to the client. That part looks incorrect, but the end result is accurate--the body isn't valid JSON.