Fix integer overflow in buffer length calculation for unpack('m') and unpack('u')#8936
Conversation
ikaronen-relex
force-pushed
the
unpack-buffer-size-overflow
branch
4 times, most recently
from
dc0600e to
b511c28
Compare
… unpack('u')
This fixes jruby#8933
ikaronen-relex
force-pushed
the
unpack-buffer-size-overflow
branch
from
b511c28 to
42e8119
Compare
|
@ikaronen-relex This is still marked WIP and it targets master which is JRuby 10.x. If you are not finished then please consider retargeting to jruby-9.4 (and we/I will merge it to 10 as well). If you are finished let me know and I will merge. I can also cherry-pick this back so just let me know when you are done. |
|
@enebo I marked it as draft since I was getting some test failures due to OOM errors. Looks like the tests pass now, so I've marked it as ready. In the process, I also fixed what appears to be an unnecessary array copy in Pack.unpackBase46Strict. That didn't fix the OOMs (but apparently splitting the specs into several smaller ones did), but I left it in since I think it's still a reasonable (and safe, AFAICT!) optimization. |
| } | ||
| } | ||
| return newString(context, new ByteList(out, 0, index)); | ||
| return newString(context, new ByteList(out, 0, index, false)); |
There was a problem hiding this comment.
Passing false here avoids an unnecessary array copy in the ByteList constructor. The out array isn't otherwise exposed outside this method, so avoiding the copy seems safe to me.
ikaronen-relex
force-pushed
the
unpack-buffer-size-overflow
branch
from
0edc0fb to
192993d
Compare
|
@ikaronen-relex Yeah I noticed you caught the extra arraycopy. Good find. |
2 participants
This fixes #8933