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
This commit is contained in:
@@ -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;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user