Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

http://example.com/ba*,-bar,-baz/_search will search over index foo #3839

Closed
ccw-morris opened this issue Oct 7, 2013 · 7 comments
Closed
Assignees
Labels
>bug :Data Management/Indices APIs APIs to create and manage indices and templates good first issue low hanging fruit

Comments

@ccw-morris
Copy link

Create three indexes: foo, bar, and baz.
Submit a search request using the URL: http://example.com/ba*,-bar,-baz/_search. This performs a search over all indices.

The logic of the MetaData.convertFromWildcards() method is as follows:
First token is ba* - expand that into bar and baz and add them to the result set.
Second token is a negation of bar so it removes that.
Third token is a negation of baz so it removes that to leave an empty set.

The problem is that an empty index set is also generated by http://example.com/_search.

There is later logic in PlainOperationRouting.computeTargetedShards() that treats the empty set as a request to search every index, including the index foo and the two indices that have been explicitly removed.

The proposed solution is:

  1. RestSearchAction.parseSearchRequest() to detect when request.param("index") is null and to explicitly populate the indices array with "_all". Both /_all/_search and /_search then follow the exact same logical path - this may avoid future bugs.
  2. PlainOperationRouting.computeTargetedShards() should treat an empty concreteIndices array as a null search.
@ghost ghost assigned drewr Oct 8, 2013
@ccw-morris
Copy link
Author

There's a related issue.

I'm looking at implementing an authorization model by appending a list of indexes or aliases that the user shouldn't see to the user request. So, if the user requests an index that they shouldn't then the logic strips it out again. Alternate solutions are welcome.

Within that context, a request for http://example.com/foo,-foo/_search makes sense. Unfortunately, this doesn't work.

When MetaData.convertFromWildcards() encounters a -foo, it will either:
a) remove it from the list of fields iff a previous term has caused an expansion
b) add all other fields iff it is the first term
c) add it as is, including the minus sign

So, foo,-foo returns an exception saying that it can't find an index called -foo.

Instead, I would propose that a non-first minus term will populate the results set with all previous entries encountered. This logic is also used when processing a wildcard expression to resolve the expression "foo,ba*".

@clintongormley
Copy link

The first issue has been fixed, the issue from the second comment is still valid:

GET  /foo,-foo/_search

@javanna javanna self-assigned this Mar 22, 2015
@javanna javanna removed the help wanted adoptme label Mar 22, 2015
@javanna javanna added the help wanted adoptme label May 28, 2015
@javanna javanna removed their assignment May 28, 2015
@clintongormley
Copy link

And, this searches all indices including bar and baz:

GET *,-bar,-baz/_search

While this search all indices except bar and baz:

GET -bar,-baz/_search

@clintongormley clintongormley added good first issue low hanging fruit :Data Management/Indices APIs APIs to create and manage indices and templates labels Sep 21, 2015
@qwerty4030
Copy link
Contributor

qwerty4030 commented Aug 26, 2016

I was able to reproduce this on latest master with the following:

PUT foo/foo-type/foo-doc
{}
PUT bar/bar-type/bar-doc
{}
PUT baz/baz-type/baz-doc
{}
GET  foo,-foo/_search

response:
{
  "error": {
    "root_cause": [
      {
        "type": "index_not_found_exception",
        "reason": "no such index",
        "resource.type": "index_expression",
        "resource.id": "-foo",
        "index_uuid": "_na_",
        "index": "-foo"
      }
    ],
    "type": "index_not_found_exception",
    "reason": "no such index",
    "resource.type": "index_expression",
    "resource.id": "-foo",
    "index_uuid": "_na_",
    "index": "-foo"
  },
  "status": 404
}

Took a quick look at the code... l'll make a PR shortly with the fix.
Unable to reproduce bug with GET *,-bar,-baz/_search and GET -bar,-baz/_search.

@clintongormley While I was investigating this I noticed the following request resulted in a similar error:

GET  foo,+foo/_search

response:
{
  "error": {
    "root_cause": [
      {
        "type": "index_not_found_exception",
        "reason": "no such index",
        "resource.type": "index_or_alias",
        "resource.id": " foo",
        "index_uuid": "_na_",
        "index": " foo"
      }
    ],
    "type": "index_not_found_exception",
    "reason": "no such index",
    "resource.type": "index_or_alias",
    "resource.id": " foo",
    "index_uuid": "_na_",
    "index": " foo"
  },
  "status": 404
}

However this error is slightly different than with GET foo,-foo/_search: it has "resource.type": "index_or_alias" instead of "resource.type": "index_expression". Also notice "index": " foo" has a space before foo. I guessed this might be something to do with URL encoding and tried the following:

GET  foo,%2Bfoo/_search

response:
{
  "error": {
    "root_cause": [
      {
        "type": "index_not_found_exception",
        "reason": "no such index",
        "resource.type": "index_expression",
        "resource.id": "+foo",
        "index_uuid": "_na_",
        "index": "+foo"
      }
    ],
    "type": "index_not_found_exception",
    "reason": "no such index",
    "resource.type": "index_expression",
    "resource.id": "+foo",
    "index_uuid": "_na_",
    "index": "+foo"
  },
  "status": 404
}

This is the same error as with GET foo,-foo/_search so that confirmed my suspicion. Is this a bug with ES or kibana/sense? Got the same behavior when I tried this with curl and an ES 2.3 cluster.

@qwerty4030
Copy link
Contributor

qwerty4030 commented Aug 26, 2016

Would the indices be resolved differently if the + was removed? If not what is the purpose of it?
GET foo,+foo/_search vs GET foo,foo/_search
GET foo,+ba*/_search vs GET foo,ba*/_search
GET +foo/_search vs GET foo/_search
GET ba*,+f*,-foo/_search vs GET ba*,f*,-foo/_search

I tried removing all the +s from WildcardExpressionResolverTests and the tests still pass so I guess its not needed?

qwerty4030 added a commit to qwerty4030/elasticsearch that referenced this issue Aug 27, 2016
…crete index followed by an add/remove concrete index.

The code now properly adds/removes the index instead of throwing an exception.

Closes elastic#3839
@javanna javanna removed the help wanted adoptme label Sep 12, 2016
@javanna javanna self-assigned this Sep 12, 2016
javanna pushed a commit to javanna/elasticsearch that referenced this issue Sep 12, 2016
…crete index followed by an add/remove concrete index.

The code now properly adds/removes the index instead of throwing an exception.

Closes elastic#3839
javanna pushed a commit to javanna/elasticsearch that referenced this issue Sep 12, 2016
…crete index followed by an add/remove concrete index.

The code now properly adds/removes the index instead of throwing an exception.

Closes elastic#3839
javanna pushed a commit to javanna/elasticsearch that referenced this issue Sep 12, 2016
…crete index followed by an add/remove concrete index.

The code now properly adds/removes the index instead of throwing an exception.

Closes elastic#3839
@gerits
Copy link

gerits commented Dec 7, 2016

There still seems to be a problem here. When I run the following query it won't work. But if I switch the position of the indices it works.
non working: GET /-foo,*/_search
working: GET /*,-foo/_search

It seems the - sign is not interpreted correctly as in following example.
Not working: GET /-foo,+bar/_search
working: GET /-*foo,+bar/_search

This is for version 5.0.2

@javanna
Copy link
Member

javanna commented Dec 7, 2016

Hi @gerits yes that is correct. It's a feature, not a bug, see #20898. The negation has an effect only if there is a preceding wildcard expression. The implicit negation that we had before led to unexpected results, especially when e.g. deleting indices.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Data Management/Indices APIs APIs to create and manage indices and templates good first issue low hanging fruit
Projects
None yet
Development

No branches or pull requests

6 participants