[crac] RFR: 8348869: [CRaC] Restore args are split by whitespaces incorrectly [v2]
Timofei Pushkin
tpushkin at openjdk.org
Thu Jan 30 15:54:13 UTC 2025
On Thu, 30 Jan 2025 14:56:16 GMT, Radim Vansa <rvansa at openjdk.org> wrote:
>> Timofei Pushkin has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Use StringBuffer to parse new restore args
>
> 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.
-------------
PR Review Comment: https://git.openjdk.org/crac/pull/193#discussion_r1935852492
More information about the crac-dev
mailing list