[rfc][icedtea-web] PolicyEditor dies on invalid CLI arguments
Jiri Vanek
jvanek at redhat.com
Mon Aug 17 08:41:34 UTC 2015
On 07/31/2015 12:19 AM, Andrew Azores wrote:
> Hi,
>
> This patch makes it so that PolicyEditor dies immediately if given invalid arguments via -codebase,
> -signedby, or -principals CLI switches.
>
> --
> Thanks,
>
> Andrew Azores
>
>
> exception-on-invalid-selector.patch
>
>
> # HG changeset patch
> # User Andrew Azores<aazores at redhat.com>
> # Date 1438294716 14400
> # Thu Jul 30 18:18:36 2015 -0400
> # Node ID 5f4260d54edaed95ece92ba7934f003275250919
> # Parent 09792ebffb27b328f47e5ab5f5e075f2c2fe0a18
> PolicyEditor dies when given invalid -codebase or -principals arguments
>
> * netx/net/sourceforge/jnlp/security/policyeditor/PolicyEditor.java
> (main): extracted helper method for getting codebase argument. Throw
> IllegalArgumentException when given invalid arguments to selector switches
> (getCodebaseArgument, getSignedByArgument, getPrincipalsArgument): new
> methods
> (cleanFilePathArgument): make parameter final for consistent style
> * tests/netx/unit/net/sourceforge/jnlp/security/policyeditor/PolicyEditorTest.java
> (testGetCodebaseArgument, testGetCodebaseArgument2,
> testGetCodebaseArgument3, testGetCodebaseArgumentWhenNotProvided,
> testGetPrincipalsArgument, testGetPrincipalsArgument2,
> testGetPrincipalsArgument3, testGetPrincipalsArgumentWhenNotProvided,
> testGetSignedByArgument, testGetSignedByArgumentWhenNotProvided): new tests
>
> diff --git a/ChangeLog b/ChangeLog
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,19 @@
> +2015-07-30 Andrew Azores<aazores at redhat.com>
> +
> + PolicyEditor dies when given invalid -codebase or -principals arguments
> + * netx/net/sourceforge/jnlp/security/policyeditor/PolicyEditor.java
> + (main): extracted helper method for getting codebase argument. Throw
> + IllegalArgumentException when given invalid arguments to selector switches
> + (getCodebaseArgument, getSignedByArgument, getPrincipalsArgument): new
> + methods
> + (cleanFilePathArgument): make parameter final for consistent style
> + * tests/netx/unit/net/sourceforge/jnlp/security/policyeditor/PolicyEditorTest.java
> + (testGetCodebaseArgument, testGetCodebaseArgument2,
> + testGetCodebaseArgument3, testGetCodebaseArgumentWhenNotProvided,
> + testGetPrincipalsArgument, testGetPrincipalsArgument2,
> + testGetPrincipalsArgument3, testGetPrincipalsArgumentWhenNotProvided,
> + testGetSignedByArgument, testGetSignedByArgumentWhenNotProvided): new tests
> +
> 2015-07-30 Andrew Azores<aazores at redhat.com>
>
> Add tests for PolicyEditor.getFilePathArgument
> 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
> @@ -1759,18 +1759,16 @@
> // not really important, so just ignore
> }
>
> +
> + final String filepath = getFilePathArgument(optionParser);
> + final String codebase = getCodebaseArgument(optionParser);
> + final String signedBy = getSignedByArgument(optionParser);
> + final Set<PolicyParser.PrincipalEntry> principals = getPrincipalsArgument(optionParser);
> +
> SwingUtilities.invokeLater(new Runnable() {
> @Override
> public void run() {
> - final String filepath = getFilePathArgument(optionParser);
> final PolicyEditorWindow frame = getPolicyEditorFrame(filepath);
> - final String codebase = optionParser.getParam(OptionsDefinitions.OPTIONS.CODEBASE);
> - final String signedBy = optionParser.getParam(OptionsDefinitions.OPTIONS.SIGNEDBY);
> - final List<String> rawPrincipals = optionParser.getParams(OptionsDefinitions.OPTIONS.PRINCIPALS);
> - final Set<PolicyParser.PrincipalEntry> principals = new HashSet<>();
> - for (int i = 0; i < rawPrincipals.size(); i+= 2) {
> - principals.add(new PolicyParser.PrincipalEntry(rawPrincipals.get(i), rawPrincipals.get(i + 1)));
> - }
> frame.getPolicyEditor().openPolicyFileSynchronously();
> frame.getPolicyEditor().addNewEntry(new PolicyIdentifier(signedBy, principals, codebase));
> frame.asWindow().setVisible(true);
> @@ -1778,7 +1776,50 @@
> });
> }
>
> - static String getFilePathArgument(OptionParser optionParser) {
> + static String getCodebaseArgument(final OptionParser optionParser) {
> + if (optionParser.hasOption(OptionsDefinitions.OPTIONS.CODEBASE)) {
> + final String codebase = optionParser.getParam(OptionsDefinitions.OPTIONS.CODEBASE);
> + try {
> + new URL(codebase);
> + } catch (final MalformedURLException e) {
> + throw new IllegalArgumentException("Invalid URL", e);
> + }
> + return codebase;
> + } else {
> + return null;
> + }
> + }
> +
> + static String getSignedByArgument(final OptionParser optionParser) {
> + if (optionParser.hasOption(OptionsDefinitions.OPTIONS.SIGNEDBY)) {
> + final String signedBy = optionParser.getParam(OptionsDefinitions.OPTIONS.SIGNEDBY);
> + if (signedBy.isEmpty()) {
> + throw new IllegalArgumentException("SignedBy cannot be empty");
> + } else {
> + return signedBy;
> + }
> + } else {
> + return null;
> + }
> + }
> +
> + static Set<PolicyParser.PrincipalEntry> getPrincipalsArgument(final OptionParser optionParser) {
> + if (optionParser.hasOption(OptionsDefinitions.OPTIONS.PRINCIPALS)) {
> + final List<String> rawPrincipals = optionParser.getParams(OptionsDefinitions.OPTIONS.PRINCIPALS);
> + if (rawPrincipals.size() % 2 != 0) {
> + throw new IllegalArgumentException("List of principals must have an even number of elements: " + rawPrincipals);
This does not seems right.p
ITs deffined as:
PRINCIPALS("-principals", "class_name principal_name", "PBOPrincipals",
NumberOfArguments.EVEN_NUMBER_SUPPORTS_EQUALS_CHAR);
So even should be already thrown.
Also the test for case like
String[] args = new String[] { "-principals", "aa", "bb" };
is missing.
Thanx!
J.
> + }
> + final Set<PolicyParser.PrincipalEntry> principals = new HashSet<>();
> + for (int i = 0; i < rawPrincipals.size(); i+= 2) {
> + principals.add(new PolicyParser.PrincipalEntry(rawPrincipals.get(i), rawPrincipals.get(i + 1)));
> + }
> + return principals;
> + } else {
> + return Collections.emptySet();
> + }
> + }
> +
> + static String getFilePathArgument(final OptionParser optionParser) {
> final boolean openDefaultFile = optionParser.hasOption(OptionsDefinitions.OPTIONS.DEFAULTFILE);
> final boolean hasFileArgument = optionParser.hasOption(OptionsDefinitions.OPTIONS.FILE);
> final boolean hasMainArgument = optionParser.mainArgExists();
> @@ -1804,7 +1845,7 @@
> return filepath;
> }
>
> - private static String cleanFilePathArgument(String filepath) {
> + private static String cleanFilePathArgument(final String filepath) {
> if (filepath == null) {
> return null;
> } else if (filepath.isEmpty() || filepath.trim().isEmpty()) {
> diff --git a/tests/netx/unit/net/sourceforge/jnlp/security/policyeditor/PolicyEditorTest.java b/tests/netx/unit/net/sourceforge/jnlp/security/policyeditor/PolicyEditorTest.java
> --- a/tests/netx/unit/net/sourceforge/jnlp/security/policyeditor/PolicyEditorTest.java
> +++ b/tests/netx/unit/net/sourceforge/jnlp/security/policyeditor/PolicyEditorTest.java
> @@ -46,12 +46,14 @@
> import java.net.URISyntaxException;
> import java.util.Collection;
> import java.util.HashSet;
> +import java.util.List;
> import java.util.Map;
> import java.util.Set;
>
> import net.sourceforge.jnlp.OptionsDefinitions;
> import net.sourceforge.jnlp.config.PathsAndFiles;
> import net.sourceforge.jnlp.util.optionparser.OptionParser;
> +import net.sourceforge.jnlp.util.optionparser.UnevenParameterException;
> import org.junit.Before;
> import org.junit.Test;
> import sun.security.provider.PolicyParser;
> @@ -375,4 +377,72 @@
> PolicyEditor.getFilePathArgument(optionParser);
> }
>
> + @Test
> + public void testGetCodebaseArgument() {
> + String[] args = new String[] { "-codebase","http://example.com" };
> + OptionParser optionParser = new OptionParser(args, OptionsDefinitions.getPolicyEditorOptions());
> + String result = PolicyEditor.getCodebaseArgument(optionParser);
> + assertTrue(result.equals("http://example.com"));
> + }
> +
> + @Test(expected = IllegalArgumentException.class)
> + public void testGetCodebaseArgument2() {
> + String[] args = new String[] { "-codebase", "" };
> + OptionParser optionParser = new OptionParser(args, OptionsDefinitions.getPolicyEditorOptions());
> + PolicyEditor.getCodebaseArgument(optionParser);
> + }
> +
> + @Test(expected = IllegalArgumentException.class)
> + public void testGetCodebaseArgument3() {
> + String[] args = new String[] { "-codebase", "example.com" };
> + OptionParser optionParser = new OptionParser(args, OptionsDefinitions.getPolicyEditorOptions());
> + PolicyEditor.getCodebaseArgument(optionParser);
> + }
> +
> + @Test(expected = IllegalArgumentException.class)
> + public void testGetCodebaseArgumentWhenNotProvided() {
> + String[] args = new String[] { "-codebase" };
> + OptionParser optionParser = new OptionParser(args, OptionsDefinitions.getPolicyEditorOptions());
> + String result = PolicyEditor.getCodebaseArgument(optionParser);
> + }
> +
> + @Test
> + public void testGetPrincipalsArgument() {
> + String[] args = new String[] { "-principals", "aa=bb" };
> + OptionParser optionParser = new OptionParser(args, OptionsDefinitions.getPolicyEditorOptions());
> + Set<PolicyParser.PrincipalEntry> result = PolicyEditor.getPrincipalsArgument(optionParser);
> + assertTrue(result.size() == 1);
> + assertTrue(result.contains(new PolicyParser.PrincipalEntry("aa", "bb")));
> + }
> +
> + @Test(expected = UnevenParameterException.class)
> + public void testGetPrincipalsArgument2() {
> + String[] args = new String[] { "-principals", "aa=bb", "cc" };
> + OptionParser optionParser = new OptionParser(args, OptionsDefinitions.getPolicyEditorOptions());
> + PolicyEditor.getPrincipalsArgument(optionParser);
> + }
> +
> + @Test
> + public void testGetPrincipalsArgumentWhenNotProvided() {
> + String[] args = new String[] { "-principals" };
> + OptionParser optionParser = new OptionParser(args, OptionsDefinitions.getPolicyEditorOptions());
> + Set<PolicyParser.PrincipalEntry> result = PolicyEditor.getPrincipalsArgument(optionParser);
> + assertTrue(result.isEmpty());
> + }
> +
> + @Test
> + public void testGetSignedByArgument() {
> + String[] args = new String[] { "-signedby", "foo" };
> + OptionParser optionParser = new OptionParser(args, OptionsDefinitions.getPolicyEditorOptions());
> + String result = PolicyEditor.getSignedByArgument(optionParser);
> + assertTrue(result.equals("foo"));
> + }
> +
> + @Test
> + public void testGetSignedByArgumentWhenNotProvided() {
> + String[] args = new String[] { "-signedby" };
> + OptionParser optionParser = new OptionParser(args, OptionsDefinitions.getPolicyEditorOptions());
> + String result = PolicyEditor.getSignedByArgument(optionParser);
> + assertTrue("expected null, got: " + result, result == null);
> + }
> }
>
More information about the distro-pkg-dev
mailing list