From a918c73fb6938e79ccb714004540aa4252788a8e Mon Sep 17 00:00:00 2001 From: Thomas Watson Date: Wed, 9 May 2018 10:55:49 -0500 Subject: Bug 532194 - Use BundlePermissions for the EvaluationCacheKey Also check for null Conditions from a row and update testcase to handle when multiple conditions are used in a single row. If there is a mix of mutable and immutable conditions then the satisfied conditions are nulled out to avoid calling them again on future permission checkes. The method checkMutable needs to handle the null values. Change-Id: I9adf9cf54bfe571d53049d48ffb44e76bc78f121 Signed-off-by: Thomas Watson --- .../tests/securityadmin/SecurityAdminUnitTests.java | 19 +++++++++++++++++++ .../osgi/internal/permadmin/EvaluationCacheKey.java | 8 ++++---- .../osgi/internal/permadmin/SecurityTable.java | 2 +- 3 files changed, 24 insertions(+), 5 deletions(-) diff --git a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/securityadmin/SecurityAdminUnitTests.java b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/securityadmin/SecurityAdminUnitTests.java index 386add007..9dc2fc42f 100644 --- a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/securityadmin/SecurityAdminUnitTests.java +++ b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/securityadmin/SecurityAdminUnitTests.java @@ -57,6 +57,7 @@ public class SecurityAdminUnitTests extends AbstractBundleTests { private static final ConditionInfo[] ALLLOCATION_CONDS = new ConditionInfo[] {new ConditionInfo("org.osgi.service.condpermadmin.BundleLocationCondition", new String[] {"*"})}; //$NON-NLS-1$ //$NON-NLS-2$ private static final ConditionInfo MUT_SAT = new ConditionInfo("ext.framework.b.TestCondition", new String[] {"MUT_SAT", "true", "false", "true"}); //$NON-NLS-1$//$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$ //$NON-NLS-5$ + private static final ConditionInfo NOT_MUT_SAT = new ConditionInfo("ext.framework.b.TestCondition", new String[] {"NOT_MUT_SAT", "false", "false", "true"}); //$NON-NLS-1$//$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$ //$NON-NLS-5$ private static final ConditionInfo POST_MUT_SAT = new ConditionInfo("ext.framework.b.TestCondition", new String[] {"POST_MUT_SAT", "true", "true", "true"}); //$NON-NLS-1$//$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$ //$NON-NLS-5$ private static final ConditionInfo POST_MUT_UNSAT = new ConditionInfo("ext.framework.b.TestCondition", new String[] {"POST_MUT_UNSAT", "true", "true", "false"}); //$NON-NLS-1$//$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$ //$NON-NLS-5$ @@ -687,6 +688,24 @@ public class SecurityAdminUnitTests extends AbstractBundleTests { tc1sat.setSatisfied(false); testSMPermission(pds, new FilePermission("test", "read"), true); //$NON-NLS-1$ //$NON-NLS-2$ + + tc1sat.setSatisfied(true); + update = cpa.newConditionalPermissionUpdate(); + rows = update.getConditionalPermissionInfos(); + rows.clear(); + rows.add(cpa.newConditionalPermissionInfo(null, new ConditionInfo[] {NOT_MUT_SAT, MUT_SAT}, READONLY_INFOS, ConditionalPermissionInfo.DENY)); + rows.add(cpa.newConditionalPermissionInfo(null, ALLLOCATION_CONDS, READONLY_INFOS, ConditionalPermissionInfo.ALLOW)); + assertTrue("failed to commit", update.commit()); //$NON-NLS-1$); + + testSMPermission(pds, new FilePermission("test", "read"), false); //$NON-NLS-1$ //$NON-NLS-2$ + // test again to make sure we get the same result + testSMPermission(pds, new FilePermission("test", "read"), false); //$NON-NLS-1$ //$NON-NLS-2$ + // test with different file name + testSMPermission(pds, new FilePermission("test2", "read"), false); //$NON-NLS-1$ //$NON-NLS-2$ + + TestCondition tc2sat = TestCondition.getTestCondition("NOT_MUT_SAT_" + test1.getBundleId()); //$NON-NLS-1$ + assertNotNull("tc2sat", tc2sat); //$NON-NLS-1$ + } public void testAccessControlContext01() { diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/permadmin/EvaluationCacheKey.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/permadmin/EvaluationCacheKey.java index d0db72521..f59472545 100644 --- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/permadmin/EvaluationCacheKey.java +++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/permadmin/EvaluationCacheKey.java @@ -17,11 +17,11 @@ class EvaluationCacheKey { private final Permission permission; - private final Long bundleId; + private final BundlePermissions bundlePermissions; EvaluationCacheKey(BundlePermissions bundlePermissions, Permission permission) { this.permission = permission; - this.bundleId = bundlePermissions.getBundle().getBundleId(); + this.bundlePermissions = bundlePermissions; } @Override @@ -33,11 +33,11 @@ class EvaluationCacheKey { return false; } EvaluationCacheKey that = (EvaluationCacheKey) o; - return Objects.equals(bundleId, that.bundleId) && Objects.equals(permission, that.permission); + return bundlePermissions == that.bundlePermissions && Objects.equals(permission, that.permission); } @Override public int hashCode() { - return Objects.hash(bundleId, permission); + return Objects.hash(bundlePermissions, permission); } } diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/permadmin/SecurityTable.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/permadmin/SecurityTable.java index 6a93c53d2..a084b889b 100644 --- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/permadmin/SecurityTable.java +++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/permadmin/SecurityTable.java @@ -111,7 +111,7 @@ public class SecurityTable extends PermissionCollection { Condition[] conditions = row.getConditions(bundlePermissions); if (conditions != null) { for (Condition condition : conditions) { - if (condition.isMutable()) { + if (condition != null && condition.isMutable()) { evaluationCache.put(evaluationCacheKey, MUTABLE); return true; } -- cgit v1.2.3