[SERVER-27201] $graphLookup triggers null pointer dereference Created: 29/Nov/16 Updated: 05/Apr/17 Resolved: 30/Nov/16 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | Aggregation Framework |
| Affects Version/s: | 3.4.0-rc3 |
| Fix Version/s: | 3.4.1, 3.5.1 |
| Type: | Bug | Priority: | Major - P3 |
| Reporter: | Salinca Andreea | Assignee: | David Storch |
| Resolution: | Done | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||||||||||||||||||||||||||||||||||||||||||
| Backwards Compatibility: | Fully Compatible | ||||||||||||||||||||||||||||||||||||||||||||||||
| Operating System: | ALL | ||||||||||||||||||||||||||||||||||||||||||||||||
| Backport Completed: | |||||||||||||||||||||||||||||||||||||||||||||||||
| Steps To Reproduce: | Run the above query and mondb crashes with:
The log file from "var/log/mongodb/mongod.log" contains:
|
||||||||||||||||||||||||||||||||||||||||||||||||
| Sprint: | Query 2016-12-12 | ||||||||||||||||||||||||||||||||||||||||||||||||
| Participants: | |||||||||||||||||||||||||||||||||||||||||||||||||
| Description |
|
Have a collection "teststructure" with the following documents:
Have a collection "objects" with the following documents that a reference field "ts" to a document from "teststructure" collection:
The following query (which should retrive recursivelly the desks that are assigned in leaf nodes - rooms) crashes mongodb service:
Run the above query and mongodb crashes. More details (about logs) in steps to reproduce. The problem seems to be related with the usage of the conditional operator in startWith expression from $graphLookup. |
| Comments |
| Comment by Githook User [ 30/Nov/16 ] | ||||||||||||||||||||||||||||||||||||
|
Author: {u'username': u'dstorch', u'name': u'David Storch', u'email': u'david.storch@10gen.com'}Message: (cherry picked from commit 7411de92863b81fdf0f7c69aba8178a070633991) | ||||||||||||||||||||||||||||||||||||
| Comment by Githook User [ 30/Nov/16 ] | ||||||||||||||||||||||||||||||||||||
|
Author: {u'username': u'dstorch', u'name': u'David Storch', u'email': u'david.storch@10gen.com'}Message: | ||||||||||||||||||||||||||||||||||||
| Comment by Salinca Andreea [ 30/Nov/16 ] | ||||||||||||||||||||||||||||||||||||
|
@Ramon Fernandez, I am happy that I could contribute to MongoDb 3.4 and that I got into 3.4 Bug Hunt winners. Thanks, | ||||||||||||||||||||||||||||||||||||
| Comment by Charlie Swanson [ 29/Nov/16 ] | ||||||||||||||||||||||||||||||||||||
|
It looks like the $graphLookup stage just forgets to forward the new ExpressionContext to the startWith expression. This should be done as part of DocumentSourceGraphLookup::doInjectExpressionContext(). This patch fixed it locally:
I was originally surprised that the fuzzer didn't catch this, but I suspect this was difficult to find because not many expression actually use the ExpressionContext, only those that might need to check what the active collation is such as $eq, $gt, etc. Further, it would need to involve such an expression which does not resolve to an ExpressionConstant. Usually, the only way to do this is to involve a field path like "$path._id" in the comparison somehow, but in this case we hit | ||||||||||||||||||||||||||||||||||||
| Comment by Ramon Fernandez Marina [ 29/Nov/16 ] | ||||||||||||||||||||||||||||||||||||
|
asalinca, this is to let you know that this bug report has made it into the 3.4 Bug Hunt winners. Our Community Team will be in touch with you soon. Thanks for helping us make MongoDB 3.4 better. Regards, | ||||||||||||||||||||||||||||||||||||
| Comment by Marko Vojvodic [ 29/Nov/16 ] | ||||||||||||||||||||||||||||||||||||
|
SERVER-25535 should fix this issue. I tested the reduced repro script on the development branch and the command executes successfully. | ||||||||||||||||||||||||||||||||||||
| Comment by Charlie Swanson [ 29/Nov/16 ] | ||||||||||||||||||||||||||||||||||||
|
I just wanted to point out that the startWith clause isn't actually being parsed as a conditional. I believe it was meant to be wrapped in a $cond. As-is, it's being interpreted as an object literal, so the graph search will start with the query
Instead of the (what I think is) expected
| ||||||||||||||||||||||||||||||||||||
| Comment by Salinca Andreea [ 29/Nov/16 ] | ||||||||||||||||||||||||||||||||||||
|
Thanks for the quick reply, I am glad to help you. Regards, | ||||||||||||||||||||||||||||||||||||
| Comment by Ramon Fernandez Marina [ 29/Nov/16 ] | ||||||||||||||||||||||||||||||||||||
|
Thanks for the detailed report asalinca, we're able to reproduce the behavior you describe and are investigating. Regards, | ||||||||||||||||||||||||||||||||||||
| Comment by Max Hirschhorn [ 29/Nov/16 ] | ||||||||||||||||||||||||||||||||||||
|
It looks like the ExpressionContext used to perform the $graphLookup is nullptr. Here's the symbolized output from a debug build:
|