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