ResourceServlet: Consistently use class load time as mtime

The GWT UI static servlets (e.g. WarGwtUiServlet) were already using
class load time as the "mtime" of a file, to account for the fact that
the build system always sets the mtime of zipfile entries to the Unix
epoch. (This is intentional, to keep bazel builds reproducible.)

PolyGerrit has the same problem served from a WAR file, but was not
applying this hack. Complicating the situation is that the same class
(PolyGerritUiServlet) is used for the dev server, where we can trust
the mtime. Hackily check whether the path in question is on-disk in the
implementation of getLastModifiedTime.

There was also a bug in ResourceServlet#maybeStream that was bypassing
getLastModifiedTime when checking If-Modified-Since; fix that as well.

While we're in here, fix the other broken class in this hierarchy,
WarDocServlet.

In retrospect, it may have been a mistake to make ResourceServlet try to
handle all possible caching behaviors, since it makes bugs like this all
too easy to introduce. Long term, may want to disentangle the on-disk
implementation, which may depend on mtime to determine staleness, from
the zipfile implementation, which shouldn't.

Bug: Issue 6885
Change-Id: Iea041774c1005cb9918c462d01aef05cff61da23
(cherry picked from commit 65c8345a96)
This commit is contained in:
Dave Borowitz
2017-10-01 11:15:40 +01:00
committed by Paladox none
parent 1a09758fd8
commit 3a3aed6ead
3 changed files with 30 additions and 2 deletions

View File

@@ -15,11 +15,17 @@
package com.google.gerrit.httpd.raw;
import com.google.common.cache.Cache;
import com.google.gerrit.common.TimeUtil;
import java.io.IOException;
import java.nio.file.FileSystems;
import java.nio.file.Path;
import java.nio.file.attribute.FileTime;
class PolyGerritUiServlet extends ResourceServlet {
private static final long serialVersionUID = 1L;
private static final FileTime NOW = FileTime.fromMillis(TimeUtil.nowMs());
private final Path ui;
PolyGerritUiServlet(Cache<Path, Resource> cache, Path ui) {
@@ -31,4 +37,16 @@ class PolyGerritUiServlet extends ResourceServlet {
protected Path getResourcePath(String pathInfo) {
return ui.resolve(pathInfo);
}
@Override
protected FileTime getLastModifiedTime(Path p) throws IOException {
if (ui.getFileSystem().equals(FileSystems.getDefault())) {
// Assets are being served from disk, so we can trust the mtime.
return super.getLastModifiedTime(p);
}
// Assume this FileSystem is serving from a WAR. All WAR outputs from the build process have
// mtimes of 1980/1/1, so we can't trust it, and return the initialization time of this class
// instead.
return NOW;
}
}

View File

@@ -31,7 +31,6 @@ import com.google.common.base.CharMatcher;
import com.google.common.cache.Cache;
import com.google.common.collect.ImmutableMap;
import com.google.common.hash.Hashing;
import com.google.gerrit.common.FileUtil;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.httpd.HtmlDomUtil;
import com.google.gwtexpui.server.CacheHeaders;
@@ -252,7 +251,7 @@ public abstract class ResourceServlet extends HttpServlet {
return true;
}
long lastModified = FileUtil.lastModified(p);
long lastModified = getLastModifiedTime(p).toMillis();
if (req.getDateHeader(IF_MODIFIED_SINCE) >= lastModified) {
rsp.setStatus(SC_NOT_MODIFIED);
return true;

View File

@@ -15,12 +15,16 @@
package com.google.gerrit.httpd.raw;
import com.google.common.cache.Cache;
import com.google.gerrit.common.TimeUtil;
import java.nio.file.FileSystem;
import java.nio.file.Path;
import java.nio.file.attribute.FileTime;
class WarDocServlet extends ResourceServlet {
private static final long serialVersionUID = 1L;
private static final FileTime NOW = FileTime.fromMillis(TimeUtil.nowMs());
private final FileSystem warFs;
WarDocServlet(Cache<Path, Resource> cache, FileSystem warFs) {
@@ -32,4 +36,11 @@ class WarDocServlet extends ResourceServlet {
protected Path getResourcePath(String pathInfo) {
return warFs.getPath("/Documentation/" + pathInfo);
}
@Override
protected FileTime getLastModifiedTime(Path p) {
// Return initialization time of this class, since the WAR outputs from the build process all
// have mtimes of 1980/1/1.
return NOW;
}
}