[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:
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: 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: |
| Comment by Githook User [ 25/Aug/17 ] |
|
Author: {'email': 'tess.avitabile@mongodb.com', 'name': 'Tess Avitabile', 'username': 'tessavitabile'}Message: |
| Comment by Tess Avitabile (Inactive) [ 17/Aug/17 ] |
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:
|
| 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: Removes expressionParserGeoCallback. |
| 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: |