[rfc][icedtea-web] Add -defaultfile switch to PolicyEditor
Andrew Azores
aazores at redhat.com
Thu Jul 30 16:14:36 UTC 2015
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.
--
Thanks,
Andrew Azores
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mainarg-and-file.patch
Type: text/x-patch
Size: 1720 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20150730/a7e5d3cc/mainarg-and-file.patch>
More information about the distro-pkg-dev
mailing list