On Sun, 17 Apr 2022 03:49:19 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
>> Alan Bateman has updated the pull request with a new target base due to a
>> merge or a rebase. The incremental webrev excludes the unrelated changes
>> brought in by the merge/rebase. The pull request contains five additional
>> commits since the last revision:
>>
>> - Refresh
>> - Refresh
>> - Merge with jdk-19+18
>> - Refresh
>> - Initial push
>
> src/java.base/share/classes/java/io/PrintStream.java line 1420:
>
>> 1418: } else {
>> 1419: synchronized (this) {
>> 1420: implFormat(format, args);
>
> I think there's a typo here. I think it should have been:
>
>
> implFormat(l, format, args);
Well spotted, the locale should be provided.
> src/java.base/share/classes/java/lang/Thread.java line 602:
>
>> 600: } else {
>> 601: // getContextClassLoader not trusted
>> 602: ClassLoader cl = parent.contextClassLoader;
>
> I might be misreading the comment on that line, but reading this if/else
> block a few times, I'm unsure what the expectations here are.
>
> It's my understanding that a call to `getContextClassLoader()` can't be
> trusted if that method has been overridden by the passed `Thread` type. In
> such cases, we don't call that method and instead just use the field value of
> `contextClassLoader` (which is a private field on `Thread`). Is that
> understanding correct? If yes, then the condition in the `if` block a few
> lines above, looks odd. It seems to be calling the `getContextClassLoader()`
> if that method is overridden by the passed `Thread` type, i.e. the untrusted
> case. Should it instead be:
>
> if (sm == null || !isCCLOverridden(parent.getClass())) {
> return parent.getContextClassLoader();
> }
>
> (notice the negation)
This code hasn't changed, it's just moved to a helper method. When running with
a security manager and the CCL methods aren't overridden, then it avoids the
permission check. However, I can see how the comment is mis-reading so we can
improve that.
-------------
PR: https://git.openjdk.java.net/jdk/pull/8166
