On Wed, 28 May 2025 11:18:21 GMT, Mikhail Yankelevich
<myankelev...@openjdk.org> wrote:
>> Weijun Wang has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> more random testing
>
> src/java.security.jgss/share/classes/sun/security/krb5/Config.java line 774:
>
>> 772: result.add(previous);
>> 773: unwritten.forEach(result::add);
>> 774: unwritten.clear();
>
> I don't think this code is covered by the tests at all.
> I have found 2 simple ways to test it:
> 1. change the line 62-66 in IncludeDup from
> ```java
> for (var inc : List.of("outside", "beginsec", "insec", "insec2",
> "insubsec", "endsubsec", "endsec")) {
> Files.writeString(Path.of(inc), String.format("""
> [a]
> b = {
> c = %s
> }
> """, inc));
> }
>
> to
> ```
> for (var inc : List.of("outside", "beginsec", "insec", "insec2",
> "insubsec", "endsubsec", "endsec")) {
> Files.writeString(Path.of(inc), String.format("""
> [a]
> b =
> { c = %s
> }
> """, inc));
> }
>
> 2. change `krb5.conf` EXAMPLE_3.COM from
> ``` java
>
> EXAMPLE_3.COM = {
> kdc = kdc.example.com
> kdc = kdc2.example.com
> inner =
> {
> aaa = nnn
> }
> }
>
> to
> ```java
>
> EXAMPLE_3.COM = {
> kdc = kdc.example.com
> kdc = kdc2.example.com
> inner =
> { aaa = nnn
> }
> }
> ```
>
> There are other ways to cover this as well as writing it's own test case,
> however I feel that it might be an overkill for this.
> What do you think?
Good catch. Instead I've enhanced the random test to cover this.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25421#discussion_r2112188183
