Merge "Limit archives to only enabled formats"

Documentation/config-gerrit.txt

@@ -1330,7 +1330,8 @@ screen:
   archive = txz
 ----
 
-Defaults to no archive commands.
+If `download.archive` is not specified defaults to all archive
+commands. Set to `off` or empty string to disable.
 
 [[gerrit]]
 === Section gerrit

gerrit-common/src/main/java/com/google/gerrit/common/data/GerritConfig.java

@@ -19,10 +19,10 @@ import com.google.gerrit.reviewdb.client.Account.FieldName;
 import com.google.gerrit.reviewdb.client.AccountGeneralPreferences;
 import com.google.gerrit.reviewdb.client.AccountGeneralPreferences.DownloadCommand;
 import com.google.gerrit.reviewdb.client.AccountGeneralPreferences.DownloadScheme;
-import com.google.gerrit.reviewdb.client.AccountGeneralPreferences.ArchiveFormat;
 import com.google.gerrit.reviewdb.client.AuthType;
 import com.google.gerrit.reviewdb.client.Project;
 
+import java.util.List;
 import java.util.Set;
 
 public class GerritConfig implements Cloneable {
@@ -54,7 +54,7 @@ public class GerritConfig implements Cloneable {
   protected int suggestFrom;
   protected int changeUpdateDelay;
   protected AccountGeneralPreferences.ChangeScreen changeScreen;
-  protected Set<ArchiveFormat> archiveFormats;
+  protected List<String> archiveFormats;
   protected int largeChangeSize;
   protected boolean newFeatures;
 
@@ -293,11 +293,11 @@ public class GerritConfig implements Cloneable {
     this.largeChangeSize = largeChangeSize;
   }
 
-  public Set<ArchiveFormat> getArchiveFormats() {
+  public List<String> getArchiveFormats() {
     return archiveFormats;
   }
 
-  public void setArchiveFormats(Set<ArchiveFormat> formats) {
+  public void setArchiveFormats(List<String> formats) {
     archiveFormats = formats;
   }
 

gerrit-gwtui/src/main/java/com/google/gerrit/client/change/DownloadBox.java

@@ -25,7 +25,6 @@ import com.google.gerrit.client.rpc.Natives;
 import com.google.gerrit.client.rpc.RestApi;
 import com.google.gerrit.extensions.common.ListChangesOption;
 import com.google.gerrit.reviewdb.client.AccountGeneralPreferences;
-import com.google.gerrit.reviewdb.client.AccountGeneralPreferences.ArchiveFormat;
 import com.google.gerrit.reviewdb.client.AccountGeneralPreferences.DownloadScheme;
 import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gwt.core.client.JavaScriptObject;
@@ -45,10 +44,8 @@ import java.util.ArrayList;
 import java.util.EnumSet;
 import java.util.Iterator;
 import java.util.List;
-import java.util.Set;
 
 class DownloadBox extends VerticalPanel {
-  private final static String ARCHIVE[] = {"tar", "tbz2", "tgz", "txz"};
   private final ChangeInfo change;
   private final String revision;
   private final PatchSet.Id psId;
@@ -149,13 +146,13 @@ class DownloadBox extends VerticalPanel {
   }
 
   private void insertArchive() {
-    Set<ArchiveFormat> activated = Gerrit.getConfig().getArchiveFormats();
+    List<String> activated = Gerrit.getConfig().getArchiveFormats();
-    if (activated.contains(ArchiveFormat.OFF)) {
+    if (activated.isEmpty()) {
       return;
     }
+
     List<Anchor> anchors = new ArrayList<>(activated.size());
-    for (String f : ARCHIVE) {
+    for (String f : activated) {
-      if (activated.contains(ArchiveFormat.valueOf(f.toUpperCase()))) {
       Anchor archive = new Anchor(f);
       archive.setHref(new RestApi("/changes/")
           .id(psId.getParentKey().get())
@@ -166,7 +163,6 @@ class DownloadBox extends VerticalPanel {
           .url());
       anchors.add(archive);
     }
-    }
 
     HorizontalPanel p = new HorizontalPanel();
     Iterator<Anchor> it = anchors.iterator();

gerrit-httpd/src/main/java/com/google/gerrit/httpd/GerritConfigProvider.java

@@ -14,12 +14,16 @@
 
 package com.google.gerrit.httpd;
 
+import com.google.common.base.Function;
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Lists;
 import com.google.gerrit.common.data.GerritConfig;
 import com.google.gerrit.common.data.GitwebConfig;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.AccountGeneralPreferences;
-import com.google.gerrit.reviewdb.client.AccountGeneralPreferences.ArchiveFormat;
 import com.google.gerrit.server.account.Realm;
+import com.google.gerrit.server.change.ArchiveFormat;
+import com.google.gerrit.server.change.GetArchive;
 import com.google.gerrit.server.config.AllProjectsName;
 import com.google.gerrit.server.config.AnonymousCowardName;
 import com.google.gerrit.server.config.AuthConfig;
@@ -37,7 +41,6 @@ import org.eclipse.jgit.lib.Config;
 
 import java.net.MalformedURLException;
 import java.util.HashSet;
-import java.util.List;
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
 
@@ -48,6 +51,7 @@ class GerritConfigProvider implements Provider<GerritConfig> {
   private final Config cfg;
   private final AuthConfig authConfig;
   private final DownloadConfig downloadConfig;
+  private final GetArchive.AllowedFormats archiveFormats;
   private final GitWebConfig gitWebConfig;
   private final AllProjectsName wildProject;
   private final SshInfo sshInfo;
@@ -62,11 +66,13 @@ class GerritConfigProvider implements Provider<GerritConfig> {
       final AuthConfig ac, final GitWebConfig gwc, final AllProjectsName wp,
       final SshInfo si, final ContactStore cs,
       final ServletContext sc, final DownloadConfig dc,
+      final GetArchive.AllowedFormats af,
       final @AnonymousCowardName String acn) {
     realm = r;
     cfg = gsc;
     authConfig = ac;
     downloadConfig = dc;
+    archiveFormats = af;
     gitWebConfig = gwc;
     sshInfo = si;
     wildProject = wp;
@@ -130,11 +136,14 @@ class GerritConfigProvider implements Provider<GerritConfig> {
         "gerrit", null, "changeScreen",
         AccountGeneralPreferences.ChangeScreen.CHANGE_SCREEN2));
     config.setLargeChangeSize(cfg.getInt("change", "largeChange", 500));
-
+    config.setArchiveFormats(Lists.newArrayList(Iterables.transform(
-    List<ArchiveFormat> allArchiveFormats =
+        archiveFormats.getAllowed(),
-        ConfigUtil.getEnumList(cfg, "download", null, "archive",
+        new Function<ArchiveFormat, String>() {
-            ArchiveFormat.OFF);
+          @Override
-    config.setArchiveFormats(new HashSet<>(allArchiveFormats));
+          public String apply(ArchiveFormat in) {
+            return in.getShortName();
+          }
+        })));
 
     config.setNewFeatures(cfg.getBoolean("gerrit", "enableNewFeatures", true));
 

gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/AccountGeneralPreferences.java

@@ -35,10 +35,6 @@ public final class AccountGeneralPreferences {
     REPO_DOWNLOAD, PULL, CHECKOUT, CHERRY_PICK, FORMAT_PATCH, DEFAULT_DOWNLOADS
   }
 
-  public static enum ArchiveFormat {
-    OFF, TAR, TBZ2, TGZ, TXZ
-  }
-
   public static enum DateFormat {
     /** US style dates: Apr 27, Feb 14, 2010 */
     STD("MMM d", "MMM d, yyyy"),

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

@@ -14,20 +14,13 @@
 
 package com.google.gerrit.server.change;
 
-import com.google.common.collect.Maps;
-
 import org.eclipse.jgit.api.ArchiveCommand;
 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.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
-import java.util.Collections;
+public enum ArchiveFormat {
-import java.util.Map;
-
-enum ArchiveFormat {
   TGZ("application/x-gzip", new TgzFormat()),
   TAR("application/x-tar", new TarFormat()),
   TBZ2("application/x-bzip2", new Tbz2Format()),
@@ -35,8 +28,6 @@ enum ArchiveFormat {
   // Zip is not supported because it may be interpreted by a Java plugin as a
   // valid JAR file, whose code would have access to cookies on the domain.
 
-  static final Logger log = LoggerFactory.getLogger(ArchiveFormat.class);
-
   private final ArchiveCommand.Format<?> format;
   private final String mimeType;
 
@@ -46,7 +37,7 @@ enum ArchiveFormat {
     ArchiveCommand.registerFormat(name(), format);
   }
 
-  String getShortName() {
+  public String getShortName() {
     return name().toLowerCase();
   }
 
@@ -61,24 +52,4 @@ enum ArchiveFormat {
   Iterable<String> getSuffixes() {
     return format.suffixes();
   }
-
-  static Map<String, ArchiveFormat> init() {
-    String[] formats = new String[values().length];
-    for (int i = 0; i < values().length; i++) {
-      formats[i] = values()[i].name();
-    }
-
-    Map<String, ArchiveFormat> exts = Maps.newLinkedHashMap();
-    for (String name : formats) {
-      try {
-        ArchiveFormat format = valueOf(name.toUpperCase());
-        for (String ext : format.getSuffixes()) {
-          exts.put(ext, format);
-        }
-      } catch (IllegalArgumentException e) {
-        log.warn("Invalid archive.format {}", name);
-      }
-    }
-    return Collections.unmodifiableMap(exts);
-  }
 }

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

@@ -15,14 +15,19 @@
 package com.google.gerrit.server.change;
 
 import com.google.common.base.Strings;
+import com.google.common.collect.ImmutableMap;
 import com.google.gerrit.extensions.restapi.BadRequestException;
 import com.google.gerrit.extensions.restapi.BinaryResult;
 import com.google.gerrit.extensions.restapi.RestReadView;
+import com.google.gerrit.server.config.ConfigUtil;
+import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.inject.Inject;
+import com.google.inject.Singleton;
 
 import org.eclipse.jgit.api.ArchiveCommand;
 import org.eclipse.jgit.api.errors.GitAPIException;
+import org.eclipse.jgit.lib.Config;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.revwalk.RevCommit;
@@ -31,18 +36,60 @@ import org.kohsuke.args4j.Option;
 
 import java.io.IOException;
 import java.io.OutputStream;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.LinkedHashSet;
 import java.util.Map;
+import java.util.Set;
+
+public class GetArchive implements RestReadView<RevisionResource> {
+  @Singleton
+  public static class AllowedFormats {
+    final ImmutableMap<String, ArchiveFormat> extensions;
+    final Set<ArchiveFormat> allowed;
+
+    @Inject
+    AllowedFormats(@GerritServerConfig Config cfg) {
+      Collection<ArchiveFormat> enabled;
+      String v = cfg.getString("download", null, "archive");
+      if (v == null) {
+        enabled = Arrays.asList(ArchiveFormat.values());
+      } else if (v.isEmpty() || "off".equalsIgnoreCase(v)) {
+        enabled = Collections.emptyList();
+      } else {
+        enabled = ConfigUtil.getEnumList(cfg,
+            "download", null, "archive",
+            ArchiveFormat.TGZ);
+      }
+
+      Map<String, ArchiveFormat> exts = new HashMap<>();
+      for (ArchiveFormat format : enabled) {
+        for (String ext : format.getSuffixes()) {
+          exts.put(ext, format);
+        }
+        exts.put(format.name().toLowerCase(), format);
+      }
+      extensions = ImmutableMap.copyOf(exts);
+      allowed = Collections.unmodifiableSet(new LinkedHashSet<>(enabled));
+    }
+
+    public Set<ArchiveFormat> getAllowed() {
+      return allowed;
+    }
+  }
 
-class GetArchive implements RestReadView<RevisionResource> {
-  private static final Map<String, ArchiveFormat> formats = ArchiveFormat.init();
   private final GitRepositoryManager repoManager;
+  private final AllowedFormats allowedFormats;
 
   @Option(name = "--format")
   private String format;
 
   @Inject
-  GetArchive(GitRepositoryManager repoManager) {
+  GetArchive(GitRepositoryManager repoManager, AllowedFormats allowedFormats) {
     this.repoManager = repoManager;
+    this.allowedFormats = allowedFormats;
   }
 
   @Override
@@ -51,7 +98,7 @@ class GetArchive implements RestReadView<RevisionResource> {
     if (Strings.isNullOrEmpty(format)) {
       throw new BadRequestException("format is not specified");
     }
-    final ArchiveFormat f = formats.get("." + format);
+    final ArchiveFormat f = allowedFormats.extensions.get("." + format);
     if (f == null) {
       throw new BadRequestException("unknown archive format");
     }