RFR: 8356997: /etc/krb5.conf parser should not forbid include/includedir directives after sections [v2]

Valerie Peng valeriep at openjdk.org
Wed Jun 18 03:38:31 UTC 2025


On Wed, 28 May 2025 15:25:40 GMT, Weijun Wang <weijun at openjdk.org> wrote:

>> Several changes are made:
>> 
>> 1. The "include" and "includedir" directives can appear everywhere, even inside a section or a sub-section. However, it only means the content is inserted here but the included file still need its own full structure -- from section to subsections.
>> 2. The same file can be included multiple times as long as not recursively.
>> 3. Everything is merged. For duplicated values, `get` returns the first one and `getAll` returns all joining by spaces.
>> 
>> Two new tests added. I also separately confirmed that they are parsed in the same way as [MIT krb5](https://github.com/krb5/krb5/blob/master/src/util/profile/test_parse.c). MIT krb5 ignores directory name after "include" but here it's an error.
>
> 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 608:

> 606:             // Already parsed. This is allowed.
> 607:             return;
> 608:         }

Should this block of code be moved up before the line of `dups = new HashSet<>(dups);`?

src/java.security.jgss/share/classes/sun/security/krb5/Config.java line 678:

> 676:         Path fullp = Paths.get(fileName).toAbsolutePath();
> 677:         Path path = Paths.get(fileName);
> 678:         if (!Files.exists(path)) {

Is `path` needed? Can't you just use `fullp` for the `!Files.exists()` check?

src/java.security.jgss/share/classes/sun/security/krb5/Config.java line 818:

> 816:      *
> 817:      * @param entry path to config file, could be an included one
> 818:      * @returns the parsed configuration

This method is changed to return void, why add this `@returns` ?

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/25421#discussion_r2153443893
PR Review Comment: https://git.openjdk.org/jdk/pull/25421#discussion_r2153220437
PR Review Comment: https://git.openjdk.org/jdk/pull/25421#discussion_r2153115117


More information about the security-dev mailing list