From a8bce41876258fc12af65eca2342d1608e639951 Mon Sep 17 00:00:00 2001
From: Luke Taylor <luke.taylor@springsource.com>
Date: Fri, 19 Aug 2011 11:45:34 -0700
Subject: [PATCH] SEC-1795: Fix possible NPEs in AclImpl.equals()

---
 .../springframework/security/acls/domain/AclImpl.java  | 10 +++++-----
 .../security/acls/domain/AclImplTests.java             | 10 ++++++++++
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/acl/src/main/java/org/springframework/security/acls/domain/AclImpl.java b/acl/src/main/java/org/springframework/security/acls/domain/AclImpl.java
index 38687b1634..7a27f31588 100644
--- a/acl/src/main/java/org/springframework/security/acls/domain/AclImpl.java
+++ b/acl/src/main/java/org/springframework/security/acls/domain/AclImpl.java
@@ -379,15 +379,15 @@ public class AclImpl implements Acl, MutableAcl, AuditableAcl, OwnershipAcl {
         if (obj instanceof AclImpl) {
             AclImpl rhs = (AclImpl) obj;
             if (this.aces.equals(rhs.aces)) {
-                if ((this.parentAcl == null && rhs.parentAcl == null) || (this.parentAcl.equals(rhs.parentAcl))) {
-                    if ((this.objectIdentity == null && rhs.objectIdentity == null) || (this.objectIdentity.equals(rhs.objectIdentity))) {
-                        if ((this.id == null && rhs.id == null) || (this.id.equals(rhs.id))) {
-                            if ((this.owner == null && rhs.owner == null) || this.owner.equals(rhs.owner)) {
+                if ((this.parentAcl == null && rhs.parentAcl == null) || (this.parentAcl !=null && this.parentAcl.equals(rhs.parentAcl))) {
+                    if ((this.objectIdentity == null && rhs.objectIdentity == null) || (this.objectIdentity != null && this.objectIdentity.equals(rhs.objectIdentity))) {
+                        if ((this.id == null && rhs.id == null) || (this.id != null && this.id.equals(rhs.id))) {
+                            if ((this.owner == null && rhs.owner == null) || (this.owner != null && this.owner.equals(rhs.owner))) {
                                 if (this.entriesInheriting == rhs.entriesInheriting) {
                                     if ((this.loadedSids == null && rhs.loadedSids == null)) {
                                         return true;
                                     }
-                                    if (this.loadedSids.size() == rhs.loadedSids.size()) {
+                                    if (this.loadedSids != null && (this.loadedSids.size() == rhs.loadedSids.size())) {
                                         for (int i = 0; i < this.loadedSids.size(); i++) {
                                             if (!this.loadedSids.get(i).equals(rhs.loadedSids.get(i))) {
                                                 return false;
diff --git a/acl/src/test/java/org/springframework/security/acls/domain/AclImplTests.java b/acl/src/test/java/org/springframework/security/acls/domain/AclImplTests.java
index 413b4d5e1c..4fbb54b57a 100644
--- a/acl/src/test/java/org/springframework/security/acls/domain/AclImplTests.java
+++ b/acl/src/test/java/org/springframework/security/acls/domain/AclImplTests.java
@@ -504,6 +504,16 @@ public class AclImplTests {
         acl.deleteAce(1);
     }
 
+    // SEC-1795
+    @Test
+    public void changingParentIsSuccessful() throws Exception {
+        AclImpl parentAcl = new AclImpl(objectIdentity, 1L, mockAuthzStrategy, mockAuditLogger);
+        AclImpl childAcl = new AclImpl(objectIdentity, 2L, mockAuthzStrategy, mockAuditLogger);
+        AclImpl changeParentAcl = new AclImpl(objectIdentity, 3L, mockAuthzStrategy, mockAuditLogger);
+
+        childAcl.setParent(parentAcl);
+        childAcl.setParent(changeParentAcl);
+    }
 
     //~ Inner Classes ==================================================================================================