Fix parser bug for empty string allocation #496
Conversation
When `HAVE_RB_ENC_INTERNED_STR` is enabled it is possible to pass through a null pointer to `rb_enc_interned_str` resulting in a segfault Fixes ruby#495
| @@ -2363,9 +2363,17 @@ static VALUE json_string_unescape(char *string, char *stringEnd, int intern, int | |||
| char buf[4]; | |||
|
|
|||
| if (bufferSize > MAX_STACK_BUFFER_SIZE) { | |||
| # ifdef HAVE_RB_ENC_INTERNED_STR | |||
There was a problem hiding this comment.
note, i've wrapped the fix with this HAVE_RB_ENC_INTERNED_STR guard as the issue only pertains to the call to rb_enc_interned_str below. Saves allocating the extra byte unnecessarily. Although, if there was concern about the extra allocation this would likely look like:
bufferStart = buffer = ALLOC_N(char, !intern || bufferSize ? bufferSize : 1);
Or would you prefer for simplicities sake to just go with the suggestion in #475 from @jeremyevans (ie without the HAVE_RB_ENC_INTERNED_STR guards)?
Of course the safest solution would be to simply allocate room for the null terminator all of the time (bufferSize + 1) but that's likely unnecessary given how the pointers are being used.
| @@ -84,6 +84,7 @@ def test_parse_simple_objects | |||
| assert_equal({ "a" => 23 }, parse(' { "a" : 23 } ')) | |||
| assert_equal({ "a" => 0.23 }, parse(' { "a" : 0.23 } ')) | |||
| assert_equal({ "a" => 0.23 }, parse(' { "a" : 0.23 } ')) | |||
| assert_equal({ "" => 123 }, parse('{"":123}')) | |||
There was a problem hiding this comment.
Technically this test is a duplicate given https://github.com/flori/json/blob/master/tests/fixtures/pass1.json#L15 however it seems reasonable to have an explicit test for this case here too.
|
confirming this fixes the issue... |
|
Hey @hsbt, any chance this could be reviewed and released if you're happy with it? Thanks in advance! |
|
+1 for releasing the fix. |
When
HAVE_RB_ENC_INTERNED_STRis enabled it is possible to pass through a null pointer torb_enc_interned_strresulting in a segfault.Fixes #495
Kudos to @jeremyevans for identifying the fix