[rfc][icedtea-web] Remove bad debug logging statements from JNLPSecurityManager

[Jiri Vanek]

On 11/04/2013 04:22 PM, Andrew Azores wrote:
> Hi,
>
> Calling cl.getPermissions(null) will potentially/usually result in an NPE being thrown and caught/printed within getPermissions, then rethrown again. This is not very helpful for producing debugging output. I don't see any easy way to do what this was intended to do so it's just removed.
>


hmm, cl.getPermission will lead to npe always, when security do exists:

   protected PermissionCollection getPermissions(CodeSource cs) {
         try {
             Permissions result = new Permissions();
             if (security != null) {
                 PermissionCollection permissions = security.getSandBoxPermissions();

                 if (cs == null) {
                     throw new NullPointerException("Code source was null");}
}catch.. just rethrow...
}
So I would say, that instead of removing the messages, somethinkg like:

 >            if (JNLPRuntime.isDebug()) {
if (!cl.isSecurityNull()}{
     OutputController.getLogger().log(OutputController.Level.ERROR_ALL, "Trying to get permission  for null  source to environment with security, its wrong ");
}else { //conntinue as before
 >                if (cl.getPermissions(null).implies(perm)){
}

Maybe you can check another NPE which can occur, but it should be not necessary.

Also I have suspicions that this can hide more serious bug:
   - why passing null to getPemrmission
   - why if (cs == null) {                    throw new NullPointerException("Code source was null");                } if security is
   - and esepcially  why  // Class from host X should be allowed to connect to host X
             if (cs.getLocation() != null && cs.getLocation().getHost().length() > 0)
                 result.add(new SocketPermission(cs.getLocation().getHost(),
                         "connect, accept"));
   when is not?

Thanx for tickling on it.

  J.

> ChangeLog:
> * netx/net/sourceforge/jnlp/runtime/JNLPSecurityManager.java: (addPermission) removed debug print statements
>
> Thanks,
>
> --
> Andrew A
>
>
> JNLPSecurityManager-bad-debug.patch
>
>
> diff --git a/netx/net/sourceforge/jnlp/runtime/JNLPSecurityManager.java b/netx/net/sourceforge/jnlp/runtime/JNLPSecurityManager.java
> --- a/netx/net/sourceforge/jnlp/runtime/JNLPSecurityManager.java
> +++ b/netx/net/sourceforge/jnlp/runtime/JNLPSecurityManager.java
> @@ -313,16 +313,8 @@
>        */
>       private void addPermission(Permission perm) {
>           if (JNLPRuntime.getApplication().getClassLoader() instanceof JNLPClassLoader) {
> -
>               JNLPClassLoader cl = (JNLPClassLoader) JNLPRuntime.getApplication().getClassLoader();
>               cl.addPermission(perm);
> -            if (JNLPRuntime.isDebug()) {
> -                if (cl.getPermissions(null).implies(perm)){
> -                    OutputController.getLogger().log(OutputController.Level.ERROR_ALL, "Added permission: " + perm.toString());
> -                } else {
> -                    OutputController.getLogger().log(OutputController.Level.ERROR_ALL, "Unable to add permission: " + perm.toString());
> -                }
> -            }
>           } else {
>               OutputController.getLogger().log(OutputController.Level.ERROR_DEBUG, "Unable to add permission: " + perm + ", classloader not JNLP.");
>           }