Fix permission backend extra query with parens
During the review the If144832047b4 the way the extra
query added by the permission backend was overlooked and,
in conjunction of a an empty query in the integration test
allowed the following ambiguity:
Query without permissions backend limitations
- author:self OR status:open => returns all open changes
or authored by the current user
Query with a permissions backend hiding a testHiddenProject
- author:self OR status:open -project:testHiddenProject
The above query is not doing what the permission backend
would expect, because is evaluated as follows:
- author:self OR (status:open AND -project:testHiddenProject)
The above query would return all changes authored by the
current user, including the ones of testHiddenProject.
Release-Notes: Fix evaluation of extra query provided by a permission backend
Change-Id: I94dea44d5c5f00a9a90c29136cf265d4759b2165
diff --git a/java/com/google/gerrit/server/restapi/change/QueryChanges.java b/java/com/google/gerrit/server/restapi/change/QueryChanges.java
index d05cbf6..2878fe2 100644
--- a/java/com/google/gerrit/server/restapi/change/QueryChanges.java
+++ b/java/com/google/gerrit/server/restapi/change/QueryChanges.java
@@ -179,7 +179,7 @@
}
for (int i = 0; i < queries.size(); i++) {
- queries.set(i, queries.get(i) + " " + queryFilter);
+ queries.set(i, "(" + queries.get(i) + ") AND (" + queryFilter + ")");
}
}
diff --git a/javatests/com/google/gerrit/acceptance/api/change/QueryChangesFilterPermissionBackendIT.java b/javatests/com/google/gerrit/acceptance/api/change/QueryChangesFilterPermissionBackendIT.java
index 07d32fe..a6c3c27 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/QueryChangesFilterPermissionBackendIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/QueryChangesFilterPermissionBackendIT.java
@@ -135,13 +135,14 @@
TestRepository<InMemoryRepository> hiddenRepo = cloneProject(hiddenProject, admin);
createChange(hiddenRepo);
- assertThat(gApi.changes().query().get()).hasSize(2);
+ String changeQuery = "author:self OR status:open";
+ assertThat(gApi.changes().query(changeQuery).get()).hasSize(2);
server
.getTestInjector()
.getInstance(TestPermissionBackend.class)
.setExtraQueryFilter("-project:" + hiddenProject);
- List<ChangeInfo> projectChanges = gApi.changes().query().get();
+ List<ChangeInfo> projectChanges = gApi.changes().query(changeQuery).get();
assertThat(projectChanges.stream().map(c -> c.changeId)).containsExactly(projectChangeId);
}
}
