[SERVER-5796] HexData undocumented, crashy Created: 09/May/12 Updated: 11/Jul/16 Resolved: 01/Jun/12 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | Shell, Usability |
| Affects Version/s: | 2.1.1 |
| Fix Version/s: | 2.1.2 |
| Type: | Bug | Priority: | Minor - P4 |
| Reporter: | A. Jesse Jiryu Davis | Assignee: | Tad Marshall |
| 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 | ||||||||
| Participants: | |||||||||
| Description |
|
'help misc' doesn't document how to format a hex string for the HexData constructor on the shell. Two attempts I made resulted in an assert and a crash, respectively:
|
| Comments |
| Comment by Tad Marshall [ 01/Jun/12 ] | ||
|
Added minimal added help to the shell's "help misc", added more user friendly validation and error message display when hex data does not match requirements, prevent segfaults following failed JavaScript commands. | ||
| Comment by auto [ 01/Jun/12 ] | ||
|
Author: {u'login': u'tadmarshall', u'name': u'Tad Marshall', u'email': u'tad@10gen.com'}Message: Add one line of explanation of what "hexstr" is supposed | ||
| Comment by Tad Marshall [ 22/May/12 ] | ||
|
The "crashy" part of this will be fixed by fixing | ||
| Comment by A. Jesse Jiryu Davis [ 10/May/12 ] | ||
|
Right, both. Turns out this is a weird bug. It's specifically the sequence of:
.... that crashes the shell. Both 'var' and 'new' keywords are needed for the crash. '64' is not special, other numbers will crash it. However, I think the string format passed to HexData should be included in the 'help misc' output, specifically that '0x' should not be part of the string. Also, instead of a C++ traceback, it might just say, "bad format." | ||
| Comment by Tad Marshall [ 10/May/12 ] | ||
|
The assertion happens because the _HexData() routine in scripting/engine_spidermonkey.cpp (lines starting at 958 in today's code) does no pre-testing of the string and passes it directly to a helper function that asserts (verify in today's code) on any character other than 0-9 and a-f(A-F). The 'x' triggers the failure. If you pass a number of arguments other than two, you get a friendly error message. Pass a non-hex character and you get the stack trace. Testing on Windows, the second example works, though the variable 'b' is displayed as BinData(0,"ZA==") and not as HexData as I had expected. I'd vote for a friendly error message on invalid input (e.g. "Error: 'x' is not a hexadecimal digit") and no assertion or stack trace. I'd vote for no segfaults under any circumstances. The documentation should state that the second parameter should be a string containing a sequence of hexadecimal digits that is even in length. | ||
| Comment by Ian Whalen (Inactive) [ 09/May/12 ] | ||
|
were you suggesting we need more docs and to handle the failure state On Wed, May 9, 2012 at 12:09 PM, A. Jesse Jiryu Davis (JIRA) < – |