Redirect CatServlet requests to DownloadContent
Reimplement CatServlet as a redirect to the DownloadContent
REST API method. Enable DownloadServlet to serve content from
parent commits upon request.
It isn't possible to reference a base commit as a revision ID
in a REST URL. A new parent parameter for DownloadContent
provides access to parent commits, replacing the obsolete
suffix parameter. A positive value of N will reference the
Nth parent commit for the specified revision. If the parameter
is missing or its value is zero, the patch commit is referenced.
The suffix for the download is now computed directly by the
DownloadContent method. The suffix is "_new" for a file from
a patch set, "_old" for a file from the base commit when there's
a single parent, or "oldN" for the Nth parent commit of many.
Change-Id: Ibedf07a62ddf6661f4050da21df7a3c7a8681c0c
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index 4b57827..cbd748c 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -3347,10 +3347,13 @@
The HTTP resource Content-Type is dependent on the file type: the
applicable type for safe files, or "application/zip" for unsafe files.
-The `suffix` parameter can be specified to decorate the names of the files.
-The suffix is inserted between the base filename and the random component or
-extension, or appended to the filename if neither such component is present.
-Only the lowercase Roman letters a-z are permitted; other characters are ignored.
+The optional, integer-valued `parent` parameter can be specified to request
+the named file from a parent commit of the specified revision. The value is
+the 1-based index of the parent's position in the commit object. If the
+parameter is omitted or the value non-positive, the patch set is referenced.
+
+Filenames are decorated with a suffix of `_new` for the current patch,
+`_old` for the only parent, or `_oldN` for the Nth parent of many.
.Request
----
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/raw/CatServlet.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/raw/CatServlet.java
index 3cbb68b..ded5ad9 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/raw/CatServlet.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/raw/CatServlet.java
@@ -15,7 +15,6 @@
package com.google.gerrit.httpd.raw;
import com.google.common.base.Optional;
-import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.Url;
import com.google.gerrit.reviewdb.client.Change;
@@ -27,36 +26,20 @@
import com.google.gerrit.server.edit.ChangeEdit;
import com.google.gerrit.server.edit.ChangeEditUtil;
import com.google.gerrit.server.git.GitRepositoryManager;
-import com.google.gerrit.server.mime.FileTypeRegistry;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.NoSuchChangeException;
-import com.google.gwtexpui.server.CacheHeaders;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
-import eu.medsea.mimeutil.MimeType;
-
-import org.eclipse.jgit.errors.RepositoryNotFoundException;
-import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.ObjectId;
-import org.eclipse.jgit.lib.ObjectLoader;
import org.eclipse.jgit.lib.ObjectReader;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
-import org.eclipse.jgit.treewalk.TreeWalk;
-import org.eclipse.jgit.util.NB;
-import java.io.FilterOutputStream;
import java.io.IOException;
-import java.io.OutputStream;
-import java.io.UnsupportedEncodingException;
-import java.security.MessageDigest;
-import java.security.SecureRandom;
-import java.util.zip.ZipEntry;
-import java.util.zip.ZipOutputStream;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
@@ -74,26 +57,17 @@
@SuppressWarnings("serial")
@Singleton
public class CatServlet extends HttpServlet {
- private static final MimeType ZIP = new MimeType("application/zip");
private final Provider<ReviewDb> requestDb;
- private final GitRepositoryManager repoManager;
- private final SecureRandom rng;
- private final FileTypeRegistry registry;
private final Provider<CurrentUser> userProvider;
private final ChangeControl.GenericFactory changeControl;
private final ChangeEditUtil changeEditUtil;
@Inject
- CatServlet(GitRepositoryManager grm,
- Provider<ReviewDb> sf,
- FileTypeRegistry ftr,
+ CatServlet(Provider<ReviewDb> sf,
ChangeControl.GenericFactory ccf,
Provider<CurrentUser> usrprv,
ChangeEditUtil ceu) {
requestDb = sf;
- repoManager = grm;
- rng = new SecureRandom();
- registry = ftr;
changeControl = ccf;
userProvider = usrprv;
changeEditUtil = ceu;
@@ -130,7 +104,6 @@
if (c < 0) {
side = 0;
-
} else {
try {
side = Integer.parseInt(keyStr.substring(c + 1));
@@ -150,15 +123,11 @@
}
final Change.Id changeId = patchKey.getParentKey().getParentKey();
- final Project project;
- final String revision;
+ String revision;
try {
final ReviewDb db = requestDb.get();
final ChangeControl control = changeControl.validateFor(changeId,
userProvider.get());
-
- project = control.getProject();
-
if (patchKey.getParentKey().get() == 0) {
// change edit
try {
@@ -190,184 +159,9 @@
return;
}
- ObjectLoader blobLoader;
- RevCommit fromCommit;
- String suffix;
String path = patchKey.getFileName();
- try (Repository repo = repoManager.openRepository(project.getNameKey())) {
- try (ObjectReader reader = repo.newObjectReader();
- RevWalk rw = new RevWalk(reader)) {
- RevCommit c;
-
- c = rw.parseCommit(ObjectId.fromString(revision));
- if (side == 0) {
- fromCommit = c;
- suffix = "new";
-
- } else if (1 <= side && side - 1 < c.getParentCount()) {
- fromCommit = rw.parseCommit(c.getParent(side - 1));
- if (c.getParentCount() == 1) {
- suffix = "old";
- } else {
- suffix = "old" + side;
- }
-
- } else {
- rsp.sendError(HttpServletResponse.SC_NOT_FOUND);
- return;
- }
-
- try (TreeWalk tw = TreeWalk.forPath(reader, path, fromCommit.getTree())) {
- if (tw == null) {
- rsp.sendError(HttpServletResponse.SC_NOT_FOUND);
- return;
- }
-
- if (tw.getFileMode(0).getObjectType() == Constants.OBJ_BLOB) {
- blobLoader = reader.open(tw.getObjectId(0), Constants.OBJ_BLOB);
-
- } else {
- rsp.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
- return;
- }
- }
- }
- } catch (RepositoryNotFoundException e) {
- getServletContext().log("Cannot open repository", e);
- rsp.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
- return;
- } catch (IOException | RuntimeException e) {
- getServletContext().log("Cannot read repository", e);
- rsp.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
- return;
- }
-
- final byte[] raw =
- blobLoader.isLarge() ? null : blobLoader.getCachedBytes();
- final long when = fromCommit.getCommitTime() * 1000L;
-
- rsp.setDateHeader("Last-Modified", when);
- CacheHeaders.setNotCacheable(rsp);
-
- try (OutputStream out = openOutputStream(
- req, rsp, blobLoader, fromCommit, when, path, suffix, raw)) {
- if (raw != null) {
- out.write(raw);
- } else {
- blobLoader.copyTo(out);
- }
- }
+ String restUrl = String.format("%s/changes/%d/revisions/%s/files/%s/download?parent=%d",
+ req.getContextPath(), changeId.get(), revision, Url.encode(path), side);
+ rsp.sendRedirect(restUrl);
}
-
- private OutputStream openOutputStream(HttpServletRequest req,
- HttpServletResponse rsp, ObjectLoader blobLoader,
- RevCommit fromCommit, long when, String path, String suffix, byte[] raw)
- throws IOException {
- MimeType contentType = registry.getMimeType(path, raw);
- if (!registry.isSafeInline(contentType)) {
- // The content may not be safe to transmit inline, as a browser might
- // interpret it as HTML or JavaScript hosted by this site. Such code
- // might then run in the site's security domain, and may be able to use
- // the user's cookies to perform unauthorized actions.
- //
- // Usually, wrapping the content into a ZIP file forces the browser to
- // save the content to the local system instead.
- //
-
- rsp.setContentType(ZIP.toString());
- rsp.setHeader("Content-Disposition", "attachment; filename=\""
- + safeFileName(path, suffix) + ".zip" + "\"");
-
- final ZipOutputStream zo = new ZipOutputStream(rsp.getOutputStream());
-
- ZipEntry e = new ZipEntry(safeFileName(path, rand(req, suffix)));
- e.setComment(fromCommit.name() + ":" + path);
- e.setSize(blobLoader.getSize());
- e.setTime(when);
- zo.putNextEntry(e);
- return new FilterOutputStream(zo) {
- @Override
- public void close() throws IOException {
- try {
- zo.closeEntry();
- } finally {
- super.close();
- }
- }
- };
-
- } else {
- rsp.setContentType(contentType.toString());
- rsp.setHeader("Content-Length", "" + blobLoader.getSize());
-
- return rsp.getOutputStream();
- }
- }
-
- private static String safeFileName(String fileName, final String suffix) {
- // Convert a file path (e.g. "src/Init.c") to a safe file name with
- // no meta-characters that might be unsafe on any given platform.
- //
- final int slash = fileName.lastIndexOf('/');
- if (slash >= 0) {
- fileName = fileName.substring(slash + 1);
- }
-
- final StringBuilder r = new StringBuilder(fileName.length());
- for (int i = 0; i < fileName.length(); i++) {
- final char c = fileName.charAt(i);
- if (c == '_' || c == '-' || c == '.' || c == '@') {
- r.append(c);
-
- } else if ('0' <= c && c <= '9') {
- r.append(c);
-
- } else if ('A' <= c && c <= 'Z') {
- r.append(c);
-
- } else if ('a' <= c && c <= 'z') {
- r.append(c);
-
- } else if (c == ' ' || c == '\n' || c == '\r' || c == '\t') {
- r.append('-');
-
- } else {
- r.append('_');
- }
- }
- fileName = r.toString();
-
- final int ext = fileName.lastIndexOf('.');
- if (ext <= 0) {
- return fileName + "_" + suffix;
-
- } else {
- return fileName.substring(0, ext) + "_" + suffix
- + fileName.substring(ext);
- }
- }
-
- private String rand(final HttpServletRequest req, final String suffix)
- throws UnsupportedEncodingException {
- // Produce a random suffix that is difficult (or nearly impossible)
- // for an attacker to guess in advance. This reduces the risk that
- // an attacker could upload a *.class file and have us send a ZIP
- // that can be invoked through an applet tag in the victim's browser.
- //
- final MessageDigest md = Constants.newMessageDigest();
- final byte[] buf = new byte[8];
-
- NB.encodeInt32(buf, 0, req.getRemotePort());
- md.update(req.getRemoteAddr().getBytes("UTF-8"));
- md.update(buf, 0, 4);
-
- NB.encodeInt64(buf, 0, TimeUtil.nowMs());
- md.update(buf, 0, 8);
-
- rng.nextBytes(buf);
- md.update(buf, 0, 8);
-
- return suffix + "-" + ObjectId.fromRaw(md.digest()).name();
- }
-
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/DownloadContent.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/DownloadContent.java
index 733ff0b..24c2f0e 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/DownloadContent.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/DownloadContent.java
@@ -30,8 +30,8 @@
public class DownloadContent implements RestReadView<FileResource> {
private final FileContentUtil fileContentUtil;
- @Option(name = "--suffix")
- private String suffix;
+ @Option(name = "--parent")
+ private Integer parent;
@Inject
DownloadContent(FileContentUtil fileContentUtil) {
@@ -47,6 +47,6 @@
rsrc.getRevision().getControl().getProjectControl().getProjectState();
ObjectId revstr = ObjectId.fromString(
rsrc.getRevision().getPatchSet().getRevision().get());
- return fileContentUtil.downloadContent(projectState, revstr, path, suffix);
+ return fileContentUtil.downloadContent(projectState, revstr, path, parent);
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/FileContentUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/FileContentUtil.java
index fef09a2..04a5851 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/FileContentUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/FileContentUtil.java
@@ -128,14 +128,20 @@
}
public BinaryResult downloadContent(ProjectState project, ObjectId revstr,
- String path, @Nullable String suffix)
+ String path, @Nullable Integer parent)
throws ResourceNotFoundException, IOException {
- suffix = Strings.emptyToNull(
- LOWERCASE_OR_DIGITS.retainFrom(Strings.nullToEmpty(suffix)));
-
try (Repository repo = openRepository(project);
RevWalk rw = new RevWalk(repo)) {
+ String suffix = "new";
RevCommit commit = rw.parseCommit(revstr);
+ if (parent != null && parent > 0) {
+ if (commit.getParentCount() == 1) {
+ suffix = "old";
+ } else {
+ suffix = "old" + parent;
+ }
+ commit = rw.parseCommit(commit.getParent(parent - 1));
+ }
ObjectReader reader = rw.getObjectReader();
TreeWalk tw = TreeWalk.forPath(reader, path, commit.getTree());
if (tw == null) {
