[rfc][icedtea-web] Add -defaultfile switch to PolicyEditor
Jiri Vanek
jvanek at redhat.com
Thu Jul 30 16:18:12 UTC 2015
On 07/30/2015 06:14 PM, Andrew Azores wrote:
> On 30/07/15 12:06 PM, Andrew Azores wrote:
>> On 30/07/15 11:47 AM, Jiri Vanek wrote:
>>> On 07/30/2015 05:26 PM, Andrew Azores wrote:
>>>> Hi,
>>>>
>>>> This patch adds a -defaultfile switch to PolicyEditor which allows for directly opening the default
>>>> policy file (the one which will be used by ITW to load custom policies for runtime permissions)
>>>> from
>>>> a command line launch. If -defaultfile and -file are both specified then PolicyEditor dies because
>>>> it doesn't make sense for both to be provided.
>>>>
>>>
>>> I really like the idea :)))
>>>
>>> What is the deeper thought behind, hmm? :)
>>>
>>> Small nits at the end.
>>>
>>> (aand.... if you have ldracz around, tell him his generate flags for autocompeltion is heavily
>>> needed! :)) )
>>>> --
>>>> Thanks,
>>>>
>>>> Andrew Azores
>>>>
>>>>
>>>> defaultfile-switch.patch
>>>>
>>>>
>>>> # HG changeset patch
>>>> # User Andrew Azores<aazores at redhat.com>
>>>> # Date 1438269921 14400
>>>> # Thu Jul 30 11:25:21 2015 -0400
>>>> # Node ID d02feb071c3889f03849d46e314de1fb7b877358
>>>> # Parent e09b5c8bf3cb03247ecd6e11e359494e11030782
>>>> Add -defaultfile switch to PolicyEditor
>>>>
>>>> 2015-07-30 Andrew Azores<aazores at redhat.com>
>>>>
>>>> Add -defaultfile switch to PolicyEditor
>>>> * NEWS: add note about defaultfile flag
>>>> * netx/net/sourceforge/jnlp/OptionsDefinitions.java (OPTIONS,
>>>> getPolicyEditorOptions): add DEFAULTFILE for
>>>> PolicyEditor
>>>> * netx/net/sourceforge/jnlp/resources/Messages.properties (PBODefaultFile,
>>>> PEDefaultFileFilePathSpecifiedError): new messages
>>>> * netx/net/sourceforge/jnlp/security/policyeditor/PolicyEditor.java
>>>> (main): add handling for -defaultfile, die when both -defaultfile and
>>>> -file are given
>>>> (openDefaultButtonAction): extract method
>>>> (getDefaultPolicyFilePath): new method extracted from
>>>> openDefaultButtonAction
>>>>
>>>> diff --git a/ChangeLog b/ChangeLog
>>>> --- a/ChangeLog
>>>> +++ b/ChangeLog
>>>> @@ -1,3 +1,19 @@
>>>> +2015-07-30 Andrew Azores<aazores at redhat.com>
>>>> +
>>>> + Add -defaultfile switch to PolicyEditor
>>>> + * NEWS: add note about defaultfile flag
>>>> + * netx/net/sourceforge/jnlp/OptionsDefinitions.java (OPTIONS,
>>>> + getPolicyEditorOptions): add DEFAULTFILE for
>>>> + PolicyEditor
>>>> + * netx/net/sourceforge/jnlp/resources/Messages.properties (PBODefaultFile,
>>>> + PEDefaultFileFilePathSpecifiedError): new messages
>>>> + * netx/net/sourceforge/jnlp/security/policyeditor/PolicyEditor.java
>>>> + (main): add handling for -defaultfile, die when both -defaultfile and
>>>> + -file are given
>>>> + (openDefaultButtonAction): extract method
>>>> + (getDefaultPolicyFilePath): new method extracted from
>>>> + openDefaultButtonAction
>>>> +
>>>> 2015-07-29 Jiri Vanek<jvanek at redhat.com>
>>>>
>>>> Added tests for single signed jar with manifest containing trusted-only
>>>> diff --git a/NEWS b/NEWS
>>>> --- a/NEWS
>>>> +++ b/NEWS
>>>> @@ -30,6 +30,7 @@
>>>> - fixed issue with -html receiving garbage in width and height
>>>> * PolicyEditor
>>>> - file flag made to work when used standalone
>>>> + - defaultfile flag added
>>>> - support for SignedBy and Principals along with existing Codebase
>>>>
>>>> New in release 1.6 (2015-XX-XX):
>>>> diff --git a/netx/net/sourceforge/jnlp/OptionsDefinitions.java
>>>> b/netx/net/sourceforge/jnlp/OptionsDefinitions.java
>>>> --- a/netx/net/sourceforge/jnlp/OptionsDefinitions.java
>>>> +++ b/netx/net/sourceforge/jnlp/OptionsDefinitions.java
>>>> @@ -86,6 +86,7 @@
>>>> //policyeditor
>>>> //-help
>>>> FILE("-file", "policy_file", "PBOFile", NumberOfArguments.ONE),
>>>> + DEFAULTFILE("-defaultfile", "PBODefaultFile"),
>>>> CODEBASE("-codebase", "url", "PBOCodebase", NumberOfArguments.ONE),
>>>> SIGNEDBY("-signedby", "certificate_alias", "PBOSignedBy", NumberOfArguments.ONE),
>>>> PRINCIPALS("-principals", "class_name principal_name", "PBOPrincipals",
>>>> NumberOfArguments.EVEN_NUMBER_SUPPORTS_EQUALS_CHAR);
>>>> @@ -176,6 +177,7 @@
>>>> return Arrays.asList(new OPTIONS[]{
>>>> OPTIONS.HELP1,
>>>> OPTIONS.FILE,
>>>> + OPTIONS.DEFAULTFILE,
>>>> OPTIONS.CODEBASE,
>>>> OPTIONS.SIGNEDBY,
>>>> OPTIONS.PRINCIPALS,
>>>> diff --git a/netx/net/sourceforge/jnlp/resources/Messages.properties
>>>> b/netx/net/sourceforge/jnlp/resources/Messages.properties
>>>> --- a/netx/net/sourceforge/jnlp/resources/Messages.properties
>>>> +++ b/netx/net/sourceforge/jnlp/resources/Messages.properties
>>>> @@ -357,6 +357,8 @@
>>>>
>>>> PBOFile=Specifies a policy file path to open. If exactly one argument is given, and it is not
>>>> this flag, it is interpreted as a file path to open, as if this flag was given first. This flag
>>>> exists \
>>>> mostly for compatibility with Policy Tool.
>>>> +PBODefaultFile=Specifies that the default user-level policy file should be opened. This is the
>>>> file which is normally used by IcedTea-Web to make decisions about custom policies and
>>>> permissions \
>>>> +for applets at runtime, unless configured otherwise.
>>>> PBOCodebase=Specifies an applet codebase URL. This can be used with the other selector options
>>>> to select a policy entry upon opening the editor; \
>>>> if no such identifier exists then it will be created and then selected.
>>>> PBOSignedBy=Specifies a certificate alias for a certificate stored in the keystore. This can
>>>> be used with the other selector options to select a policy entry upon opening the editor; \
>>>> @@ -776,6 +778,7 @@
>>>> PEInvalidIdentifier=Please fill in/modify at least one of the fields.
>>>> PEIdentifierMatchesAll=Please fill in/modify at least one of the fields.
>>>> PEClipboardAccessError=Could not read from clipboard
>>>> +PEDefaultFileFilePathSpecifiedError=Either -file or -defaultfile may be specified, but not both
>>>>
>>>> PEHelpMenu=Help
>>>> PEAboutPolicyEditorItem=About PolicyEditor
>>>> diff --git a/netx/net/sourceforge/jnlp/security/policyeditor/PolicyEditor.java
>>>> b/netx/net/sourceforge/jnlp/security/policyeditor/PolicyEditor.java
>>>> --- a/netx/net/sourceforge/jnlp/security/policyeditor/PolicyEditor.java
>>>> +++ b/netx/net/sourceforge/jnlp/security/policyeditor/PolicyEditor.java
>>>> @@ -356,7 +356,7 @@
>>>> return;
>>>> }
>>>> try {
>>>> - PolicyEditor.this.setFile(new File(new
>>>> URI(PathsAndFiles.JAVA_POLICY.getFullPath())).getAbsolutePath());
>>>> + PolicyEditor.this.setFile(getDefaultPolicyFilePath());
>>>> PolicyEditor.this.getFile().createNewFile();
>>>> } catch (final IOException | URISyntaxException e) {
>>>> OutputController.getLogger().log(e);
>>>> @@ -516,6 +516,10 @@
>>>> setupLayout();
>>>> }
>>>>
>>>> + private static String getDefaultPolicyFilePath() throws URISyntaxException {
>>>> + return new File(new URI(PathsAndFiles.JAVA_POLICY.getFullPath())).getAbsolutePath();
>>>> + }
>>>
>>> Are you sure you ened URI???
>>> Isnt return PathsAndFiles.JAVA_POLICY.getFullPath() absolutley enough?
>>>
>>> If you look into PathsAndFiles you will see that you are converting string->uri->file->string
>>> Why uri inthe middle? If it isnotneeded, then string->file->string have no sense...
>>
>> PathsAndFiles.JAVA_POLICY.getFullPath() returns the path with a file:// protocol prepended to it,
>> so it's actually giving a URI to a filesystem resource, not simply a filesystem path. The
>> File(String) constructor doesn't like this, and so rather than trying to do something like trim
>> the file:// protocol off with string manipulation, I do the "create a URI from it and use that for
>> the File" hoop-jumping.
>>
>>>> +
>>>> private void addDefaultAllAppletsIdentifier() {
>>>> addNewEntry(PolicyIdentifier.ALL_APPLETS_IDENTIFIER);
>>>> }
>>>> @@ -1758,13 +1762,29 @@
>>>> SwingUtilities.invokeLater(new Runnable() {
>>>> @Override
>>>> public void run() {
>>>> - String filepath = optionParser.getParam(OptionsDefinitions.OPTIONS.FILE);
>>>> - if (filepath == null || filepath.isEmpty() || filepath.trim().isEmpty()) {
>>>> - // maybe the user just forgot the -file flag, so try to open anyway
>>>> - filepath = optionParser.getMainArg();
>>>> + final boolean openDefaultFile =
>>>> optionParser.hasOption(OptionsDefinitions.OPTIONS.DEFAULTFILE);
>>>> + final boolean hasFileArgument =
>>>> optionParser.hasOption(OptionsDefinitions.OPTIONS.FILE);
>>>> + if (hasFileArgument && openDefaultFile) {
>>>> + throw new IllegalArgumentException(R("PEDefaultFileFilePathSpecifiedError"));
>>>> }
>>>> - if (filepath == null || filepath.isEmpty() || filepath.trim().isEmpty()) {
>>>
>>> Isnt there missing condiotion of "defautl parameter"
>>>
>>> like
>>> policyeditor -defautlfile otherFile or policyeditor otherFile -defautlfile
>>> shold be invalid in same way as
>>> policyeditor -defautlfile -file otherFile or policyeditor -file otherFile -defautlfile
>>
>> Right, good catch.
>>
>>>
>>>> - filepath = null;
>>>> +
>>>> + String filepath = null;
>>>> + if (hasFileArgument) {
>>>> + filepath = optionParser.getParam(OptionsDefinitions.OPTIONS.FILE);
>>>> + if (filepath == null || filepath.isEmpty() || filepath.trim().isEmpty()) {
>>>> + // maybe the user just forgot the -file flag, so try to open anyway
>>>> + filepath = optionParser.getMainArg();
>>>> + }
>>>> + if (filepath == null || filepath.isEmpty() || filepath.trim().isEmpty()) {
>>>> + filepath = null;
>>>> + }
>>>> + } else if (openDefaultFile) {
>>>> + try {
>>>> + filepath = getDefaultPolicyFilePath();
>>>> + } catch (URISyntaxException e) {
>>>> + OutputController.getLogger().log(e);
>>>> + throw new RuntimeException(e);
>>>> + }
>>>> }
>>>> final PolicyEditorWindow frame = getPolicyEditorFrame(filepath);
>>>> final String codebase =
>>>> optionParser.getParam(OptionsDefinitions.OPTIONS.CODEBASE);
>>>>
>>>
>>> Cant this be done better?
>>> The hunk is a bit unreadable. But Maybe I'm just tired. If you dont figure something (definitely)
>>> nicer, go you can go with this one.
>>>
>>>
>>>
>>
>> Rolled in with the fix above.
>>
>
> Attached is a related a follow-up patch, not directly part of the -defaultfile feature. Just
> prohibiting providing both a -file switch as well as a main argument, so that ex. "policyeditor
> -file foo bar" or "policyeditor bar -file foo" is not allowed.
>
Ok for head and 1.6 ( I think it is valid although i have not checked)
Maybe mention it in news,....
Thanx!
More information about the distro-pkg-dev
mailing list