[CDRIVER-2006] mongoc_gridfs_find_one_with_opts(): the error parameter is not initialized on failure Created: 29/Jan/17 Updated: 03/May/17 Resolved: 28/Mar/17 |
|
| Status: | Closed |
| Project: | C Driver |
| Component/s: | GridFS |
| Affects Version/s: | 1.5.3 |
| Fix Version/s: | 1.7.0 |
| Type: | Bug | Priority: | Major - P3 |
| Reporter: | Arseny Vakhrushev | Assignee: | A. Jesse Jiryu Davis |
| Resolution: | Done | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Attachments: |
|
||||||||||||
| Issue Links: |
|
||||||||||||
| Description |
|
And yet again we have a situation when despite a failed status, the error parameter is not initialized. Now starring mongoc_gridfs_find_one_with_opts(). I am maybe paranoid, but this lack of consistency is pretty serious. Adding to that, these types of errors can lead to anything from innocent garbage spills to breach in security to sporadic crashes, etc. As a counter-measure, I'd suggest users explicitly initialize error containers with something like BSON_ERROR_INITIALIZER. It won't solve the inconsistency reporting problem, but at least it will prevent adverse consequences of uninitialized values. The example:
The output:
Valgrind:
|
| Comments |
| Comment by Githook User [ 28/Mar/17 ] |
|
Author: {u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}Message: |
| Comment by Arseny Vakhrushev [ 30/Jan/17 ] |
|
I just thought... Is it really an error that a file is not found? It's much like a successful db.findOne() that returns no results, right? Or, if put another way, it's an empty mongoc_gridfs_file_list_t returned by mongoc_gridfs_find_with_opts(). In this case, it's definitely not an error because mongoc_gridfs_file_list_error() will return false for an empty list which is expected behaviour. So, it is probably much better to just set error.domain = error.code = 0 in that case and explicitly mention in the manual that the user must check against error.code to determine if there's a real error. I am pretty sure I saw a similar situation somewhere in mongoc (can't remember where exactly now) when a pointer to something is returned. And when it's NULL, it's not necessarily an error condition. You are probably aware better than me though. |
| Comment by Arseny Vakhrushev [ 29/Jan/17 ] |
|
Right on, Jesse. It also might be a good idea to check all invocations of mongoc_cursor_error() throughout the code for similar concerns. |
| Comment by A. Jesse Jiryu Davis [ 29/Jan/17 ] |
|
Thanks Arseny. Here, we should check mongoc_cursor_error(list->cursor, &list->error). If that's false, we should set list->error to a new error code, MONGOC_ERROR_GRIDFS_FILE_NOT_FOUND. |