Merge "FileContentUtil: throw 400 if trying get content of directory"

java/com/google/gerrit/server/change/FileContentUtil.java

@@ -100,7 +100,7 @@ public class FileContentUtil {
 
   public BinaryResult getContent(
       Repository repo, ProjectState project, ObjectId revstr, String path)
-      throws IOException, ResourceNotFoundException {
+      throws IOException, ResourceNotFoundException, BadRequestException {
     try (RevWalk rw = new RevWalk(repo)) {
       RevCommit commit = rw.parseCommit(revstr);
       try (TreeWalk tw = TreeWalk.forPath(rw.getObjectReader(), path, commit.getTree())) {
@@ -114,6 +114,10 @@ public class FileContentUtil {
           return BinaryResult.create(id.name()).setContentType(X_GIT_GITLINK).base64();
         }
 
+        if (mode == org.eclipse.jgit.lib.FileMode.TREE) {
+          throw new BadRequestException("cannot retrieve content of directories");
+        }
+
         ObjectLoader obj = repo.open(id, OBJ_BLOB);
         byte[] raw;
         try {

java/com/google/gerrit/server/fixes/FixReplacementInterpreter.java

@@ -20,6 +20,7 @@ import static java.util.stream.Collectors.groupingBy;
 import com.google.gerrit.common.RawInputUtil;
 import com.google.gerrit.entities.Comment;
 import com.google.gerrit.entities.FixReplacement;
+import com.google.gerrit.extensions.restapi.BadRequestException;
 import com.google.gerrit.extensions.restapi.BinaryResult;
 import com.google.gerrit.extensions.restapi.ResourceConflictException;
 import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
@@ -69,7 +70,8 @@ public class FixReplacementInterpreter {
       ProjectState projectState,
       ObjectId patchSetCommitId,
       List<FixReplacement> fixReplacements)
-      throws ResourceNotFoundException, IOException, ResourceConflictException {
+      throws BadRequestException, ResourceNotFoundException, IOException,
+          ResourceConflictException {
     requireNonNull(fixReplacements, "Fix replacements must not be null");
 
     Map<String, List<FixReplacement>> fixReplacementsPerFilePath =
@@ -91,7 +93,8 @@ public class FixReplacementInterpreter {
       ObjectId patchSetCommitId,
       String filePath,
       List<FixReplacement> fixReplacements)
-      throws ResourceNotFoundException, IOException, ResourceConflictException {
+      throws BadRequestException, ResourceNotFoundException, IOException,
+          ResourceConflictException {
     String fileContent = getFileContent(repository, projectState, patchSetCommitId, filePath);
     String newFileContent = getNewFileContent(fileContent, fixReplacements);
     return new ChangeFileContentModification(filePath, RawInputUtil.create(newFileContent));
@@ -99,7 +102,7 @@ public class FixReplacementInterpreter {
 
   private String getFileContent(
       Repository repository, ProjectState projectState, ObjectId patchSetCommitId, String filePath)
-      throws ResourceNotFoundException, IOException {
+      throws ResourceNotFoundException, BadRequestException, IOException {
     try (BinaryResult fileContent =
         fileContentUtil.getContent(repository, projectState, patchSetCommitId, filePath)) {
       return fileContent.asString();

java/com/google/gerrit/server/restapi/change/ApplyFix.java

@@ -18,6 +18,7 @@ import com.google.gerrit.entities.PatchSet;
 import com.google.gerrit.entities.Project;
 import com.google.gerrit.extensions.common.EditInfo;
 import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.extensions.restapi.BadRequestException;
 import com.google.gerrit.extensions.restapi.ResourceConflictException;
 import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.Response;
@@ -65,8 +66,8 @@ public class ApplyFix implements RestModifyView<FixResource, Void> {
 
   @Override
   public Response<EditInfo> apply(FixResource fixResource, Void nothing)
-      throws AuthException, ResourceConflictException, IOException, ResourceNotFoundException,
-          PermissionBackendException {
+      throws AuthException, BadRequestException, ResourceConflictException, IOException,
+          ResourceNotFoundException, PermissionBackendException {
     RevisionResource revisionResource = fixResource.getRevisionResource();
     Project.NameKey project = revisionResource.getProject();
     ProjectState projectState = projectCache.checkedGet(project);

javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java

@@ -1327,6 +1327,23 @@ public class RevisionIT extends AbstractDaemonTest {
     assertContent(r, COMMIT_MSG, r.getCommit().getFullMessage());
   }
 
+  @Test
+  public void cannotGetContentOfDirectory() throws Exception {
+    Map<String, String> files = ImmutableMap.of("dir/file1.txt", "content 1");
+    PushOneCommit.Result result =
+        pushFactory.create(admin.newIdent(), testRepo, SUBJECT, files).to("refs/for/master");
+    result.assertOkStatus();
+
+    assertThrows(
+        BadRequestException.class,
+        () ->
+            gApi.changes()
+                .id(result.getChangeId())
+                .revision(result.getCommit().name())
+                .file("dir")
+                .content());
+  }
+
   @Test
   public void contentType() throws Exception {
     PushOneCommit.Result r = createChange();