[crac] RFR: 8348869: [CRaC] Restore args are split by whitespaces incorrectly [v2]
Radim Vansa
rvansa at openjdk.org
Mon Feb 3 07:52:10 UTC 2025
On Thu, 30 Jan 2025 15:50:29 GMT, Timofei Pushkin <tpushkin at openjdk.org> wrote:
>> src/java.base/share/classes/jdk/internal/crac/mirror/Core.java line 252:
>>
>>> 250: curArgBuilder.setLength(0);
>>> 251: }
>>> 252: case escChar -> curArgBuilder.append(newArguments.charAt(++i));
>>
>> This version looks much better, but I wonder what happens here if the escape is the last char - this code looks like it would throw OOBE (as there's no check after incrementation). It seems that situation is covered in the test case as well - I don't understand why it won't fail.
>
> It does not fail because this code receives not the original arguments string but the escaped one constructed [here](https://github.com/openjdk/crac/pull/193/files/cadaa8bb86404d883885324b6a95be2105e0c1fb#diff-c3caaf3e347b1a477e2b7278cb6a35da04a597de5632c13351a6e4e5a2924d21R1869). For example, if the original string was "\" it will be "\\\" here, the code will parse the first slash and skip the second one. And since the code constructing the escaped string is ours, unless there is will be a bug there, the escaped string should never end with a single slash.
>
> I thought about this too and considered placing an assert here (or even an explicit check) but since the single-slash situation should happen only if there is a bug in our code I decided that the default OOBE will be enough for us to identify the issue.
Thanks for the explanation, I think it is fine then.
-------------
PR Review Comment: https://git.openjdk.org/crac/pull/193#discussion_r1938922440
More information about the crac-dev
mailing list