Merge "PreviewSubmit: fix generation of compressed tar entries"

Documentation/config-gerrit.txt

@@ -1828,6 +1828,15 @@ valid JAR file, whose code would have access to cookies on the domain.
 For this reason `zip` format is always excluded from formats offered
 through the `Download` drop down or accessible in the REST API.
 
+[[download.maxBundleSize]]download.maxBundleSize::
++
+Specifies the maximum size of a bundle in bytes that can be downloaded.
+As bundles are kept in memory this setting is to protect the server
+from a single request consuming too much heap when generating
+a bundle and thereby impacting other users.
++
+Defaults to 100MB.
+
 [[gc]]
 === Section gc
 

gerrit-acceptance-tests/BUCK

@@ -29,6 +29,7 @@ java_library(
 
     '//lib/bouncycastle:bcpg',
     '//lib/bouncycastle:bcprov',
+    '//lib/commons:compress',
     '//lib/greenmail:greenmail',
     '//lib/guice:guice',
     '//lib/guice:guice-assistedinject',

gerrit-acceptance-tests/BUILD

@@ -30,6 +30,7 @@ java_library2(
         "//lib:servlet-api-3_1-without-neverlink",
         "//lib/bouncycastle:bcpg",
         "//lib/bouncycastle:bcprov",
+        "//lib/commons:compress",
         "//lib/guice",
         "//lib/guice:guice-assistedinject",
         "//lib/guice:guice-servlet",

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java

@@ -647,6 +647,11 @@ public abstract class AbstractSubmit extends AbstractDaemonTest {
     return gApi.changes().id(changeId).current().submitPreview();
   }
 
+  protected BinaryResult submitPreview(String changeId, String format)
+      throws Exception {
+    return gApi.changes().id(changeId).current().submitPreview(format);
+  }
+
   protected void assertSubmittable(String changeId) throws Exception {
     assertThat(get(changeId, SUBMITTABLE).submittable)
         .named("submit bit on ChangeInfo")

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByMergeIfNecessaryIT.java

@@ -30,14 +30,23 @@ import com.google.gerrit.extensions.restapi.RestApiException;
 import com.google.gerrit.reviewdb.client.Branch;
 import com.google.gerrit.reviewdb.client.Project;
 
+import org.apache.commons.compress.archivers.ArchiveStreamFactory;
+import org.apache.commons.compress.archivers.tar.TarArchiveEntry;
+import org.apache.commons.compress.archivers.tar.TarArchiveInputStream;
 import org.eclipse.jgit.junit.TestRepository;
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.revwalk.RevTree;
 import org.eclipse.jgit.transport.RefSpec;
 import org.junit.Test;
 
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.io.InputStream;
+import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
+import java.util.zip.GZIPInputStream;
 
 public class SubmitByMergeIfNecessaryIT extends AbstractSubmitByMerge {
 
@@ -533,4 +542,33 @@ public class SubmitByMergeIfNecessaryIT extends AbstractSubmitByMerge {
     assertRefUpdatedEvents();
     assertChangeMergedEvents();
   }
+
+  @Test
+  public void testPreviewSubmitTgz() throws Exception {
+    Project.NameKey p1 = createProject("project-name");
+
+    TestRepository<?> repo1 = cloneProject(p1);
+    PushOneCommit.Result change1 = createChange(repo1, "master",
+        "test", "a.txt", "1", "topic");
+    approve(change1.getChangeId());
+
+    // get a preview before submitting:
+    BinaryResult request = submitPreview(change1.getChangeId(), "tgz");
+
+    assertThat(request.getContentType()).isEqualTo("application/x-gzip");
+    File tempfile = File.createTempFile("test", null);
+    request.writeTo(new FileOutputStream(tempfile));
+
+    InputStream is = new GZIPInputStream(new FileInputStream(tempfile));
+
+    List<String> untarredFiles = new LinkedList<>();
+    try (TarArchiveInputStream tarInputStream = (TarArchiveInputStream)
+        new ArchiveStreamFactory().createArchiveInputStream("tar", is)) {
+      TarArchiveEntry entry = null;
+      while ((entry = (TarArchiveEntry)tarInputStream.getNextEntry()) != null) {
+        untarredFiles.add(entry.getName());
+      }
+    }
+    assertThat(untarredFiles).containsExactly(name("project-name") + ".git");
+  }
 }

gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/RevisionApi.java

@@ -41,6 +41,7 @@ public interface RevisionApi {
   void submit() throws RestApiException;
   void submit(SubmitInput in) throws RestApiException;
   BinaryResult submitPreview() throws RestApiException;
+  BinaryResult submitPreview(String format) throws RestApiException;
   void publish() throws RestApiException;
   ChangeApi cherryPick(CherryPickInput in) throws RestApiException;
   ChangeApi rebase() throws RestApiException;
@@ -278,6 +279,11 @@ public interface RevisionApi {
       throw new NotImplementedException();
     }
 
+    @Override
+    public BinaryResult submitPreview(String format) throws RestApiException {
+      throw new NotImplementedException();
+    }
+
     @Override
     public SubmitType testSubmitType(TestSubmitRuleInput in)
         throws RestApiException {

gerrit-server/src/main/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java

@@ -220,7 +220,12 @@ class RevisionApiImpl implements RevisionApi {
 
   @Override
   public BinaryResult submitPreview() throws RestApiException {
-    submitPreview.setFormat("zip");
+    return submitPreview("zip");
+  }
+
+  @Override
+  public BinaryResult submitPreview(String format) throws RestApiException {
+    submitPreview.setFormat(format);
     return submitPreview.apply(revision);
   }
 

gerrit-server/src/main/java/com/google/gerrit/server/change/ArchiveFormat.java

@@ -14,51 +14,27 @@
 
 package com.google.gerrit.server.change;
 
-import org.apache.commons.compress.archivers.ArchiveEntry;
 import org.apache.commons.compress.archivers.ArchiveOutputStream;
-import org.apache.commons.compress.archivers.tar.TarArchiveEntry;
-import org.apache.commons.compress.archivers.zip.ZipArchiveEntry;
 import org.eclipse.jgit.api.ArchiveCommand;
+import org.eclipse.jgit.api.ArchiveCommand.Format;
 import org.eclipse.jgit.archive.TarFormat;
 import org.eclipse.jgit.archive.Tbz2Format;
 import org.eclipse.jgit.archive.TgzFormat;
 import org.eclipse.jgit.archive.TxzFormat;
 import org.eclipse.jgit.archive.ZipFormat;
+import org.eclipse.jgit.lib.FileMode;
+import org.eclipse.jgit.lib.ObjectLoader;
 
+import java.io.Closeable;
 import java.io.IOException;
 import java.io.OutputStream;
 
 public enum ArchiveFormat {
-  TGZ("application/x-gzip", new TgzFormat()) {
-    @Override
-    public ArchiveEntry prepareArchiveEntry(String fileName) {
-      return new TarArchiveEntry(fileName);
-    }
-  },
-  TAR("application/x-tar", new TarFormat()) {
-    @Override
-    public ArchiveEntry prepareArchiveEntry(String fileName) {
-      return new TarArchiveEntry(fileName);
-    }
-  },
-  TBZ2("application/x-bzip2", new Tbz2Format()) {
-    @Override
-    public ArchiveEntry prepareArchiveEntry(String fileName) {
-      return new TarArchiveEntry(fileName);
-    }
-  },
-  TXZ("application/x-xz", new TxzFormat()) {
-    @Override
-    public ArchiveEntry prepareArchiveEntry(String fileName) {
-      return new TarArchiveEntry(fileName);
-    }
-  },
-  ZIP("application/x-zip", new ZipFormat()) {
-    @Override
-    public ArchiveEntry prepareArchiveEntry(String fileName) {
-      return new ZipArchiveEntry(fileName);
-    }
-  };
+  TGZ("application/x-gzip", new TgzFormat()),
+  TAR("application/x-tar", new TarFormat()),
+  TBZ2("application/x-bzip2", new Tbz2Format()),
+  TXZ("application/x-xz", new TxzFormat()),
+  ZIP("application/x-zip", new ZipFormat());
 
   private final ArchiveCommand.Format<?> format;
   private final String mimeType;
@@ -90,5 +66,11 @@ public enum ArchiveFormat {
     return (ArchiveOutputStream)this.format.createArchiveOutputStream(o);
   }
 
-  public abstract ArchiveEntry prepareArchiveEntry(final String fileName);
+  public <T extends Closeable> void putEntry(T out, String path, byte[] data)
+      throws IOException {
+    @SuppressWarnings("unchecked")
+    ArchiveCommand.Format<T> fmt = (Format<T>) format;
+    fmt.putEntry(out, path, FileMode.REGULAR_FILE,
+          new ObjectLoader.SmallObject(FileMode.TYPE_FILE, data));
+  }
 }

gerrit-server/src/main/java/com/google/gerrit/server/change/LimitedByteArrayOutputStream.java

@@ -0,0 +1,70 @@
+// Copyright (C) 2016 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.change;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.OutputStream;
+
+class LimitedByteArrayOutputStream extends OutputStream {
+
+  private final int maxSize;
+  private final ByteArrayOutputStream buffer;
+
+  /**
+   * Constructs a LimitedByteArrayOutputStream, which stores output
+   * in memory up to a certain specified size. When the output exceeds
+   * the specified size a LimitExceededException is thrown.
+   *
+   * @param max the maximum size in bytes which may be stored.
+   * @param initial the initial size. It must be smaller than the max size.
+   */
+  public LimitedByteArrayOutputStream(int max, int initial) {
+    checkArgument(initial <= max);
+    maxSize = max;
+    buffer = new ByteArrayOutputStream(initial);
+  }
+
+  private void checkOversize(int additionalSize) throws IOException {
+    if (buffer.size() + additionalSize > maxSize) {
+      throw new LimitExceededException();
+    }
+  }
+
+  @Override
+  public void write(int b) throws IOException{
+    checkOversize(1);
+    buffer.write(b);
+  }
+
+  @Override
+  public void write(byte[] b, int off, int len) throws IOException {
+    checkOversize(len);
+    buffer.write(b, off, len);
+  }
+
+  /**
+   * @return a newly allocated byte array with contents of the buffer.
+   */
+  public byte[] toByteArray() {
+    return buffer.toByteArray();
+  }
+
+  class LimitExceededException extends IOException {
+    private static final long serialVersionUID = 1L;
+  }
+}

gerrit-server/src/main/java/com/google/gerrit/server/change/PreviewSubmit.java

@@ -19,6 +19,7 @@ import com.google.gerrit.extensions.api.changes.SubmitInput;
 import com.google.gerrit.extensions.restapi.BadRequestException;
 import com.google.gerrit.extensions.restapi.BinaryResult;
 import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
+import com.google.gerrit.extensions.restapi.NotImplementedException;
 import com.google.gerrit.extensions.restapi.PreconditionFailedException;
 import com.google.gerrit.extensions.restapi.RestApiException;
 import com.google.gerrit.extensions.restapi.RestReadView;
@@ -26,6 +27,8 @@ import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.change.LimitedByteArrayOutputStream.LimitExceededException;
+import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.git.MergeOp;
 import com.google.gerrit.server.git.MergeOpRepoManager;
 import com.google.gerrit.server.git.MergeOpRepoManager.OpenRepo;
@@ -36,6 +39,7 @@ import com.google.inject.Provider;
 import com.google.inject.Singleton;
 
 import org.apache.commons.compress.archivers.ArchiveOutputStream;
+import org.eclipse.jgit.lib.Config;
 import org.eclipse.jgit.lib.NullProgressMonitor;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.transport.BundleWriter;
@@ -49,10 +53,12 @@ import java.util.Set;
 
 @Singleton
 public class PreviewSubmit implements RestReadView<RevisionResource> {
+  private static int MAX_DEFAULT_BUNDLE_SIZE = 100 * 1024 * 1024;
+
   private final Provider<ReviewDb> dbProvider;
   private final Provider<MergeOp> mergeOpProvider;
   private final AllowedFormats allowedFormats;
-
+  private int maxBundleSize;
   private String format;
 
   @Option(name = "--format")
@@ -63,10 +69,13 @@ public class PreviewSubmit implements RestReadView<RevisionResource> {
   @Inject
   PreviewSubmit(Provider<ReviewDb> dbProvider,
       Provider<MergeOp> mergeOpProvider,
-      AllowedFormats allowedFormats) {
+      AllowedFormats allowedFormats,
+      @GerritServerConfig Config cfg) {
     this.dbProvider = dbProvider;
     this.mergeOpProvider = mergeOpProvider;
     this.allowedFormats = allowedFormats;
+    this.maxBundleSize = cfg.getInt("download", "maxBundleSize",
+        MAX_DEFAULT_BUNDLE_SIZE);
   }
 
   @Override
@@ -120,27 +129,33 @@ public class PreviewSubmit implements RestReadView<RevisionResource> {
       bin = new BinaryResult() {
         @Override
         public void writeTo(OutputStream out) throws IOException {
-          ArchiveOutputStream aos = f.createArchiveOutputStream(out);
-
-          for (Project.NameKey p : projects) {
-            OpenRepo or = orm.getRepo(p);
-            BundleWriter bw = new BundleWriter(or.getRepo());
-            bw.setObjectCountCallback(null);
-            bw.setPackConfig(null);
-            Collection<ReceiveCommand> refs = or.getUpdate().getRefUpdates();
-            for (ReceiveCommand r : refs) {
-              bw.include(r.getRefName(), r.getNewId());
-              if (!r.getOldId().equals(ObjectId.zeroId())) {
-                bw.assume(or.getCodeReviewRevWalk().parseCommit(r.getOldId()));
+          try (ArchiveOutputStream aos = f.createArchiveOutputStream(out)) {
+            for (Project.NameKey p : projects) {
+              OpenRepo or = orm.getRepo(p);
+              BundleWriter bw = new BundleWriter(or.getRepo());
+              bw.setObjectCountCallback(null);
+              bw.setPackConfig(null);
+              Collection<ReceiveCommand> refs = or.getUpdate().getRefUpdates();
+              for (ReceiveCommand r : refs) {
+                bw.include(r.getRefName(), r.getNewId());
+                ObjectId oldId = r.getOldId();
+                if (!oldId.equals(ObjectId.zeroId())) {
+                  bw.assume(or.getCodeReviewRevWalk().parseCommit(oldId));
+                }
               }
+              // This naming scheme cannot produce directory/file conflicts
+              // as no projects contains ".git/":
+              String path = p.get() + ".git";
+
+              LimitedByteArrayOutputStream bos =
+                  new LimitedByteArrayOutputStream(maxBundleSize, 1024);
+              bw.writeBundle(NullProgressMonitor.INSTANCE, bos);
+              f.putEntry(aos, path, bos.toByteArray());
             }
-            // This naming scheme cannot produce directory/file conflicts
-            // as no projects contains ".git/":
-            aos.putArchiveEntry(f.prepareArchiveEntry(p.get() + ".git"));
-            bw.writeBundle(NullProgressMonitor.INSTANCE, aos);
-            aos.closeArchiveEntry();
+          } catch (LimitExceededException e) {
+            throw new NotImplementedException("The bundle is too big to "
+                + "generate at the server");
           }
-          aos.finish();
         }
       };
     }