[SERVER-7010] ObjectId(ObjectId("4e859938e4b0c81c1411b0c7")) triggers stackdump Created: 11/Sep/12 Updated: 11/Jul/16 Resolved: 11/Sep/12 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | None |
| Affects Version/s: | 2.2.0 |
| Fix Version/s: | 2.3.0 |
| Type: | Bug | Priority: | Major - P3 |
| Reporter: | Jean-Marie Gaillourdet | Assignee: | Tad Marshall |
| Resolution: | Done | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Environment: |
OS X as well as Linux using the mongo shell |
||
| Issue Links: |
|
||||||||
| Backwards Compatibility: | Fully Compatible | ||||||||
| Operating System: | ALL | ||||||||
| Participants: | |||||||||
| Description |
|
tomcat@luna334:~$ mongo --port 27019 While the expression might be nonsensical it should definitely lead to a proper runtime error not to a stacktrace. |
| Comments |
| Comment by auto [ 11/Sep/12 ] | |||||||||||||||||||||||
|
Author: {u'date': u'2012-09-11T07:16:13-07:00', u'email': u'tad@10gen.com', u'name': u'Tad Marshall'}Message: Change two uses of massert() to uassert() in | |||||||||||||||||||||||
| Comment by Tad Marshall [ 11/Sep/12 ] | |||||||||||||||||||||||
|
I checked engine.cpp and engine_spidermonkey.cpp for other masserts and the only additional ones I found were for actual failures, not validation, so they are correct. | |||||||||||||||||||||||
| Comment by Tad Marshall [ 11/Sep/12 ] | |||||||||||||||||||||||
|
It looks like anything unexpected passed as an argument to ObjectId() generates a stack trace.
Version 2.0.7 does the same thing. In src/mongo/scripting/engine.cpp at line 161 (in today's master code), the Scope::validateObjectIdString() routine uses an massert test that the string length is exactly 24, and then does another massert on line 170 if a non-hex character is found. Because this is our first test of user-supplied values, these should be uassert's (User Assertions) and not massert's (Message Assertions). This would prevent the stack trace. We might want to audit this code to see if other validation tests of user input use massert when uassert would be a better choice. |