[rfc][icedtea-web] following permissions attribute

[Andrew Azores]

On 03/10/2014 02:42 PM, Jiri Vanek wrote:
> All should be fixed. Thanx!
>
> There are some deeper changes caused by moving from Boolean->enum.
>
> Also I found few crippled test by (already previous version of ) this 
> patch. So those are fixed
>
> Thanx,
>   J.
>

Just a few fairly minor nits left.

> +        public String permissionsToString() {
> +            String s = getAttribute(PERMISSIONS);
> +            if (s == null) {
> +                return "Not defined";
> +            } else if (s.trim().equalsIgnoreCase("sandbox")) {
> +                return s.trim();
> +            } else if (s.trim().equalsIgnoreCase("all-permissions")) {
> +                return s.trim();
> +            } else {
> +                return "illegal";
> +            }
> +
> +
> +        }

Kill the extra whitespace at the end of the method please.

> +        if (permissions == ManifestBoolean.UNDEFINED) {
> +            if (level == AppletSecurityLevel.DENY_UNSIGNED) {
> +                throw new LaunchException("Your Extended applets security is at 'Very high', and this application is missing the 'permissions' attribute in manifest. This is fatal");
> +            }
> +            if (level == AppletSecurityLevel.ASK_UNSIGNED) {
> +                boolean  a = SecurityDialogs.showMissingPermissionsAttributeDialogue(file.getTitle(), file.getCodeBase());
> +                if (!a) {
> +                    throw new LaunchException("Your Extended applets security is at 'high' and  this applicationis missing the 'permissions' attribute in manifest. And you have refused to run it.");
> +                }
> +            }
> +            //default for missing is sandbox
> +            if (!SecurityDesc.SANDBOX_PERMISSIONS.equals(security.getSecurityType())) {
> +                throw new LaunchException("The 'permissions' attribute is not specified, and application is requesting permissions. This is fatal");
> +            }
> +        } else {
> +            if (permissions == ManifestBoolean.TRUE) {
> +                if (SecurityDesc.SANDBOX_PERMISSIONS.equals(security.getSecurityType())) {
> +                    OutputController.getLogger().log("The permissions attribute of this application is " + file.getManifestsAttributes().permissionsToString() + "' and security is '" + security.getSecurityType() + "'. Thats correct");
> +                } else {
> +                    throw new LaunchException("The 'permissions' attribute is '" + file.getManifestsAttributes().permissionsToString() + "' but  security is '" + security.getSecurityType() + "'. This is fatal");
> +                }
> +            } else {
> +                if (SecurityDesc.SANDBOX_PERMISSIONS.equals(security.getSecurityType())) {
> +                    throw new LaunchException("The 'permissions' attribute is '" + file.getManifestsAttributes().permissionsToString() + "' but  security is' " + security.getSecurityType() + "'. This is fatal");
> +                } else {
> +                    OutputController.getLogger().log("The permissions attribute of this application is '" + file.getManifestsAttributes().permissionsToString() + "' and security is '" + security.getSecurityType() + "'. Thats correct");
> +                }
> +            }
> +        }

Rather than:

if (UNDEFINED) {
} else {
     if (TRUE) {
     } else { // implicit FALSE
     }
}

could you change this to if TRUE/else if FALSE/else if UNDEFINED/else? 
Or a switch?

> +     public static boolean  showMissingPermissionsAttributeDialogue(String title, URL codeBase) {

You have two spaces after 'boolean' ;)

> +         // result 0 = Yes, 1 = No
> +         if (selectedValue instanceof Integer) {
> +             // If the selected value can be cast to Integer, use that value
> +             int i = ((Integer) selectedValue).intValue();
> +             if (i == 0) {
> +                 return true;
> +             } else {
> +                 return false;
> +             }
> +         } else {
> +             // Otherwise default to "cancel"
> +             return false;
> +         }

SecurityDialogs.getIntegerResponseAsBoolean(Object) already does this 
for you.

> diff -r 483ab446ea4c tests/netx/unit/net/sourceforge/jnlp/runtime/CodeBaseClassLoaderTest.java
> --- a/tests/netx/unit/net/sourceforge/jnlp/runtime/CodeBaseClassLoaderTest.java	Mon Mar 10 12:29:47 2014 -0400
> +++ b/tests/netx/unit/net/sourceforge/jnlp/runtime/CodeBaseClassLoaderTest.java	Mon Mar 10 19:38:27 2014 +0100
> @@ -53,12 +53,29 @@
>   import net.sourceforge.jnlp.runtime.JNLPClassLoader.CodeBaseClassLoader;
>   import net.sourceforge.jnlp.annotations.Bug;
>   import net.sourceforge.jnlp.annotations.Remote;
> +import net.sourceforge.jnlp.config.DeploymentConfiguration;
> +import net.sourceforge.jnlp.security.appletextendedsecurity.AppletSecurityLevel;
> +import net.sourceforge.jnlp.security.appletextendedsecurity.AppletStartupSecuritySettings;
> +import net.sourceforge.jnlp.util.logging.NoStdOutErrTest;
>   import org.junit.AfterClass;
>   import org.junit.Assert;
> +import org.junit.BeforeClass;
>   
>   import org.junit.Test;
>   
> -public class CodeBaseClassLoaderTest {
> +public class CodeBaseClassLoaderTest extends NoStdOutErrTest {
> +
> +    private static AppletSecurityLevel level;
> +
> +    @BeforeClass
> +    public static void setPermissions(){
> +     level = AppletStartupSecuritySettings.getInstance().getSecurityLevel();
> +     JNLPRuntime.getConfiguration().setProperty(DeploymentConfiguration.KEY_SECURITY_LEVEL, AppletSecurityLevel.ALLOW_UNSIGNED.toChars());
> +    }
> +    @AfterClass
> +    public static void resetPermissions(){
> +    JNLPRuntime.getConfiguration().setProperty(DeploymentConfiguration.KEY_SECURITY_LEVEL, level.toChars());
> +    }

Fix indentation please, also add spaces between () and {.

>   public class JNLPFileTest extends NoStdOutErrTest {
>   
> +
> +    private static AppletSecurityLevel level;
> +
> +    @BeforeClass
> +    public static void setPermissions(){
> +     level = AppletStartupSecuritySettings.getInstance().getSecurityLevel();
> +     JNLPRuntime.getConfiguration().setProperty(DeploymentConfiguration.KEY_SECURITY_LEVEL, AppletSecurityLevel.ALLOW_UNSIGNED.toChars());
> +    }
> +    @AfterClass
> +    public static void resetPermissions(){
> +    JNLPRuntime.getConfiguration().setProperty(DeploymentConfiguration.KEY_SECURITY_LEVEL, level.toChars());
> +    }

Same.

Thanks,

-- 
Andrew A