PHPC-920: Regression test for mongoc_cursor_destroy() crash #557
Conversation
| $query = new MongoDB\Driver\Query([], ['projection' => ['' => 1]]); | ||
|
|
||
| echo throws(function() use ($manager, $query) { | ||
| $cursor = $manager->executeQuery(NS, $query); |
There was a problem hiding this comment.
With libmongoc 1.5.4, the segfault occurs here when the cursor is checked for an error, an exception is thrown, and the cursor is destroyed. In 1.5.5, the segfault is fixed but we still throw the exception here.
I think it might make more sense to do the same BSON validation that libmongoc does during cursor initialization in our own Query constructor, so we can instead throw an InvalidArgumentException earlier. This duplicates the validation logic, but the filter and options documents should be small enough where this isn't a concern. If this sound reasonable, I'll open a new ticket to address this for 1.3.0.
There was a problem hiding this comment.
It depends a little bit how much code it involves. I'm generally sceptical about doing these in two places. In general, I do think it's better to have an InvalidArgumentException here.
There was a problem hiding this comment.
To clarify, we are currently limited to a RuntimeException when executeQuery() is called, because _mongoc_cursor_new_with_opts() reports an error with the MONGOC_ERROR_CURSOR domain and MONGOC_ERROR_CURSOR_INVALID_CURSOR code. I wasn't sure if you were suggesting an InvalidArgumentException at this very point (i.e. executeQuery()), but that's not possible.
Throwing an InvalidArgumentException at Query construct time should require less than ten lines of code. I'll create a ticket and submit a PR soon.
| $query = new MongoDB\Driver\Query([], ['projection' => ['' => 1]]); | ||
|
|
||
| echo throws(function() use ($manager, $query) { | ||
| $cursor = $manager->executeQuery(NS, $query); |
There was a problem hiding this comment.
It depends a little bit how much code it involves. I'm generally sceptical about doing these in two places. In general, I do think it's better to have an InvalidArgumentException here.
https://jira.mongodb.org/browse/PHPC-920