Return-Path: X-Original-To: apmail-jackrabbit-oak-dev-archive@minotaur.apache.org Delivered-To: apmail-jackrabbit-oak-dev-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 6816A10897 for ; Tue, 14 Jan 2014 18:24:04 +0000 (UTC) Received: (qmail 11839 invoked by uid 500); 14 Jan 2014 18:24:03 -0000 Delivered-To: apmail-jackrabbit-oak-dev-archive@jackrabbit.apache.org Received: (qmail 11733 invoked by uid 500); 14 Jan 2014 18:24:03 -0000 Mailing-List: contact oak-dev-help@jackrabbit.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: oak-dev@jackrabbit.apache.org Delivered-To: mailing list oak-dev@jackrabbit.apache.org Received: (qmail 11725 invoked by uid 99); 14 Jan 2014 18:24:03 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 14 Jan 2014 18:24:03 +0000 X-ASF-Spam-Status: No, hits=-0.0 required=5.0 tests=RCVD_IN_DNSWL_NONE,SPF_HELO_PASS X-Spam-Check-By: apache.org Received-SPF: unknown (athena.apache.org: error in processing during lookup of anchela@adobe.com) Received: from [207.46.163.189] (HELO na01-bn1-obe.outbound.protection.outlook.com) (207.46.163.189) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 14 Jan 2014 18:23:57 +0000 Received: from BLUPR02MB328.namprd02.prod.outlook.com (10.141.77.151) by BLUPR02MB325.namprd02.prod.outlook.com (10.141.77.142) with Microsoft SMTP Server (TLS) id 15.0.842.7; Tue, 14 Jan 2014 18:23:35 +0000 Received: from BLUPR02MB328.namprd02.prod.outlook.com ([10.141.77.151]) by BLUPR02MB328.namprd02.prod.outlook.com ([10.141.77.151]) with mapi id 15.00.0842.003; Tue, 14 Jan 2014 18:23:35 +0000 From: Angela Schreiber To: "oak-dev@jackrabbit.apache.org" Subject: Re: svn commit: r1552419 - in /jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol: AccessControlValidator.java AccessControlValidatorProvider.java Thread-Topic: svn commit: r1552419 - in /jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol: AccessControlValidator.java AccessControlValidatorProvider.java Thread-Index: AQHO/PnrzkY+U34kLkSmP6KiaEjvn5qEwkOA Date: Tue, 14 Jan 2014 18:23:34 +0000 Message-ID: References: <20131219203400.43E0123889E7@eris.apache.org> In-Reply-To: <20131219203400.43E0123889E7@eris.apache.org> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: user-agent: Microsoft-MacOutlook/14.3.9.131030 x-originating-ip: [192.147.117.11] x-forefront-prvs: 0091C8F1EB x-forefront-antispam-report: SFV:NSPM;SFS:(10019001)(679001)(779001)(689001)(51704005)(189002)(199002)(479174003)(24454002)(54356001)(53806001)(49866001)(79102001)(54316002)(50986001)(46102001)(63696002)(66066001)(47976001)(65816001)(19580405001)(47736001)(83322001)(19580395003)(51856001)(80976001)(81816001)(92726001)(92566001)(36756003)(93136001)(76786001)(76796001)(56776001)(76482001)(77982001)(74876001)(87936001)(4396001)(81686001)(31966008)(87266001)(80022001)(81342001)(56816005)(85306002)(59766001)(69226001)(74662001)(2656002)(81542001)(83072002)(15975445006)(15202345003)(90146001)(47446002)(74502001)(74706001)(83506001)(85852003)(74366001)(42262001);DIR:OUT;SFP:1102;SCL:1;SRVR:BLUPR02MB325;H:BLUPR02MB328.namprd02.prod.outlook.com;CLIP:192.147.117.11;FPR:;RD:InfoNoRecords;MX:1;A:1;LANG:en; Content-Type: text/plain; charset="us-ascii" Content-ID: <402B704EB27D3C49B97783952B5DF28B@namprd02.prod.outlook.com> Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: adobe.com X-Virus-Checked: Checked by ClamAV on apache.org hi jukka if am not mistaken this change introduces causes the following line to compare different objects with equals: the following line used to compare String with String but now compares String.equals(TypePredicate) in the AccessControlValidator: line 204 if=20 (MIX_REP_REPO_ACCESS_CONTROLLABLE.equals(requiredMixin)) { i don't know how to get the requiredMixin string out of the predicate. so, maybe you can fix that right away? thanks angela On 19/12/13 21:34, "jukka@apache.org" wrote: >Author: jukka >Date: Thu Dec 19 20:33:59 2013 >New Revision: 1552419 > >URL: http://svn.apache.org/r1552419 >Log: >OAK-1296: Use TypePredicate instead of NodeType.isNodeType() for >NodeState type checks > >Use TypePredicate in AccessControlValidator > >Modified: > =20 >jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu >rity/authorization/accesscontrol/AccessControlValidator.java > =20 >jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu >rity/authorization/accesscontrol/AccessControlValidatorProvider.java > >Modified:=20 >jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu >rity/authorization/accesscontrol/AccessControlValidator.java >URL:=20 >http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/o >rg/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessContro >lValidator.java?rev=3D1552419&r1=3D1552418&r2=3D1552419&view=3Ddiff >=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >=3D=3D=3D=3D >---=20 >jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu >rity/authorization/accesscontrol/AccessControlValidator.java (original) >+++=20 >jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu >rity/authorization/accesscontrol/AccessControlValidator.java Thu Dec 19 >20:33:59 2013 >@@ -35,7 +35,8 @@ import org.apache.jackrabbit.oak.api.Pro > import org.apache.jackrabbit.oak.api.Tree; > import org.apache.jackrabbit.oak.api.Type; > import org.apache.jackrabbit.oak.core.AbstractTree; >-import=20 >org.apache.jackrabbit.oak.plugins.nodetype.ReadOnlyNodeTypeManager; >+import org.apache.jackrabbit.oak.core.ImmutableTree; >+import org.apache.jackrabbit.oak.plugins.nodetype.TypePredicate; > import org.apache.jackrabbit.oak.spi.commit.DefaultValidator; > import org.apache.jackrabbit.oak.spi.commit.Validator; > import=20 >org.apache.jackrabbit.oak.spi.security.authorization.accesscontrol.AccessC >ontrolConstants; >@@ -57,25 +58,35 @@ import static org.apache.jackrabbit.oak. > */ > class AccessControlValidator extends DefaultValidator implements >AccessControlConstants { >=20 >- private final Tree parentBefore; >- private final Tree parentAfter; >+ private final ImmutableTree parentAfter; >=20 > private final PrivilegeBitsProvider privilegeBitsProvider; > private final PrivilegeManager privilegeManager; > private final RestrictionProvider restrictionProvider; >- private final ReadOnlyNodeTypeManager ntMgr; >=20 >- AccessControlValidator(Tree parentBefore, Tree parentAfter, >+ private final TypePredicate isRepoAccessControllable; >+ private final TypePredicate isAccessControllable; >+ >+ AccessControlValidator(ImmutableTree parentAfter, > PrivilegeManager privilegeManager, > PrivilegeBitsProvider privilegeBitsProvider, >- RestrictionProvider restrictionProvider, >- ReadOnlyNodeTypeManager ntMgr) { >- this.parentBefore =3D parentBefore; >+ RestrictionProvider restrictionProvider) { > this.parentAfter =3D parentAfter; > this.privilegeBitsProvider =3D privilegeBitsProvider; > this.privilegeManager =3D privilegeManager; > this.restrictionProvider =3D restrictionProvider; >- this.ntMgr =3D ntMgr; >+ this.isRepoAccessControllable =3D new >TypePredicate(parentAfter.getNodeState(), >MIX_REP_REPO_ACCESS_CONTROLLABLE); >+ this.isAccessControllable =3D new >TypePredicate(parentAfter.getNodeState(), MIX_REP_ACCESS_CONTROLLABLE); >+ } >+ >+ private AccessControlValidator( >+ AccessControlValidator parent, ImmutableTree parentAfter) { >+ this.parentAfter =3D parentAfter; >+ this.privilegeBitsProvider =3D parent.privilegeBitsProvider; >+ this.privilegeManager =3D parent.privilegeManager; >+ this.restrictionProvider =3D parent.restrictionProvider; >+ this.isRepoAccessControllable =3D parent.isRepoAccessControllable= ; >+ this.isAccessControllable =3D parent.isAccessControllable; > } >=20 > //----------------------------------------------------------< >Validator >--- >@@ -106,19 +117,18 @@ class AccessControlValidator extends Def >=20 > @Override > public Validator childNodeAdded(String name, NodeState after) throws >CommitFailedException { >- Tree treeAfter =3D checkNotNull(parentAfter.getChild(name)); >+ ImmutableTree treeAfter =3D >checkNotNull(parentAfter.getChild(name)); >=20 > checkValidTree(parentAfter, treeAfter, after); >- return new AccessControlValidator(null, treeAfter, >privilegeManager, privilegeBitsProvider, restrictionProvider, ntMgr); >+ return new AccessControlValidator(this, treeAfter); > } >=20 > @Override > public Validator childNodeChanged(String name, NodeState before, >NodeState after) throws CommitFailedException { >- Tree treeBefore =3D checkNotNull(parentBefore.getChild(name)); >- Tree treeAfter =3D checkNotNull(parentAfter.getChild(name)); >+ ImmutableTree treeAfter =3D >checkNotNull(parentAfter.getChild(name)); >=20 > checkValidTree(parentAfter, treeAfter, after); >- return new AccessControlValidator(treeBefore, treeAfter, >privilegeManager, privilegeBitsProvider, restrictionProvider, ntMgr); >+ return new AccessControlValidator(this, treeAfter); > } >=20 > @Override >@@ -129,7 +139,7 @@ class AccessControlValidator extends Def >=20 > //------------------------------------------------------------< >private >--- >=20 >- private void checkValidTree(Tree parentAfter, Tree treeAfter, >NodeState nodeAfter) throws CommitFailedException { >+ private void checkValidTree(ImmutableTree parentAfter, Tree >treeAfter, NodeState nodeAfter) throws CommitFailedException { > if (isPolicy(treeAfter)) { > checkValidPolicy(parentAfter, treeAfter, nodeAfter); > } else if (isAccessControlEntry(treeAfter)) { >@@ -155,11 +165,10 @@ class AccessControlValidator extends Def > } > } >=20 >- private void checkValidPolicy(Tree parent, Tree policyTree, >NodeState policyNode) throws CommitFailedException { >- String mixinType =3D >(REP_REPO_POLICY.equals(policyTree.getName())) ? >- MIX_REP_REPO_ACCESS_CONTROLLABLE : >- MIX_REP_ACCESS_CONTROLLABLE; >- checkValidAccessControlledNode(parent, mixinType); >+ private void checkValidPolicy(ImmutableTree parent, Tree policyTree, >NodeState policyNode) throws CommitFailedException { >+ TypePredicate requiredMixin =3D >(REP_REPO_POLICY.equals(policyTree.getName())) ? >+ isRepoAccessControllable : isAccessControllable; >+ checkValidAccessControlledNode(parent, requiredMixin); >=20 > Collection validPolicyNames =3D (parent.isRoot()) ? > POLICY_NODE_NAMES : >@@ -182,13 +191,13 @@ class AccessControlValidator extends Def > } > } >=20 >- private void checkValidAccessControlledNode(Tree >accessControlledTree, String requiredMixin) throws CommitFailedException { >+ private void checkValidAccessControlledNode(ImmutableTree >accessControlledTree, TypePredicate requiredMixin) throws >CommitFailedException { > if=20 >(AC_NODETYPE_NAMES.contains(TreeUtil.getPrimaryTypeName(accessControlledTr >ee))) { > throw accessViolation(5, "Access control policy within >access control content (" + accessControlledTree.getPath() + ')'); > } >=20 >- String msg =3D "Isolated policy node. Parent is not of type " + >requiredMixin; >- if (!ntMgr.isNodeType(accessControlledTree, requiredMixin)) { >+ if (!requiredMixin.apply(accessControlledTree.getNodeState())) { >+ String msg =3D "Isolated policy node. Parent is not of type " >+ requiredMixin; > throw accessViolation(6, msg); > } >=20 > >Modified:=20 >jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu >rity/authorization/accesscontrol/AccessControlValidatorProvider.java >URL:=20 >http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/o >rg/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessContro >lValidatorProvider.java?rev=3D1552419&r1=3D1552418&r2=3D1552419&view=3Ddif= f >=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >=3D=3D=3D=3D >---=20 >jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu >rity/authorization/accesscontrol/AccessControlValidatorProvider.java >(original) >+++=20 >jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu >rity/authorization/accesscontrol/AccessControlValidatorProvider.java Thu >Dec 19 20:33:59 2013 >@@ -20,11 +20,9 @@ import javax.annotation.Nonnull; >=20 > import org.apache.jackrabbit.api.security.authorization.PrivilegeManager; > import org.apache.jackrabbit.oak.api.Root; >-import org.apache.jackrabbit.oak.api.Tree; > import org.apache.jackrabbit.oak.core.ImmutableRoot; > import org.apache.jackrabbit.oak.core.ImmutableTree; > import org.apache.jackrabbit.oak.namepath.NamePathMapper; >-import=20 >org.apache.jackrabbit.oak.plugins.nodetype.ReadOnlyNodeTypeManager; > import org.apache.jackrabbit.oak.spi.commit.Validator; > import org.apache.jackrabbit.oak.spi.commit.ValidatorProvider; > import org.apache.jackrabbit.oak.spi.security.SecurityProvider; >@@ -52,17 +50,15 @@ public class AccessControlValidatorProvi > @Nonnull > @Override > public Validator getRootValidator(NodeState before, NodeState after) >{ >- Tree rootBefore =3D new ImmutableTree(before); >- Tree rootAfter =3D new ImmutableTree(after); >+ ImmutableTree rootAfter =3D new ImmutableTree(after); >=20 > RestrictionProvider restrictionProvider =3D >getConfig(AuthorizationConfiguration.class).getRestrictionProvider(); >=20 > Root root =3D new ImmutableRoot(before); > PrivilegeManager privilegeManager =3D >getConfig(PrivilegeConfiguration.class).getPrivilegeManager(root, >NamePathMapper.DEFAULT); > PrivilegeBitsProvider privilegeBitsProvider =3D new >PrivilegeBitsProvider(root); >- ReadOnlyNodeTypeManager ntMgr =3D >ReadOnlyNodeTypeManager.getInstance(before); >=20 >- return new AccessControlValidator(rootBefore, rootAfter, >privilegeManager, privilegeBitsProvider, restrictionProvider, ntMgr); >+ return new AccessControlValidator(rootAfter, privilegeManager, >privilegeBitsProvider, restrictionProvider); > } >=20 > private T getConfig(Class configClass) { > >