kuzzleio / kuzzle

Open-source Back-end, self-hostable & ready to use - Real-time, storage, advanced search - Web, Apps, Mobile, IoT -
https://kuzzle.io
Apache License 2.0
1.43k stars 123 forks source link

Unable to remove filters containing nested attributes #302

Closed scottinet closed 8 years ago

scottinet commented 8 years ago

Renamed this issue after finding more about the bug. See below

This bug has been reported on the beta branch of Kuzzle, and is still live on the latest develop branch.

How to reproduce:

{
   "and":[
      {
         "or":[
            {
               "term":{
                  "accepted":true
               }
            },
            {
               "term":{
                  "user.id":"jane"
               }
            }
         ]
      },
      {
         "term":{
            "channel":"spotkanie"
         }
      }
   ]
}
TypeError: Cannot read property 'fields' of undefined
    at Filters.removeFilterPath (/var/app/lib/api/dsl/filters.js:559:15)
    at /var/app/lib/api/dsl/filters.js:371:24
    at /var/app/node_modules/async/lib/async.js:181:20
    at Object.async.forEachOf.async.eachOf (/var/app/node_modules/async/lib/async.js:233:13)
    at Object.async.forEach.async.each (/var/app/node_modules/async/lib/async.js:209:22)
    at Filters.addSubscription.testFieldFilters.async.each.async.each.removeFieldFilter.filterId [as removeFieldFilter] (/var/app/lib/api/dsl/filters.js:370:11)
    at /var/app/lib/api/dsl/index.js:124:22
    at /var/app/node_modules/async/lib/async.js:718:13
    at async.forEachOf.async.eachOf (/var/app/node_modules/async/lib/async.js:233:13)
    at _parallel (/var/app/node_modules/async/lib/async.js:717:9)
    at Object.async.parallel (/var/app/node_modules/async/lib/async.js:731:9)
    at Dsl.test.async.parallel.remove.async.parallel [as remove] (/var/app/lib/api/dsl/index.js:118:11)
    at HotelClerk.cleanUpRooms (/var/app/lib/api/core/hotelClerk.js:454:28)
    at HotelClerk.removeRoomForCustomer (/var/app/lib/api/core/hotelClerk.js:509:23)
    at /var/app/lib/api/core/hotelClerk.js:154:29
    at /var/app/node_modules/async/lib/async.js:181:20
    at Object.async.forEachOf.async.eachOf (/var/app/node_modules/async/lib/async.js:233:13)
    at Object.async.forEach.async.each (/var/app/node_modules/async/lib/async.js:209:22)
    at HotelClerk.addSubscription.getChannels.removeSubscription.removeCustomerFromAllRooms.connection [as removeCustomerFromAllRooms] (/var/app/lib/api/core/hotelClerk.js:153:11)
    at RouterController.execute.removeConnection (/var/app/lib/api/controllers/routerController.js:125:25)
    at Lb.onDisconnect (/var/app/lib/api/core/entryPoints/lb.js:67:24)
    at /var/app/lib/services/broker/wsBrokerClient.js:53:45
scottinet commented 8 years ago

Ok I've been able to pinpoint the problem.

It can be reproduced with this much simpler filter:

{
  "term": {
    "user.id": "foobar"
  }
}

We store the field name user.id allowing users to filter on a nested attribute. And it works fine when testing documents. It doesn't work so well when trying to remove the filter though, because we're removing a filter path (detailing how to navigate in the filters object structure), where each path component is separated by a . character:

collection.index.user.id.hashedValue

And when the removal function tries to get each component to remove a client from it, we get a collection => index => user => id => value path instead of collection => index => user.id => value

This leads to scanning recursively the filters object for unknown paths, leading to undefined sub-objects, making Kuzzle crash.

scottinet commented 8 years ago

A fix has been submitted for review. If it gets validated, I'll backport it to the beta version of Kuzzle.