diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectState.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectState.java index 9f6f6e71a8..a72fc1618d 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectState.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectState.java @@ -197,6 +197,7 @@ public class ProjectState { List all = new ArrayList(); Set seen = new HashSet(); + ProjectState allProjects = projectCache.getAllProjects(); seen.add(getProject().getNameKey()); ProjectState s = this; @@ -209,7 +210,9 @@ public class ProjectState { } s = projectCache.get(parent); } while (s != null); - all.addAll(projectCache.getAllProjects().getLocalAccessSections()); + if (seen.add(allProjects.getProject().getNameKey())) { + all.addAll(allProjects.getLocalAccessSections()); + } return all; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/SectionSortCache.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/SectionSortCache.java index c0d2034cf6..2bc957ebcf 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/SectionSortCache.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/SectionSortCache.java @@ -28,6 +28,8 @@ import com.google.inject.TypeLiteral; import com.google.inject.name.Named; import org.apache.commons.lang.StringUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.util.Arrays; import java.util.Collections; @@ -38,6 +40,9 @@ import java.util.List; /** Caches the order AccessSections should be sorted for evaluation. */ @Singleton public class SectionSortCache { + private static final Logger log = + LoggerFactory.getLogger(SectionSortCache.class); + private static final String CACHE_NAME = "permission_sort"; public static Module module() { @@ -79,10 +84,11 @@ public class SectionSortCache { } } else { + boolean poison = false; IdentityHashMap srcMap = new IdentityHashMap(); for (int i = 0; i < cnt; i++) { - srcMap.put(sections.get(i), i); + poison |= srcMap.put(sections.get(i), i) != null; } Collections.sort(sections, new MostSpecificComparator(ref)); @@ -97,7 +103,11 @@ public class SectionSortCache { } } - cache.put(key, new EntryVal(srcIdx)); + if (poison) { + log.error("Received duplicate AccessSection instances, not caching sort"); + } else { + cache.put(key, new EntryVal(srcIdx)); + } } } diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java index 29b27628c9..77775d8d15 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java @@ -145,6 +145,18 @@ public class RefControlTest extends TestCase { u.controlForRef("refs/heads/foobar").canUpload()); } + public void testInheritDuplicateSections() { + grant(parent, READ, admin, "refs/*"); + grant(local, READ, devs, "refs/heads/*"); + local.getProject().setParentName(parent.getProject().getName()); + assertTrue("a can read", user("a", admin).isVisible()); + + local = new ProjectConfig(new Project.NameKey("local")); + local.createInMemory(); + grant(local, READ, devs, "refs/*"); + assertTrue("d can read", user("d", devs).isVisible()); + } + public void testInheritRead_OverrideWithDeny() { grant(parent, READ, registered, "refs/*"); grant(local, READ, registered, "refs/*").setDeny(); @@ -321,7 +333,6 @@ public class RefControlTest extends TestCase { local = new ProjectConfig(new Project.NameKey("local")); local.createInMemory(); - local.getProject().setParentName(parent.getProject().getName()); sectionSorter = new PermissionCollection.Factory(