Specify allowed archive formats in gitiles.config
With the default compression options, some archive formats may be
CPU-intensive enough to slow down a server, so give administrators the
option to disable them.
Formats are specified as a whitelist of archive.format options,
defaulting to everything (except zip, which is still not supported for
security reasons). A link is only shown in the commit detail template
for the first specified format.
Since archive formats are now needed outside of ArchiveServlet, move
the enum to its own file.
Change-Id: I8d5a66678aae75f2bbe54ef4883ca4a070b96817
diff --git a/gitiles-servlet/src/main/java/com/google/gitiles/ArchiveFormat.java b/gitiles-servlet/src/main/java/com/google/gitiles/ArchiveFormat.java
new file mode 100644
index 0000000..16fdf50
--- /dev/null
+++ b/gitiles-servlet/src/main/java/com/google/gitiles/ArchiveFormat.java
@@ -0,0 +1,91 @@
+// Copyright 2013 Google Inc. All Rights Reserved.
+//
+// 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.gitiles;
+
+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.eclipse.jgit.lib.Config;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.Collections;
+import java.util.Map;
+
+enum ArchiveFormat {
+ 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 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;
+
+ private ArchiveFormat(String mimeType, ArchiveCommand.Format<?> format) {
+ this.format = format;
+ this.mimeType = mimeType;
+ ArchiveCommand.registerFormat(name(), format);
+ }
+
+ String getShortName() {
+ return name().toLowerCase();
+ }
+
+ String getMimeType() {
+ return mimeType;
+ }
+
+ String getDefaultSuffix() {
+ return getSuffixes().iterator().next();
+ }
+
+ Iterable<String> getSuffixes() {
+ return format.suffixes();
+ }
+
+ static ArchiveFormat getDefault(Config cfg) {
+ return byExtension(cfg).values().iterator().next();
+ }
+
+ static Map<String, ArchiveFormat> byExtension(Config cfg) {
+ String[] formats = cfg.getStringList("archive", null, "format");
+ if (formats.length == 0) {
+ 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);
+ }
+}
diff --git a/gitiles-servlet/src/main/java/com/google/gitiles/ArchiveServlet.java b/gitiles-servlet/src/main/java/com/google/gitiles/ArchiveServlet.java
index b7d49dd..a38c463 100644
--- a/gitiles-servlet/src/main/java/com/google/gitiles/ArchiveServlet.java
+++ b/gitiles-servlet/src/main/java/com/google/gitiles/ArchiveServlet.java
@@ -17,22 +17,16 @@
import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND;
import static javax.servlet.http.HttpServletResponse.SC_OK;
-import com.google.common.collect.ImmutableMap;
-
import org.eclipse.jgit.api.ArchiveCommand;
import org.eclipse.jgit.api.errors.GitAPIException;
-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.errors.IncorrectObjectTypeException;
import org.eclipse.jgit.http.server.ServletUtils;
+import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevWalk;
import java.io.IOException;
import java.util.Map;
-import java.util.Set;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServletRequest;
@@ -41,42 +35,11 @@
public class ArchiveServlet extends BaseServlet {
private static final long serialVersionUID = 1L;
- private enum Format {
- TAR("application/x-tar", new TarFormat()),
- TGZ("application/x-gzip", new TgzFormat()),
- TBZ2("application/x-bzip2", new Tbz2Format()),
- TXZ("application/x-xz", new TxzFormat());
- // 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.
+ private final Map<String, ArchiveFormat> byExt;
- private final ArchiveCommand.Format<?> format;
- private final String mimeType;
-
- private Format(String mimeType, ArchiveCommand.Format<?> format) {
- this.format = format;
- this.mimeType = mimeType;
- ArchiveCommand.registerFormat(name(), format);
- }
- }
-
- private static final Map<String, Format> FORMATS_BY_EXTENSION;
-
- static {
- ImmutableMap.Builder<String, Format> exts = ImmutableMap.builder();
- for (Format format : Format.values()) {
- for (String ext : format.format.suffixes()) {
- exts.put(ext, format);
- }
- }
- FORMATS_BY_EXTENSION = exts.build();
- }
-
- static Set<String> validExtensions() {
- return FORMATS_BY_EXTENSION.keySet();
- }
-
- public ArchiveServlet() {
+ public ArchiveServlet(Config cfg) {
super(null);
+ byExt = ArchiveFormat.byExtension(cfg);
}
@Override
@@ -99,9 +62,9 @@
walk.release();
}
- Format format = FORMATS_BY_EXTENSION.get(view.getExtension());
+ ArchiveFormat format = byExt.get(view.getExtension());
String filename = getFilename(view, rev, view.getExtension());
- setDownloadHeaders(req, res, filename, format.mimeType);
+ setDownloadHeaders(req, res, filename, format.getMimeType());
res.setStatus(SC_OK);
try {
diff --git a/gitiles-servlet/src/main/java/com/google/gitiles/CommitSoyData.java b/gitiles-servlet/src/main/java/com/google/gitiles/CommitSoyData.java
index ff879ba..d84721b 100644
--- a/gitiles-servlet/src/main/java/com/google/gitiles/CommitSoyData.java
+++ b/gitiles-servlet/src/main/java/com/google/gitiles/CommitSoyData.java
@@ -62,7 +62,7 @@
/** Valid sets of keys to include in Soy data for commits. */
public static enum KeySet {
DETAIL("author", "committer", "sha", "tree", "treeUrl", "parents", "message", "logUrl",
- "tgzUrl"),
+ "archiveUrl", "archiveType"),
DETAIL_DIFF_TREE(DETAIL, "diffTree"),
SHORTLOG("abbrevSha", "url", "shortMessage", "author", "branches", "tags"),
DEFAULT(DETAIL);
@@ -88,6 +88,7 @@
private RevWalk walk;
private GitilesView view;
private Map<AnyObjectId, Set<Ref>> refsById;
+ private ArchiveFormat archiveFormat;
public CommitSoyData setLinkifier(Linkifier linkifier) {
this.linkifier = checkNotNull(linkifier, "linkifier");
@@ -99,6 +100,11 @@
return this;
}
+ public CommitSoyData setArchiveFormat(ArchiveFormat archiveFormat) {
+ this.archiveFormat = checkNotNull(archiveFormat, "archiveFormat");
+ return this;
+ }
+
public Map<String, Object> toSoyData(HttpServletRequest req, RevCommit commit, KeySet ks)
throws IOException {
checkKeys(ks);
@@ -133,8 +139,12 @@
if (ks.contains("logUrl")) {
data.put("logUrl", urlFromView(view, commit, GitilesView.log()));
}
- if (ks.contains("tgzUrl")) {
- data.put("tgzUrl", urlFromView(view, commit, GitilesView.archive().setExtension(".tar.gz")));
+ if (ks.contains("archiveUrl")) {
+ data.put("archiveUrl", urlFromView(view, commit,
+ GitilesView.archive().setExtension(archiveFormat.getDefaultSuffix())));
+ }
+ if (ks.contains("archiveType")) {
+ data.put("archiveType", archiveFormat.getShortName());
}
if (ks.contains("tree")) {
data.put("tree", ObjectId.toString(commit.getTree()));
@@ -175,6 +185,9 @@
private void checkKeys(KeySet ks) {
checkState(!ks.contains("diffTree") || walk != null, "RevWalk required for diffTree");
+ if (ks.contains("archiveUrl") || ks.contains("archiveType")) {
+ checkState(archiveFormat != null, "archive format required");
+ }
}
// TODO(dborowitz): Extract this.
diff --git a/gitiles-servlet/src/main/java/com/google/gitiles/DiffServlet.java b/gitiles-servlet/src/main/java/com/google/gitiles/DiffServlet.java
index bd97f6f..c34ef9f 100644
--- a/gitiles-servlet/src/main/java/com/google/gitiles/DiffServlet.java
+++ b/gitiles-servlet/src/main/java/com/google/gitiles/DiffServlet.java
@@ -15,6 +15,7 @@
package com.google.gitiles;
import static com.google.common.base.Preconditions.checkNotNull;
+
import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND;
import com.google.common.base.Charsets;
@@ -23,6 +24,7 @@
import org.eclipse.jgit.errors.IncorrectObjectTypeException;
import org.eclipse.jgit.errors.MissingObjectException;
import org.eclipse.jgit.http.server.ServletUtils;
+import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
@@ -46,10 +48,12 @@
private static final String PLACEHOLDER = "id=\"DIFF_OUTPUT_BLOCK\"";
private final Linkifier linkifier;
+ private final ArchiveFormat archiveFormat;
- public DiffServlet(Renderer renderer, Linkifier linkifier) {
+ public DiffServlet(Config cfg, Renderer renderer, Linkifier linkifier) {
super(renderer);
this.linkifier = checkNotNull(linkifier, "linkifier");
+ this.archiveFormat = ArchiveFormat.getDefault(checkNotNull(cfg, "cfg"));
}
@Override
@@ -81,6 +85,7 @@
if (showCommit) {
data.put("commit", new CommitSoyData()
.setLinkifier(linkifier)
+ .setArchiveFormat(archiveFormat)
.toSoyData(req, walk.parseCommit(view.getRevision().getId())));
}
if (!data.containsKey("repositoryName") && (view.getRepositoryName() != null)) {
diff --git a/gitiles-servlet/src/main/java/com/google/gitiles/GitilesFilter.java b/gitiles-servlet/src/main/java/com/google/gitiles/GitilesFilter.java
index cb2e459..6f4468d 100644
--- a/gitiles-servlet/src/main/java/com/google/gitiles/GitilesFilter.java
+++ b/gitiles-servlet/src/main/java/com/google/gitiles/GitilesFilter.java
@@ -201,7 +201,7 @@
}
Filter repositoryFilter = new RepositoryFilter(resolver);
- Filter viewFilter = new ViewFilter(accessFactory, urls, visibilityCache);
+ Filter viewFilter = new ViewFilter(this.config, accessFactory, urls, visibilityCache);
Filter dispatchFilter = new DispatchFilter(filters, servlets);
ServletBinder root = serveRegex(ROOT_REGEX).through(viewFilter);
@@ -233,17 +233,17 @@
case REFS:
return new RefServlet(renderer, timeCache);
case REVISION:
- return new RevisionServlet(renderer, linkifier());
+ return new RevisionServlet(config, renderer, linkifier());
case PATH:
return new PathServlet(renderer, urls);
case DIFF:
- return new DiffServlet(renderer, linkifier());
+ return new DiffServlet(config, renderer, linkifier());
case LOG:
return new LogServlet(renderer, linkifier());
case DESCRIBE:
return new DescribeServlet();
case ARCHIVE:
- return new ArchiveServlet();
+ return new ArchiveServlet(config);
default:
throw new IllegalArgumentException("Invalid view type: " + view);
}
diff --git a/gitiles-servlet/src/main/java/com/google/gitiles/GitilesView.java b/gitiles-servlet/src/main/java/com/google/gitiles/GitilesView.java
index 25e66b8..38ee7e6 100644
--- a/gitiles-servlet/src/main/java/com/google/gitiles/GitilesView.java
+++ b/gitiles-servlet/src/main/java/com/google/gitiles/GitilesView.java
@@ -115,7 +115,7 @@
default:
break;
}
- if (type == Type.ARCHIVE) {
+ if (type == Type.ARCHIVE && other.type == Type.ARCHIVE) {
extension = other.extension;
}
// Don't copy params.
diff --git a/gitiles-servlet/src/main/java/com/google/gitiles/RevisionServlet.java b/gitiles-servlet/src/main/java/com/google/gitiles/RevisionServlet.java
index bf9de35..c22d627 100644
--- a/gitiles-servlet/src/main/java/com/google/gitiles/RevisionServlet.java
+++ b/gitiles-servlet/src/main/java/com/google/gitiles/RevisionServlet.java
@@ -15,7 +15,9 @@
package com.google.gitiles;
import static com.google.common.base.Preconditions.checkNotNull;
+
import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND;
+
import static org.eclipse.jgit.lib.Constants.OBJ_BLOB;
import static org.eclipse.jgit.lib.Constants.OBJ_COMMIT;
import static org.eclipse.jgit.lib.Constants.OBJ_TAG;
@@ -28,6 +30,7 @@
import org.eclipse.jgit.errors.IncorrectObjectTypeException;
import org.eclipse.jgit.errors.MissingObjectException;
import org.eclipse.jgit.http.server.ServletUtils;
+import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Repository;
@@ -51,10 +54,12 @@
private static final Logger log = LoggerFactory.getLogger(RevisionServlet.class);
private final Linkifier linkifier;
+ private final ArchiveFormat archiveFormat;
- public RevisionServlet(Renderer renderer, Linkifier linkifier) {
+ public RevisionServlet(Config cfg, Renderer renderer, Linkifier linkifier) {
super(renderer);
this.linkifier = checkNotNull(linkifier, "linkifier");
+ this.archiveFormat = ArchiveFormat.getDefault(checkNotNull(cfg, "cfg"));
}
@Override
@@ -78,6 +83,7 @@
"data", new CommitSoyData()
.setLinkifier(linkifier)
.setRevWalk(walk)
+ .setArchiveFormat(archiveFormat)
.toSoyData(req, (RevCommit) obj, KeySet.DETAIL_DIFF_TREE)));
break;
case OBJ_TREE:
diff --git a/gitiles-servlet/src/main/java/com/google/gitiles/ViewFilter.java b/gitiles-servlet/src/main/java/com/google/gitiles/ViewFilter.java
index 5b373de..4f60c12 100644
--- a/gitiles-servlet/src/main/java/com/google/gitiles/ViewFilter.java
+++ b/gitiles-servlet/src/main/java/com/google/gitiles/ViewFilter.java
@@ -19,17 +19,21 @@
import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND;
-import org.eclipse.jgit.http.server.ServletUtils;
-import org.eclipse.jgit.http.server.glue.WrappedRequest;
-
import java.io.IOException;
import java.util.Map;
+import java.util.Set;
import javax.servlet.FilterChain;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
+import org.eclipse.jgit.http.server.ServletUtils;
+import org.eclipse.jgit.http.server.glue.WrappedRequest;
+import org.eclipse.jgit.lib.Config;
+
+import com.google.common.collect.Sets;
+
/** Filter to parse URLs and convert them to {@link GitilesView}s. */
public class ViewFilter extends AbstractHttpFilter {
// TODO(dborowitz): Make this public in JGit (or implement getRegexGroup
@@ -76,12 +80,14 @@
private final GitilesUrls urls;
private final GitilesAccess.Factory accessFactory;
private final VisibilityCache visibilityCache;
+ private final Set<String> archiveExts;
- public ViewFilter(GitilesAccess.Factory accessFactory, GitilesUrls urls,
- VisibilityCache visibilityCache) {
+ public ViewFilter(Config cfg, GitilesAccess.Factory accessFactory,
+ GitilesUrls urls, VisibilityCache visibilityCache) {
this.urls = checkNotNull(urls, "urls");
this.accessFactory = checkNotNull(accessFactory, "accessFactory");
this.visibilityCache = checkNotNull(visibilityCache, "visibilityCache");
+ this.archiveExts = Sets.newHashSet(ArchiveFormat.byExtension(cfg).keySet());
}
@Override
@@ -142,7 +148,7 @@
private GitilesView.Builder parseArchiveCommand(
HttpServletRequest req, String repoName, String path) throws IOException {
String ext = null;
- for (String e : ArchiveServlet.validExtensions()) {
+ for (String e : archiveExts) {
if (path.endsWith(e)) {
path = path.substring(0, path.length() - e.length());
ext = e;
diff --git a/gitiles-servlet/src/main/resources/com/google/gitiles/templates/ObjectDetail.soy b/gitiles-servlet/src/main/resources/com/google/gitiles/templates/ObjectDetail.soy
index 2d3a259..73ac298 100644
--- a/gitiles-servlet/src/main/resources/com/google/gitiles/templates/ObjectDetail.soy
+++ b/gitiles-servlet/src/main/resources/com/google/gitiles/templates/ObjectDetail.soy
@@ -36,7 +36,8 @@
* url: URL to a detail page for the tree entry.
* diffUrl: URL to a diff page for the tree entry's diff in this commit.
* @param logUrl URL to a log page starting at this commit.
- * @param tgzUrl URL to a download link of this commit as a tarball.
+ * @param archiveUrl URL to a download link of this commit as an archive.
+ * @param archiveType type of the archive to download.
*/
{template .commitDetail}
<div class="git-commit">
@@ -48,9 +49,7 @@
<span class="log-link">
[<a href="{$logUrl}">{msg desc="text for the log link"}log{/msg}</a>]
</span>
- <span class="download-link">
- [<a href="{$tgzUrl}">{msg desc="text for the tarball link"}tgz{/msg}</a>]
- </span>
+ <span class="download-link">[<a href="{$archiveUrl}">{$archiveType}</a>]</span>
</td>
<td>{sp}</td>
</tr>
diff --git a/gitiles-servlet/src/test/java/com/google/gitiles/ViewFilterTest.java b/gitiles-servlet/src/test/java/com/google/gitiles/ViewFilterTest.java
index 0f00174..201ac9d 100644
--- a/gitiles-servlet/src/test/java/com/google/gitiles/ViewFilterTest.java
+++ b/gitiles-servlet/src/test/java/com/google/gitiles/ViewFilterTest.java
@@ -36,6 +36,7 @@
import org.eclipse.jgit.internal.storage.dfs.DfsRepositoryDescription;
import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
import org.eclipse.jgit.junit.TestRepository;
+import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.revwalk.RevCommit;
import com.google.common.collect.ImmutableList;
@@ -421,6 +422,7 @@
};
ViewFilter vf = new ViewFilter(
+ new Config(),
new TestGitilesAccess(repo.getRepository()),
TestGitilesUrls.URLS,
new VisibilityCache(false));
