Merge branch 'stable-2.16' into stable-3.0

* stable-2.16:
  VersionMetaData: Don't close passed in RevWalk
  AccessIT: Add tests for when group appears twice for same rule
  Prevent infinite loops with GWT UI and HTTP auth
  Change bouncycastle urls
  ListAccess: Fix incorrect behavior when group appears twice for same rule

Change-Id: Ie13fb6b21cf3478ecd9321bfc97c52f74416a4cf

4 files changed, 101 insertions, 3 deletions

diff --git a/java/com/google/gerrit/server/git/meta/VersionedMetaData.java b/java/com/google/gerrit/server/git/meta/VersionedMetaData.java
index a4cfcbfb5b..5cda23808f 100644
--- a/java/com/google/gerrit/server/git/meta/VersionedMetaData.java
+++ b/java/com/google/gerrit/server/git/meta/VersionedMetaData.java
@@ -424,7 +424,10 @@ public abstract class VersionedMetaData {
       public void close() {
         newTree = null;
 
-        rw.close();
+        if (revWalk == null) {
+          rw.close();
+        }
+
         if (objInserter == null && inserter != null) {
           inserter.close();
           inserter = null;
diff --git a/java/com/google/gerrit/server/restapi/project/GetAccess.java b/java/com/google/gerrit/server/restapi/project/GetAccess.java
index d6c07ddf0e..171c2e5f6d 100644
--- a/java/com/google/gerrit/server/restapi/project/GetAccess.java
+++ b/java/com/google/gerrit/server/restapi/project/GetAccess.java
@@ -332,7 +332,7 @@ public class GetAccess implements RestReadView<ProjectResource> {
         }
         AccountGroup.UUID group = r.getGroup().getUUID();
         if (group != null) {
-          pInfo.rules.put(group.get(), info);
+          pInfo.rules.putIfAbsent(group.get(), info); // First entry for the group wins
           loadGroup(groups, group);
         }
       }
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java b/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java
index 72af07583e..eac911db9f 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java
@@ -17,6 +17,9 @@ import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.truth.Truth8.assertThat;
 import static com.google.gerrit.extensions.client.ListChangesOption.MESSAGES;
 import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
+import static java.util.Arrays.asList;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
 
 import com.google.gerrit.acceptance.AbstractDaemonTest;
 import com.google.gerrit.acceptance.GitUtil;
@@ -25,6 +28,7 @@ import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
 import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
 import com.google.gerrit.common.data.AccessSection;
 import com.google.gerrit.common.data.GlobalCapability;
+import com.google.gerrit.common.data.GroupReference;
 import com.google.gerrit.common.data.Permission;
 import com.google.gerrit.extensions.api.access.AccessSectionInfo;
 import com.google.gerrit.extensions.api.access.PermissionInfo;
@@ -51,6 +55,7 @@ import com.google.gerrit.server.group.SystemGroupBackend;
 import com.google.gerrit.server.project.ProjectConfig;
 import com.google.inject.Inject;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
 import org.eclipse.jgit.junit.TestRepository;
@@ -673,6 +678,90 @@ public class AccessIT extends AbstractDaemonTest {
     return gApi.projects().name(newProjectName.get());
   }
 
+  @Test
+  public void grantAllowAndDenyForSameGroup() throws Exception {
+    GroupReference registeredUsers = systemGroupBackend.getGroup(REGISTERED_USERS);
+    String access = "access";
+    List<String> allowThenDeny =
+        asList(registeredUsers.toConfigValue(), "deny " + registeredUsers.toConfigValue());
+    // Clone repository to forcefully add permission
+    TestRepository<InMemoryRepository> allProjectsRepo = cloneProject(allProjects, admin);
+
+    // Fetch permission ref
+    GitUtil.fetch(allProjectsRepo, "refs/meta/config:cfg");
+    allProjectsRepo.reset("cfg");
+
+    // Load current permissions
+    String config =
+        gApi.projects()
+            .name(allProjects.get())
+            .branch(RefNames.REFS_CONFIG)
+            .file(ProjectConfig.PROJECT_CONFIG)
+            .asString();
+
+    // Append and push allowThenDeny permissions
+    Config cfg = new Config();
+    cfg.fromText(config);
+    cfg.setStringList(access, AccessSection.HEADS, Permission.READ, allowThenDeny);
+    config = cfg.toText();
+    PushOneCommit push =
+        pushFactory.create(
+            db, admin.getIdent(), allProjectsRepo, "Subject", ProjectConfig.PROJECT_CONFIG, config);
+    push.to(RefNames.REFS_CONFIG).assertOkStatus();
+
+    ProjectAccessInfo pai = gApi.projects().name(allProjects.get()).access();
+    Map<String, AccessSectionInfo> local = pai.local;
+    AccessSectionInfo heads = local.get(AccessSection.HEADS);
+    Map<String, PermissionInfo> permissions = heads.permissions;
+    PermissionInfo read = permissions.get(Permission.READ);
+    Map<String, PermissionRuleInfo> rules = read.rules;
+    assertEquals(
+        rules.get(registeredUsers.getUUID().get()),
+        new PermissionRuleInfo(PermissionRuleInfo.Action.ALLOW, false));
+  }
+
+  @Test
+  public void grantDenyAndAllowForSameGroup() throws Exception {
+    GroupReference registeredUsers = systemGroupBackend.getGroup(REGISTERED_USERS);
+    String access = "access";
+    List<String> denyThenAllow =
+        asList("deny " + registeredUsers.toConfigValue(), registeredUsers.toConfigValue());
+    // Clone repository to forcefully add permission
+    TestRepository<InMemoryRepository> allProjectsRepo = cloneProject(allProjects, admin);
+
+    // Fetch permission ref
+    GitUtil.fetch(allProjectsRepo, "refs/meta/config:cfg");
+    allProjectsRepo.reset("cfg");
+
+    // Load current permissions
+    String config =
+        gApi.projects()
+            .name(allProjects.get())
+            .branch(RefNames.REFS_CONFIG)
+            .file(ProjectConfig.PROJECT_CONFIG)
+            .asString();
+
+    // Append and push denyThenAllow permissions
+    Config cfg = new Config();
+    cfg.fromText(config);
+    cfg.setStringList(access, AccessSection.HEADS, Permission.READ, denyThenAllow);
+    config = cfg.toText();
+    PushOneCommit push =
+        pushFactory.create(
+            db, admin.getIdent(), allProjectsRepo, "Subject", ProjectConfig.PROJECT_CONFIG, config);
+    push.to(RefNames.REFS_CONFIG).assertOkStatus();
+
+    ProjectAccessInfo pai = gApi.projects().name(allProjects.get()).access();
+    Map<String, AccessSectionInfo> local = pai.local;
+    AccessSectionInfo heads = local.get(AccessSection.HEADS);
+    Map<String, PermissionInfo> permissions = heads.permissions;
+    PermissionInfo read = permissions.get(Permission.READ);
+    Map<String, PermissionRuleInfo> rules = read.rules;
+    assertEquals(
+        rules.get(registeredUsers.getUUID().get()),
+        new PermissionRuleInfo(PermissionRuleInfo.Action.DENY, false));
+  }
+
   private ProjectAccessInput newProjectAccessInput() {
     ProjectAccessInput p = new ProjectAccessInput();
     p.add = new HashMap<>();
diff --git a/resources/com/google/gerrit/httpd/auth/container/LoginRedirect.html b/resources/com/google/gerrit/httpd/auth/container/LoginRedirect.html
index 0567468e8c..54c366148d 100644
--- a/resources/com/google/gerrit/httpd/auth/container/LoginRedirect.html
+++ b/resources/com/google/gerrit/httpd/auth/container/LoginRedirect.html
@@ -4,6 +4,12 @@
     <title>Gerrit Code Review</title>
     <script type="text/javascript">
       var href = window.location.href;
+      var query = "";
+      var q = href.indexOf('?');
+      if (q >= 0) {
+        query = href.substring(q);
+        href = href.substring(0,q);
+      }
       var p = href.indexOf('#');
       var token;
       if (p >= 0) {
@@ -12,7 +18,7 @@
       } else {
         token = '';
       }
-      window.location.replace(href + 'login/' + token);
+      window.location.replace(href + 'login/' + token + query);
     </script>
   </head>
   <body>