[rfc][icedtea-web] following permissions attribute

[Andrew Azores]

On 03/05/2014 09:22 AM, Jiri Vanek wrote:
> On 03/04/2014 11:22 PM, Andrew Azores wrote:
>> On 02/25/2014 12:16 PM, Jiri Vanek wrote:
>>> AIS
>>>
>>> http://docs.oracle.com/javase/7/docs/technotes/guides/jweb/security/manifest.html#permissions 
>>>
>>>
>>> Please, any reviewer, read the specification. It appeared to be 
>>> tricky to transform its spoken language to algorithm... Well at 
>>> least default was specified without fog :(
>>>
>>> Thanx for any comments!
>>> J.
>>>
>>> not sure how with unittests, but reproducers are on the way...
>>
>> Are you just looking for additional help figuring out what the spec 
>> is trying to define? Or was a patch forgotten here?
>>
>
> Crap. Sorry. Forgot to attach aptch :(

Messages.properties:
> +# missing permissions dialogue
> +MissingPermissionsMainTitle=Application <span color='red'> {0} </span> \
> +form codebase <span color='red'> {1} </span> is missing the 
> permissions attribute. \
> +Applications without this attribute should not be trusted. Do you 
> allow this application to run?
"Do you want to run this application?" or "Do you wish to allow this 
application to run?"
> +MissingPermissionsInfo=For more information you can visit:<br/>\
> +<a 
> href="http://docs.oracle.com/javase/7/docs/technotes/guides/jweb/security/manifest.html#permissions"> 
> \
> +http://docs.oracle.com/javase/7/docs/technotes/guides/jweb/security/manifest.html#permissions</a> 
> <br/> \
> +and<br/> <a 
> href="http://docs.oracle.com/javase/7/docs/technotes/guides/jweb/security/no_redeploy.html"> 
> \
> +http://docs.oracle.com/javase/7/docs/technotes/guides/jweb/security/no_redeploy.html</a>


JNLPClassLoader:
> +        Boolean permissions = 
> file.getManifestsAttributes().isSandboxForced();

I'm not liking the use of the boxed type here... true/false/null is kind 
of ugly. Can an enum be made for the three states, rather than actually 
using null as an acceptable value? I know this isn't directly relevant 
to this patch, though.

> +                throw new LaunchException("Your Extended applets 
> security is at 'Very high', and this applicationis missing the 
> 'permissions' attribute in manifest. This is fatal");
> +                    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.");

"applicationis" - missing a space. Should this go in Messages.properties?

And please consider moving this method into the new SecurityDelegate as 
well (which is newer than this patch, I know)



SecurityDialogs:
> +     public static int showMissingPermissionsAttributeDialogue(String 
> title, URL codeBase) {
> +
> +        if (!shouldPromptUser()) {
> +            return 1;
> +        }
> +
> +        SecurityDialogMessage message = new SecurityDialogMessage();
> +        message.dialogType = 
> DialogType.UNSIGNED_EAS_NO_PERMISSIONS_WARNING;
> +        message.extras = new Object[]{title, codeBase.toExternalForm()};
> +        Object selectedValue = getUserResponse(message);
> +
> +        // result 0 = Yes, 1 = No
> +        if (selectedValue instanceof Integer) {
> +            // If the selected value can be cast to Integer, use that 
> value
> +            return ((Integer) selectedValue).intValue();
> +        } else {
> +            // Otherwise default to "cancel"
> +            return 1;
> +        }
> +    }

As in the ALAC review, return a boolean or AppletAction here instead please.

Thanks,

-- 
Andrew A