[rfc][icedteaweb] Add comments about permission attributes not being checked in reproducers
Jiri Vanek
jvanek at redhat.com
Wed Mar 11 15:23:08 UTC 2015
On 03/10/2015 08:54 PM, Lukasz Dracz wrote:> Hello,
>
> I have attached a patch to show, what I was thinking of as a potential fix.
>
maybe a bit of owerkill, but why not.
> I made a new BasicValueValidator that allows combination of options alongside single options.
> I changed attributes check property from a boolean to an enum
> with the following options ALL, NONE, PERMISSIONS, TRUSTED, CODEBASE, ALAC
> With the new MultipleStringValueValidator I have made it that
> attributes check property can either be ALL, NONE or a
> combination of PERMISSIONS, TRUSTED, CODEBASE, ALAC (delimited by "," no spaces)
Please add ENTRYPOINT
>
> In this way it can be specified which checks you want to do
> for ex. only TRUSTED and CODEBASE attributes would look like
> deployment.manifest.attributes.check=TRUSTED,CODEBASE
>
> The fix for the tests is also included that changes the deployment.manifest.attributes.check in the beforeClass
> and puts back the previous value afterClass. Also deployment.security.level is changed to ALLOW_UNSIGNED and back in
> the same fashion for the Partially Signed tests.
>
> I have mostly attached the full patch to get feedback on this direction of a fix ?
> Any comments or thoughts would be great.
>
> For review I think it would be best if I split this patch into two, for easier reviewing
>
> - The MultipleStringValueValidator
> - The Fix with Manifest Attributes Check and in the tests
>
> Other option is to go with the original suggestion of just an enum with true, false, permissions_only with the same
> changing the deployment.properties BeforeClass fix. In this case, I would prefer the enum to be ALL, NONE or
> PERMISSIONS (/PERMISSIONS_ONLY) instead.
if this fixs the testsm then +1 from me for what you have done here. It is allowing good versatility and actually to test what is necessary.
>
> Thoughts ?
few inline.
>
>
> Thank you,
> Lukasz Dracz
>
> ----- Original Message -----
>> >From: "Jiri Vanek"<jvanek at redhat.com>
>> >To: "Lukasz Dracz"<ldracz at redhat.com>
>> >Cc: "IcedTea Distro List"<distro-pkg-dev at openjdk.java.net>
>> >Sent: Friday, March 6, 2015 9:11:58 AM
>> >Subject: Re: [rfc][icedteaweb] Add comments about permission attributes not being checked in reproducers
>> >
>> >On 03/05/2015 10:52 PM, Lukasz Dracz wrote:
>>> > >Hello,
>>> > >
>>> > >----- Original Message -----
>>>> > >>From: "Jiri Vanek"<jvanek at redhat.com>
>>>> > >>To: "Lukasz Dracz"<ldracz at redhat.com>, "IcedTea Distro List"
>>>> > >><distro-pkg-dev at openjdk.java.net>
>>>> > >>Sent: Thursday, March 5, 2015 2:48:25 AM
>>>> > >>Subject: Re: [rfc][icedteaweb] Add comments about permission attributes
>>>> > >>not being checked in reproducers
>>>> > >>
>>>> > >>On 03/04/2015 10:00 PM, Lukasz Dracz wrote:
>>>>> > >>>Hello,
>>>>> > >>>
>>>>> > >>>I have been looking into why certain reproducers are failing. For the
>>>>> > >>>following tests
>>>>> > >>>
>>>>> > >>>testPartiallySignedAppletWithSandboxPermissionsInManifestLaunchWithSignedHTMLApp
>>>>> > >>>testPartiallySignedJNLPAppletWithSandboxPermissionsInManifestLaunchWithSignedApp
>>>>> > >>>testSignedAppletWithSandboxPermissionsInManifestHtml
>>>>> > >>>testSignedAppletWithSandboxPermissionsInManifestHtmlJnlpHref
>>>>> > >>>testSignedAppletWithSandboxPermissionsInManifestJnlpApplet
>>>>> > >>>testSignedAppletWithSandboxPermissionsInManifestJnlpApplication
>>>>> > >>>
>>>>> > >>>I found that changeset 1129:0284eb954ebc is reason.
>>>>> > >>>
>>>>> > >>>The reason why is that the permissions attributes check has been moved
>>>>> > >>>from
>>>>> > >>>always being checked into an if statement
>>>>> > >>>where they are only checked when deployment.manifest.attributes.check is
>>>>> > >>>set to true. I have tested and found the tests to pass when this
>>>>> > >>>deployment.manifest.attributes.check is set to true.
>>>>> > >>>
>>>>> > >>>This patch just adds comments to the tests for reference to help explain
>>>>> > >>>to
>>>>> > >>>anyone in the future looking at the tests. At the moment I don't have a
>>>>> > >>>solution, does anyone have an idea on how to properly fix this or handle
>>>>> > >>>this ?
>>>>> > >>>I had tried to set the property to true for the tests and revert it back
>>>>> > >>>after to no avail. I will try to think more on this problem.
>>>>> > >>>
>>>> > >>
>>>> > >>Hi!
>>>> > >>
>>>> > >>So yes - the testsuite is running with
>>>> > >>deployment.manifest.attributes.check=false.
>>>> > >>
>>>> > >>IMHO best would be to tune it to run without any
>>>> > >>deployment.manifest.attributes.check.
>>>> > >>
>>>> > >>
>>>> > >>For your group of tests - I think the best way to go is :
>>>> > >>@beforeclass ensure that no deployment.manifest.attributes.check is
>>>> > >>present
>>>> > >>in
>>>> > >>deploymnet.properties. If it is, remove it.
>>>> > >>@afterclass return it back, if it was here.
>>>> > >>
>>>> > >>
>>>> > >>What do you think? Note - tehre is plenty util methods which loads file
>>>> > >>to
>>>> > >>string or save string to
>>>> > >>file. Just do not add more :)
>>> > >
>>> > >I have tested the @beforeclass @afterclass method of editing the
>>> > >deployment.properties file and well it works somewhat, some of the tests
>>> > >pass now
>>> > >but the other ones will prompt a pop-up and if you click to proceed it
>>> > >passes but obviously failing to click proceed it fails.
>> >Maybe instead of beforeclass and after class use this setup only for
>> >specified tests?
>> >
>>> > >
>>> > >This defeats the purpose of the manifest attributes check being set to
>>> > >false.
>>> > >I can't seem to think of a solution I would like.
>> >yes:(
>>> > >I suppose we could split the deployment.manifest.attributes.check into two
>>> > >properties, by adding a new one that decides whether to check the
>>> > >Permissions Attribute and the default deployment.manifest.attributes.check
>>> > >would check all the other attributes.
>>> > >
>>> > >For the test we would edit the manifest like you mentioned with
>>> > >@beforeclass to change the
>>> > >deployment.manifest.permissions.attributes.check to true and keep
>>> > >deployment.manifest.attributes.check false.
>>> > >What do you think about this solution ?
>>> > >Do you have any other ideas ?
>> >
>> >The only other idea of mine is to make deployment.manifest.attributes.check
>> >tri state (enum TRUE
>> >FALSE PERMISSIONS_ONLY) instead of boolean an new attribute - true (all are
>> >tests) false (none is
>> >tested) all_but_permissions - then only permission s attribnute is check.
>> >
>> >The deployment.manifest.attributes.check is itw Internal, so we can do
>> >moreover whatever we need
>> >with it. And the code is documentation itself .
>> >
>> >imho - the tristate is better to impl. maintain read. But instead of it, go
>> >with any option you wont
>> >to fix those tests.
>>> > >
>>> > >I suppose we could also try to make it possible to edit the property
>>> > >directly and having that change updated when done. I would need to look
>>> > >into how feasible this is.
>> >
>> >Not sure If I understood:(
>>> > >
>>> > >Thoughts ?
>>> > >
>>> > >Thank you,
>>> > >Lukasz Dracz
>>> > >
>>> > >
>>> > >
>>> > >
>>>> > >>Thank you!
>>>> > >>
>>>> > >>J.
>>>> > >>
>>>> > >>
>>>> > >>
>>>> > >>
>>>> > >>
>>>> > >>
>>>> > >>
>> >
>> >
>
> combinationManifestAttributesCheck-2.patch
>
>
> diff --git a/netx/net/sourceforge/jnlp/config/BasicValueValidators.java b/netx/net/sourceforge/jnlp/config/BasicValueValidators.java
> --- a/netx/net/sourceforge/jnlp/config/BasicValueValidators.java
> +++ b/netx/net/sourceforge/jnlp/config/BasicValueValidators.java
> @@ -202,6 +202,69 @@
> }
>
> /**
> + * Checks that the value is one of the acceptable single String values
> + * or an acceptable combination of String values
> + */
> + private static class MultipleStringValueValidator implements ValueValidator {
> + String[] singleOptions = null;
> + String[] comboOptions = null;
make them final.
They may be set in constructor only.
> +
> + public MultipleStringValueValidator(String[] singleOptions, String[] comboOptions) {
> + this.singleOptions = singleOptions;
> + this.comboOptions = comboOptions;
> + }
> +
> + @Override
> + public void validate(Object value) throws IllegalArgumentException {
> + Object possibleValue = value;
> + if (!(possibleValue instanceof String)) {
> + throw new IllegalArgumentException("Must be a string");
> + }
> +
> + String stringVal = (String) possibleValue;
> + boolean found = false;
> + for (String knownVal : singleOptions) {
> + if (knownVal.equals(stringVal)) {
> + found = true;
> + break;
> + }
> + }
> +
> + if (!found) {
> + String[] possibleCombo = stringVal.split(",");
separate this split in into static method in this class. and move the "," to constant.
See the reasons for this later in code
> + for (String val : possibleCombo) {
> + if (comboOptionsContains(val)) {
> + found = true;
> + } else {
> + throw new IllegalArgumentException();
> + }
> + }
> + }
> +
> + if (!found) {
> + throw new IllegalArgumentException();
> + }
> + }
> +
> + private boolean comboOptionsContains(String possibleVal) {
> + for (String value : comboOptions) {
> + if (value.equals(possibleVal)) {
> + return true;
> + }
> + }
> + return false;
> + }
> +
> + @Override
> + public String getPossibleValues() {
> + String message = "(Values that can be used alone only): " + Arrays.toString(singleOptions) +
> + " (Values that can be used in combination): " + Arrays.toString(comboOptions);
Mention in second one that the constant of above is an delimiter. and no space expected
Also it can behave nicer if one of the arrays is null or empty...
> + return message;
> + }
Except this one, use also
ManifestAttributeCheckValidator extends MultipleStringValueValidator
public ManifestAttributeCheckValidator(){
super(youKNowTheValues1,youKNowTheValues2)
Check the behaviour with http://mail.openjdk.java.net/pipermail/distro-pkg-dev/2015-March/031043.html
}
> +
> + }
> +
> + /**
> * Checks that the value is a URL
> */
> private static class UrlValidator implements ValueValidator {
> @@ -262,6 +325,18 @@
> }
>
> /**
> + * Returns a {@link ValueValidator} that checks if an object is a string from
> + * one of the provided single option Strings or a combination from
> + * the provided combination Strings.
> + * @param singleValues an array of Strings which are considered valid only by themselves
> + * @param comboValues an array of Strings which are considered valid in any combination
> + * with themselves
> + */
> + public static ValueValidator getMultipleStringValidator(String[] singleValues, String[] comboValues) {
> + return new MultipleStringValueValidator(singleValues, comboValues);
> + }
add getter also for the ManifestAttributeCheckValidator
> +
> + /**
> * @return a {@link ValueValidator} that checks if an object represents a
> * valid url
> */
> diff --git a/netx/net/sourceforge/jnlp/config/Defaults.java b/netx/net/sourceforge/jnlp/config/Defaults.java
> --- a/netx/net/sourceforge/jnlp/config/Defaults.java
> +++ b/netx/net/sourceforge/jnlp/config/Defaults.java
> @@ -43,6 +43,8 @@
> import net.sourceforge.jnlp.ShortcutDesc;
> import static net.sourceforge.jnlp.config.PathsAndFiles.*;
> import net.sourceforge.jnlp.runtime.JNLPProxySelector;
> +import net.sourceforge.jnlp.runtime.ManifestAttributesChecker;
> +
> import static net.sourceforge.jnlp.runtime.Translator.R;
>
> /**
> @@ -409,8 +411,16 @@
> //enable manifest-attributes checks
> {
> DeploymentConfiguration.KEY_ENABLE_MANIFEST_ATTRIBUTES_CHECK,
> - BasicValueValidators.getBooleanValidator(),
> - String.valueOf(true)
> + BasicValueValidators.getMultipleStringValidator(new String[] {
> + ManifestAttributesChecker.MANIFEST_ATTRIBUTES_CHECK.ALL.toString(),
> + ManifestAttributesChecker.MANIFEST_ATTRIBUTES_CHECK.NONE.toString()
> + }, new String[] {
> + ManifestAttributesChecker.MANIFEST_ATTRIBUTES_CHECK.PERMISSIONS.toString(),
> + ManifestAttributesChecker.MANIFEST_ATTRIBUTES_CHECK.CODEBASE.toString(),
> + ManifestAttributesChecker.MANIFEST_ATTRIBUTES_CHECK.TRUSTED.toString(),
> + ManifestAttributesChecker.MANIFEST_ATTRIBUTES_CHECK.ALAC.toString()
USe the ManifestAttributeCheckValidator
> + }),
> + String.valueOf("ALL")
> }
> };
>
> diff --git a/netx/net/sourceforge/jnlp/runtime/ManifestAttributesChecker.java b/netx/net/sourceforge/jnlp/runtime/ManifestAttributesChecker.java
> --- a/netx/net/sourceforge/jnlp/runtime/ManifestAttributesChecker.java
> +++ b/netx/net/sourceforge/jnlp/runtime/ManifestAttributesChecker.java
> @@ -37,7 +37,9 @@
> package net.sourceforge.jnlp.runtime;
>
> import java.net.URL;
> +import java.util.Arrays;
> import java.util.HashSet;
> +import java.util.List;
> import java.util.Set;
>
> import net.sourceforge.jnlp.ExtensionDesc;
> @@ -75,21 +77,45 @@
> this.securityDelegate = securityDelegate;
> }
>
> + public enum MANIFEST_ATTRIBUTES_CHECK {
> + ALL,
> + NONE,
> + PERMISSIONS,
> + CODEBASE,
> + TRUSTED,
> + ALAC
Please add also ENTRYPOINT (or I will forget it in next patch)
> + }
> +
> void checkAll() throws LaunchException {
> - if (isCheckEnabled()) {
> - checkPermissionsAttribute();
> + List<String> attributesCheck = getAttributesCheck();
> + if (attributesCheck.contains(MANIFEST_ATTRIBUTES_CHECK.ALL.toString())) {
> checkTrustedOnlyAttribute();
> checkCodebaseAttribute();
> checkPermissionsAttribute();
> checkApplicationLibraryAllowableCodebaseAttribute();
> } else {
> - OutputController.getLogger().log(OutputController.Level.WARNING_ALL, MANIFEST_CHECK_DISABLED_MESSAGE);
> + if (attributesCheck.contains(MANIFEST_ATTRIBUTES_CHECK.TRUSTED.toString())) {
> + checkTrustedOnlyAttribute();
> + }
> + if (attributesCheck.contains(MANIFEST_ATTRIBUTES_CHECK.CODEBASE.toString())) {
> + checkCodebaseAttribute();
> + }
> + if (attributesCheck.contains(MANIFEST_ATTRIBUTES_CHECK.PERMISSIONS.toString())) {
> + checkPermissionsAttribute();
> + }
> + if (attributesCheck.contains(MANIFEST_ATTRIBUTES_CHECK.ALAC.toString())) {
> + checkApplicationLibraryAllowableCodebaseAttribute();
> + }
> + if (attributesCheck.contains(MANIFEST_ATTRIBUTES_CHECK.NONE.toString())) {
> + OutputController.getLogger().log(OutputController.Level.WARNING_ALL, MANIFEST_CHECK_DISABLED_MESSAGE);
> + }
I would rewrite this hunk a bit:
List<String> attributesCheck = getAttributesCheck();
if (attributesCheck.contains(MANIFEST_ATTRIBUTES_CHECK.ALL.toString())) {
checkTrustedOnlyAttribute();
checkCodebaseAttribute();
checkPermissionsAttribute();
checkApplicationLibraryAllowableCodebaseAttribute();
} else {
if (attributesCheck.contains(MANIFEST_ATTRIBUTES_CHECK.TRUSTED.toString())) {
checkTrustedOnlyAttribute();
}
if (attributesCheck.contains(MANIFEST_ATTRIBUTES_CHECK.CODEBASE.toString())) {
checkCodebaseAttribute();
}
if (attributesCheck.contains(MANIFEST_ATTRIBUTES_CHECK.PERMISSIONS.toString())) {
checkPermissionsAttribute();
}
if (attributesCheck.contains(MANIFEST_ATTRIBUTES_CHECK.ALAC.toString())) {
checkApplicationLibraryAllowableCodebaseAttribute();
}
if (attributesCheck.contains(MANIFEST_ATTRIBUTES_CHECK.NONE.toString())) {
OutputController.getLogger().log(OutputController.Level.WARNING_ALL, MANIFEST_CHECK_DISABLED_MESSAGE);
}
}
=>
List<String> attributesCheck = getAttributesCheck();
*maybe this can be done on enum level rather then on Sting level?
if (attributesCheck.contains(MANIFEST_ATTRIBUTES_CHECK.TRUSTED) OR attributesCheck.haveOnlyAll) {
checkTrustedOnlyAttribute();
} else {
OutputController.getLogger().log(OutputController.Level.WARNING_ALL, "check on Trusted-Only skipped because properrty of enable.manifest... have value(s) of: reprint that);
if (attributesCheck.contains(MANIFEST_ATTRIBUTES_CHECK.CODEBASE.toString())) {
checkCodebaseAttribute();
}
... same for rest. What do you think?
}
> }
> }
>
> - public static boolean isCheckEnabled() {
> + public static List<String> getAttributesCheck() {
> final String deploymentProperty = JNLPRuntime.getConfiguration().getProperty(DeploymentConfiguration.KEY_ENABLE_MANIFEST_ATTRIBUTES_CHECK);
> - return Boolean.parseBoolean(deploymentProperty);
> + String[] attributesCheck = deploymentProperty.split(",");
> + return Arrays.asList(attributesCheck);
Well, map back to ENUM rather.
> }
>
> /**
> diff --git a/tests/netx/unit/net/sourceforge/jnlp/runtime/JNLPFileTest.java b/tests/netx/unit/net/sourceforge/jnlp/runtime/JNLPFileTest.java
> --- a/tests/netx/unit/net/sourceforge/jnlp/runtime/JNLPFileTest.java
> +++ b/tests/netx/unit/net/sourceforge/jnlp/runtime/JNLPFileTest.java
> @@ -65,7 +65,7 @@
> @BeforeClass
> public static void setPermissions() {
> level = AppletStartupSecuritySettings.getInstance().getSecurityLevel();
> - attCheckValue = ManifestAttributesChecker.isCheckEnabled();
> + attCheckValue = ManifestAttributesChecker.getAttributesCheck();
> JNLPRuntime.getConfiguration().setProperty(DeploymentConfiguration.KEY_SECURITY_LEVEL, AppletSecurityLevel.ALLOW_UNSIGNED.toChars());
> JNLPRuntime.getConfiguration().setProperty(DeploymentConfiguration.KEY_ENABLE_MANIFEST_ATTRIBUTES_CHECK, String.valueOf(true));
Are you sure that this is the only change needed here?
JNLPRuntime.getConfiguration().setProperty(DeploymentConfiguration.KEY_ENABLE_MANIFEST_ATTRIBUTES_CHECK, String.valueOf(attCheckValue));
sounds more fragile...
> }
Please add one or two simple unittests for new (two!) validators
> diff --git a/tests/reproducers/custom/PartiallySignedAppletManifestSpecifiesSandbox/testcases/PartiallySignedAppletManifestSpecifiesSandboxTests.java b/tests/reproducers/custom/PartiallySignedAppletManifestSpecifiesSandbox/testcases/PartiallySignedAppletManifestSpecifiesSandboxTests.java
> --- a/tests/reproducers/custom/PartiallySignedAppletManifestSpecifiesSandbox/testcases/PartiallySignedAppletManifestSpecifiesSandboxTests.java
> +++ b/tests/reproducers/custom/PartiallySignedAppletManifestSpecifiesSandbox/testcases/PartiallySignedAppletManifestSpecifiesSandboxTests.java
> @@ -36,6 +36,10 @@
> */
>
> import static org.junit.Assert.assertTrue;
> +
> +import java.io.File;
> +import java.io.IOException;
> +
> import net.sourceforge.jnlp.ProcessResult;
> import net.sourceforge.jnlp.annotations.Bug;
> import net.sourceforge.jnlp.annotations.NeedsDisplay;
> @@ -44,6 +48,9 @@
> import net.sourceforge.jnlp.browsertesting.Browsers;
> import net.sourceforge.jnlp.closinglisteners.AutoOkClosingListener;
>
> +import net.sourceforge.jnlp.util.FileUtils;
> +import org.junit.AfterClass;
> +import org.junit.BeforeClass;
> import org.junit.Test;
>
> public class PartiallySignedAppletManifestSpecifiesSandboxTests extends BrowserTest {
> @@ -56,6 +63,67 @@
> private static final String STACKTRACE_NOT_GRANT_PERMISSIONS_TYPE = "Cannot grant permissions to unsigned jars";
> private static final String USER_HOME = System.getProperty("user.home");
>
> + private static File deployFile;
> + private static String attributesCheck;
> + private static String securityLevel;
> + private static boolean foundAttributesCheck = false;
> + private static boolean foundSecurityLevel = false;
> +
> + @BeforeClass
> + public static void setupDeploymentProperties() throws IOException {
> + String location = System.getProperty("user.home") + "/.config/icedtea-web/deployment.properties";
NO! use PathsAndFiles!!!
> + deployFile = new File(location);
> + String properties = FileUtils.loadFileAsString(deployFile);
> + String deployProperties = "";
> +
Please simplify this:
> + for (String line : properties.split("\n")) {
> + if (line.contains("deployment.manifest.attributes.check")) {
> + attributesCheck = line;
> + deployProperties += "deployment.manifest.attributes.check=PERMISSIONS\n";
> + foundAttributesCheck = true;
not needed, or not? You can use attributesCheck and null/not null
> + } else if (line.contains("deployment.security.level")) {
> + securityLevel = line;
> + deployProperties += "deployment.security.level=ALLOW_UNSIGNED\n";
> + foundSecurityLevel = true;
same here, or not?
> + } else {
> + deployProperties += line + "\n";
> + }
> + }
> +
> + if (!foundAttributesCheck) {
> + deployProperties += "deployment.manifest.attributes.check=PERMISSIONS\n";
> + }
> + if (!foundSecurityLevel) {
> + deployProperties += "deployment.security.level=ALLOW_UNSIGNED\n";
> + }
> +
> + FileUtils.saveFile(deployProperties, deployFile);
> + }
> +
> + @AfterClass
> + public static void setbackDeploymentProperties() throws IOException {
> + String location = System.getProperty("user.home") + "/.config/icedtea-web/deployment.properties";
NO! use PathsAndFiles!!!
> + deployFile = new File(location);
> + String properties = FileUtils.loadFileAsString(deployFile);
> + String deployProperties = "";
> +
> + for (String line : properties.split("\n")) {
> + if (line.contains("deployment.manifest.attributes.check")) {
> + if (foundAttributesCheck) {
> + deployProperties += attributesCheck + "\n";
> + }
> + } else if (line.contains("deployment.security.level")) {
> + if (foundSecurityLevel) {
> + deployProperties += securityLevel + "\n";
> + }
> + } else {
> + deployProperties += line + "\n";
> + }
> + }
> +
> + FileUtils.saveFile(deployProperties, deployFile);
> + }
> +
> @Test
> @NeedsDisplay
> @TestInBrowsers(testIn={Browsers.one})
> diff --git a/tests/reproducers/signed/SignedAppletManifestSpecifiesSandbox/testcases/SignedAppletManifestSpecifiesSandboxTests.java b/tests/reproducers/signed/SignedAppletManifestSpecifiesSandbox/testcases/SignedAppletManifestSpecifiesSandboxTests.java
> --- a/tests/reproducers/signed/SignedAppletManifestSpecifiesSandbox/testcases/SignedAppletManifestSpecifiesSandboxTests.java
> +++ b/tests/reproducers/signed/SignedAppletManifestSpecifiesSandbox/testcases/SignedAppletManifestSpecifiesSandboxTests.java
> @@ -36,6 +36,10 @@
> */
>
> import static org.junit.Assert.assertTrue;
> +
> +import java.io.File;
> +import java.io.IOException;
> +
> import net.sourceforge.jnlp.ProcessResult;
> import net.sourceforge.jnlp.annotations.Bug;
> import net.sourceforge.jnlp.annotations.NeedsDisplay;
> @@ -44,6 +48,9 @@
> import net.sourceforge.jnlp.browsertesting.Browsers;
> import net.sourceforge.jnlp.closinglisteners.AutoOkClosingListener;
>
> +import net.sourceforge.jnlp.util.FileUtils;
> +import org.junit.AfterClass;
> +import org.junit.BeforeClass;
> import org.junit.Test;
>
> public class SignedAppletManifestSpecifiesSandboxTests extends BrowserTest {
> @@ -56,6 +63,54 @@
> private static final String JNLP_EXPECTED_STDOUT = "Initialization Error";
> private static final String JNLP_EXPECTED_STDERR = "net.sourceforge.jnlp.LaunchException";
>
> + private static File deployFile;
> + private static String attributesCheck;
> + private static boolean foundAttributesCheck = false;
> +
> + @BeforeClass
> + public static void setupDeploymentProperties() throws IOException {
> + String location = System.getProperty("user.home") + "/.config/icedtea-web/deployment.properties";
> + deployFile = new File(location);
> + String properties = FileUtils.loadFileAsString(deployFile);
> + String deployProperties = "";
> +
> + for (String line : properties.split("\n")) {
> + if (line.contains("deployment.manifest.attributes.check")) {
> + attributesCheck = line;
> + deployProperties += "deployment.manifest.attributes.check=PERMISSIONS\n";
> + foundAttributesCheck = true;
> + } else {
> + deployProperties += line + "\n";
> + }
> + }
> +
> + if (!foundAttributesCheck) {
> + deployProperties += "deployment.manifest.attributes.check=PERMISSIONS\n";
> + }
> +
> + FileUtils.saveFile(deployProperties, deployFile);
> + }
> +
> + @AfterClass
> + public static void setbackDeploymentProperties() throws IOException {
> + String location = System.getProperty("user.home") + "/.config/icedtea-web/deployment.properties";
> + deployFile = new File(location);
> + String properties = FileUtils.loadFileAsString(deployFile);
> + String deployProperties = "";
> +
> + for (String line : properties.split("\n")) {
> + if (line.contains("deployment.manifest.attributes.check")) {
> + if (foundAttributesCheck) {
> + deployProperties += attributesCheck + "\n";
> + }
> + } else {
> + deployProperties += line + "\n";
> + }
> + }
> +
> + FileUtils.saveFile(deployProperties, deployFile);
> + }
> +
> @Test
> @NeedsDisplay
> @TestInBrowsers(testIn={Browsers.one})
>
Great work!
J.
More information about the distro-pkg-dev
mailing list