[SERVER-29840] arrayFilters behave incorrectly with geoNear predicates Created: 23/Jun/17  Updated: 30/Oct/23  Resolved: 25/Aug/17

Status: Closed
Project: Core Server
Component/s: Querying, Write Ops
Affects Version/s: None
Fix Version/s: 3.5.13

Type: Bug Priority: Major - P3
Reporter: David Storch Assignee: Tess Avitabile (Inactive)
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Backwards Compatibility: Fully Compatible
Operating System: ALL
Sprint: Query 2017-07-31, Query 2017-08-21, Query 2017-09-11
Participants:

 Description   

The predicate inside arrayFilters for update does not ban query language "extensions" such as text search, geospatial search, and JavaScript execution. Although top-level predicates like $where and $text are disallowed inside arrayFilters by this check, this becomes a problem for geoNear:

> db.c.drop()
true
> db.c.insert({a: [1, 4, 9]})
WriteResult({ "nInserted" : 1 })
> db.c.update({}, {$set: {"a.$[i]": 9}}, {arrayFilters: [{"i": {$near: {$geometry: {type: "Point", coordinates: [0, 0]}}}}]})
WriteResult({ "nMatched" : 1, "nUpserted" : 0, "nModified" : 1 })
> db.c.find()
{ "_id" : ObjectId("594d79c59af796e3dcf96707"), "a" : [ 9, 9, 9 ] }

The update should fail, but instead it modifies all elements in the array. This is because geoNear has a stub MatchExpression node which doesn't actually get used for matching:

https://github.com/mongodb/mongo/blob/614c7e06924d2b490c4ad4e8916b2b7db610b7da/src/mongo/db/matcher/expression_geo.cpp#L415-L420

A fix for this issue will likely involve using ExtensionsCallbackDisallowExtensions rather than ExtensionsCallbackReal while parsing the array filters. This would fix the geoNear problem, as well as improve the error message for $text and $where. We should also quickly audit the list of match expression operators to make sure that there is nothing else which needs to be banned inside arrayFilters.



 Comments   
Comment by Githook User [ 25/Aug/17 ]

Author:

{'email': 'tess.avitabile@mongodb.com', 'name': 'Tess Avitabile', 'username': 'tessavitabile'}

Message: SERVER-29840 Add allowed features bitmask to MatchExpressionParser::parse
Branch: master
https://github.com/mongodb/mongo/commit/b19f95495d1df437722e6a0c85ea5ca6f91cdd8b

Comment by Githook User [ 25/Aug/17 ]

Author:

{'email': 'tess.avitabile@mongodb.com', 'name': 'Tess Avitabile', 'username': 'tessavitabile'}

Message: SERVER-29840 Add allowed features bitmask to MatchExpressionParser::parse
Branch: master
https://github.com/10gen/mongo-enterprise-modules/commit/cb54ac809f0792bc5d607cf88e686d3e6579b18e

Comment by Tess Avitabile (Inactive) [ 17/Aug/17 ]
  1. All of those call sites use ExtensionsCallbackDisallowExtensions, so they already ban $text and $where.
  2. No, I don't think all those sites need to ban $expr. Only partial index filters need to ban $expr. And possibly document validators, if it does not work for a document validator to throw.
  3. Yes, there are two call sites which would want to ban $expr but not $geoNear/$text/$where: group command, upsert.

Given that, I think it's best to go with your original proposal.

Comment by David Storch [ 17/Aug/17 ]

I have a few questions which would need to be answered in order to figure out if we can get away with your much simpler counter-suggestion:

  1. Do all the call sites that you listed already ban $text and $where? If they don't, do any of those places actually even work correctly with $text and $where?
  2. Should all those call sites also ban $expr?
  3. Do we anticipate any call sites which would want to ban $expr but not $geoNear/$text/$where or vice versa?
Comment by Tess Avitabile (Inactive) [ 17/Aug/17 ]

david.storch, do you know if we have a use case at the moment for allowing some, but not all, extensions? I was wondering if it may be simpler to just use ExtensionsCallbackDisallowExtensions to ban $geoNear, if that is what all call sites actually want. For example,

Comment by David Storch [ 14/Aug/17 ]

tess.avitabile, the remaining work is to merge a concept of "feature banning" in the MatchExpression parser with ExtensionsCallback. I was envisioning a bit vector of banned features (kText, kJavascript, kGeoNear, etc.) which is held by the ExtensionsCallback base class. I guess ExtensionsCallbackDisallowExtensions would make all features banned, and would prevent you from configuring the bit vector in some other way. Perhaps ExtensionsCallbackReal, on the other hand, allows the caller to configure the bit vector explicitly?

Comment by Githook User [ 20/Jul/17 ]

Author:

{u'username': u'dstorch', u'name': u'David Storch', u'email': u'david.storch@10gen.com'}

Message: SERVER-29840 Merge libexpressions and libexpressions_geo.

Removes expressionParserGeoCallback.
Branch: master
https://github.com/mongodb/mongo/commit/c69b19ff673242230fecb27fff422f552b4f48d3

Comment by Githook User [ 20/Jul/17 ]

Author:

{u'username': u'dstorch', u'name': u'David Storch', u'email': u'david.storch@10gen.com'}

Message: SERVER-29840 Use ExtensionsCallbackDisallowExtensions in ArrayFilter.
Branch: master
https://github.com/mongodb/mongo/commit/032bcef34fce7a9bd9ced1652be4bfa79d1a3e6d

Generated at Thu Feb 08 04:21:57 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.