On Thu, 17 Jun 2021 17:21:04 GMT, Weijun Wang <wei...@openjdk.org> wrote:

>> More loudly and precise warning messages when a security manager is either 
>> enabled at startup or installed at runtime.
>> 
>> This is new PR for the `openjdk/jdk17` repo copied from 
>> https://github.com/openjdk/jdk/pull/4400. A new commit is added.
>
> Weijun Wang has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   verbose warning message test and renaming in System.java

src/java.base/share/classes/java/lang/System.java line 381:

> 379:         if (allowSecurityManager()) {
> 380:             var caller = Reflection.getCallerClass();
> 381:             String signature = caller.getName() + " (" + 
> codeSource(caller) + ")";

Hello Weijun,

Given that the `codeSource()` method above is allowed to return `null`, there 
could be a case where the warning message printed would be something like:

> 
> WARNING: A terminally deprecated method in java.lang.System has been called
> WARNING: System::setSecurityManager has been called by foo.bar.Hello (null)
> WARNING: Please consider reporting this to the maintainers of foo.bar.Hello
> WARNING: System::setSecurityManager will be removed in a future release
> 

Would that be OK or do you think the presence of "(null)" be unnecessary and 
confusing? Maybe in such cases that line should just say 
"System::setSecurityManager has been called by foo.bar.Hello"?

Another minor nit - the variable is named `signature` which initially gave me 
an impression that it was the method signature of the caller code, but that 
isn't the case. Should this variable be renamed perhaps?

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

PR: https://git.openjdk.java.net/jdk17/pull/13

Reply via email to