Merge "Sidebar UX improvements"
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 48b572a..91a1117 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -1450,6 +1450,16 @@
+
By default 100,000.
+[[change.maxFiles]]change.maxFileSizeDownload::
++
+The link:rest-api-changes.html#get-content[GetContent] and
+link:rest-api-changes.html#get-safe-content[DownloadContent] REST APIs will
+refuse to load files larger than this limit (in bytes). 0 or negative values
+mean "unlimited".
+
++
+By default 0 (unlimited).
+
[[change.maxPatchSets]]change.maxPatchSets::
+
Maximum number of patch sets allowed per change. If this is insufficient,
diff --git a/java/com/google/gerrit/server/change/FileContentUtil.java b/java/com/google/gerrit/server/change/FileContentUtil.java
index c54b902..4c0eb69 100644
--- a/java/com/google/gerrit/server/change/FileContentUtil.java
+++ b/java/com/google/gerrit/server/change/FileContentUtil.java
@@ -25,6 +25,7 @@
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.BinaryResult;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
+import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.mime.FileTypeRegistry;
import com.google.gerrit.server.project.ProjectState;
@@ -39,6 +40,7 @@
import java.util.zip.ZipOutputStream;
import org.eclipse.jgit.errors.LargeObjectException;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
+import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectLoader;
@@ -60,11 +62,15 @@
private final GitRepositoryManager repoManager;
private final FileTypeRegistry registry;
+ private final long maxFileSizeBytes;
@Inject
- FileContentUtil(GitRepositoryManager repoManager, FileTypeRegistry ftr) {
+ FileContentUtil(
+ GitRepositoryManager repoManager, FileTypeRegistry ftr, @GerritServerConfig Config config) {
this.repoManager = repoManager;
this.registry = ftr;
+ long maxFileSizeDownload = config.getLong("change", null, "maxFileSizeDownload", 0);
+ this.maxFileSizeBytes = maxFileSizeDownload > 0 ? maxFileSizeDownload : Long.MAX_VALUE;
}
/**
@@ -117,6 +123,7 @@
}
ObjectLoader obj = repo.open(id, OBJ_BLOB);
+ checkMaxFileSizeBytes(obj);
byte[] raw;
try {
raw = obj.getCachedBytes(MAX_SIZE);
@@ -154,7 +161,7 @@
public BinaryResult downloadContent(
ProjectState project, ObjectId revstr, String path, @Nullable Integer parent)
- throws ResourceNotFoundException, IOException {
+ throws ResourceNotFoundException, IOException, BadRequestException {
try (Repository repo = openRepository(project);
RevWalk rw = new RevWalk(repo)) {
String suffix = "new";
@@ -179,6 +186,8 @@
ObjectId id = tw.getObjectId(0);
ObjectLoader obj = repo.open(id, OBJ_BLOB);
+ checkMaxFileSizeBytes(obj);
+
byte[] raw;
try {
raw = obj.getCachedBytes(MAX_SIZE);
@@ -194,6 +203,16 @@
}
}
+ private void checkMaxFileSizeBytes(ObjectLoader obj) throws BadRequestException {
+ if (obj.getSize() > this.maxFileSizeBytes) {
+ throw new BadRequestException(
+ String.format(
+ "File too big. File size: %d bytes. Configured 'maxFileSizeDownload' limit: %d"
+ + " bytes.",
+ obj.getSize(), this.maxFileSizeBytes));
+ }
+ }
+
private BinaryResult wrapBlob(
String path,
final ObjectLoader obj,
diff --git a/java/com/google/gerrit/server/config/GerritGlobalModule.java b/java/com/google/gerrit/server/config/GerritGlobalModule.java
index 7086447..7a50bdd 100644
--- a/java/com/google/gerrit/server/config/GerritGlobalModule.java
+++ b/java/com/google/gerrit/server/config/GerritGlobalModule.java
@@ -224,6 +224,7 @@
import com.google.gerrit.server.util.IdGenerator;
import com.google.gerrit.server.util.ThreadLocalRequestContext;
import com.google.gerrit.server.validators.AccountActivationValidationListener;
+import com.google.gerrit.server.validators.CustomKeyedValueValidationListener;
import com.google.gerrit.server.validators.GroupCreationValidationListener;
import com.google.gerrit.server.validators.HashtagValidationListener;
import com.google.gerrit.server.validators.OutgoingEmailValidationListener;
@@ -420,6 +421,7 @@
DynamicSet.setOf(binder(), HashtagValidationListener.class);
DynamicSet.setOf(binder(), OutgoingEmailValidationListener.class);
DynamicSet.setOf(binder(), AccountActivationValidationListener.class);
+ DynamicSet.setOf(binder(), CustomKeyedValueValidationListener.class);
DynamicItem.itemOf(binder(), AvatarProvider.class);
DynamicSet.setOf(binder(), LifecycleListener.class);
DynamicSet.setOf(binder(), TopMenu.class);
diff --git a/java/com/google/gerrit/server/restapi/change/DownloadContent.java b/java/com/google/gerrit/server/restapi/change/DownloadContent.java
index 74c0acc..ae801b4 100644
--- a/java/com/google/gerrit/server/restapi/change/DownloadContent.java
+++ b/java/com/google/gerrit/server/restapi/change/DownloadContent.java
@@ -16,6 +16,7 @@
import static com.google.gerrit.server.project.ProjectCache.illegalState;
+import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.BinaryResult;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.Response;
@@ -44,7 +45,7 @@
@Override
public Response<BinaryResult> apply(FileResource rsrc)
- throws ResourceNotFoundException, IOException, NoSuchChangeException {
+ throws BadRequestException, ResourceNotFoundException, IOException, NoSuchChangeException {
String path = rsrc.getPatchKey().fileName();
RevisionResource rev = rsrc.getRevision();
return Response.ok(
diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
index cc7712b..ef7a14c 100644
--- a/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
@@ -1718,6 +1718,34 @@
}
@Test
+ @GerritConfig(name = "change.maxFileSizeDownload", value = "10")
+ public void content_maxFileSizeDownload() throws Exception {
+ Map<String, String> files =
+ ImmutableMap.of("dir/file1.txt", " 9 bytes ", "dir/file2.txt", "11 bytes xx");
+ PushOneCommit.Result result =
+ pushFactory.create(admin.newIdent(), testRepo, SUBJECT, files).to("refs/for/master");
+ result.assertOkStatus();
+
+ // 9 bytes should be fine, because the limit is 10 bytes.
+ assertContent(result, "dir/file1.txt", " 9 bytes ");
+
+ // 11 bytes should throw, because the limit is 10 bytes.
+ BadRequestException exception =
+ assertThrows(
+ BadRequestException.class,
+ () ->
+ gApi.changes()
+ .id(result.getChangeId())
+ .revision(result.getCommit().name())
+ .file("dir/file2.txt")
+ .content());
+ assertThat(exception)
+ .hasMessageThat()
+ .isEqualTo(
+ "File too big. File size: 11 bytes. Configured 'maxFileSizeDownload' limit: 10 bytes.");
+ }
+
+ @Test
public void patchsetLevelContentDoesNotExist() throws Exception {
PushOneCommit.Result change = createChange();
assertThrows(
diff --git a/javatests/com/google/gerrit/server/query/project/AbstractQueryProjectsTest.java b/javatests/com/google/gerrit/server/query/project/AbstractQueryProjectsTest.java
index b119104..4402249 100644
--- a/javatests/com/google/gerrit/server/query/project/AbstractQueryProjectsTest.java
+++ b/javatests/com/google/gerrit/server/query/project/AbstractQueryProjectsTest.java
@@ -23,7 +23,6 @@
import com.google.common.base.CharMatcher;
import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.ImmutableSet;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Project;
@@ -69,13 +68,19 @@
import java.util.Iterator;
import java.util.List;
import java.util.Locale;
-import java.util.Set;
import org.junit.After;
import org.junit.Before;
import org.junit.Ignore;
import org.junit.Rule;
import org.junit.Test;
+/**
+ * Tests queries against the project index.
+ *
+ * <p>Note, returned projects are sorted by name. Projects that start with a capital letter are
+ * returned first. {@code InMemoryModule} sets that name for the All-Users project to {@code
+ * Test-Projects}, this is why it's returned after {@code Alll-Users}.
+ */
@Ignore
public abstract class AbstractQueryProjectsTest extends GerritServerTests {
@Rule public final GerritTestName testName = new GerritTestName();
@@ -112,6 +117,8 @@
protected Injector injector;
protected AccountInfo currentUserInfo;
protected CurrentUser user;
+ protected ProjectInfo allProjectsInfo;
+ protected ProjectInfo allUsersInfo;
protected abstract Injector createInjector();
@@ -141,6 +148,13 @@
user = userFactory.create(userId);
requestContext.setContext(newRequestContext(userId));
currentUserInfo = gApi.accounts().id(userId.get()).get();
+
+ // All-Projects and All-Users are not indexed, index them now.
+ gApi.projects().name(allProjects.get()).index(/* indexChildren= */ false);
+ gApi.projects().name(allUsers.get()).index(/* indexChildren= */ false);
+
+ allProjectsInfo = gApi.projects().name(allProjects.get()).get();
+ allUsersInfo = gApi.projects().name(allUsers.get()).get();
}
protected void initAfterLifecycleStart() throws Exception {}
@@ -170,6 +184,8 @@
ProjectInfo project = createProject(name("project"));
assertQuery("name:" + project.name, project);
+ assertQuery("name:" + allProjects.get(), allProjectsInfo);
+ assertQuery("name:" + allUsers.get(), allUsersInfo);
// only exact match
ProjectInfo projectWithHyphen = createProject(name("project-with-hyphen"));
@@ -188,12 +204,15 @@
@Test
public void byParentOfAllProjects() throws Exception {
- Set<String> excludedProjects = ImmutableSet.of(allProjects.get(), allUsers.get());
- ProjectInfo[] projects =
- gApi.projects().list().get().stream()
- .filter(p -> !excludedProjects.contains(p.name))
- .toArray(s -> new ProjectInfo[s]);
- assertQuery("parent:" + allProjects.get(), projects);
+ ProjectInfo parent1 = createProject(name("parent1"));
+ createProject(name("child"), parent1.name);
+
+ ProjectInfo parent2 = createProject(name("parent2"));
+ createProject(name("child2"), parent2.name);
+
+ // TODO: All-Users should be returned as well, since it's a direct child project under
+ // All-Projects
+ assertQuery("parent:" + allProjects.get(), parent1, parent2);
}
@Test
@@ -231,7 +250,7 @@
ProjectInfo project1 = createProjectWithState(name("project1"), ProjectState.ACTIVE);
ProjectInfo project2 = createProjectWithState(name("project2"), ProjectState.READ_ONLY);
- assertQuery("state:active", project1);
+ assertQuery("state:active", allUsersInfo, allProjectsInfo, project1);
assertQuery("state:read-only", project2);
}
