From 5938ca6817a2356110982be0d2f6ba784e3af671 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Mon, 31 Oct 2022 12:00:25 -0500 Subject: [PATCH] Wrap Jetty's Resource type to force an invalid last-modified header Jetty always returns the last-modified date of the file, but Gradle unsets the "true" last modified date in favor of ensuring that build files cache only based on their contents. Disabling this feature is possible, but still may present as buggy in Jetty, such as if a 1.0 release occurred, then 2.0, then a 1.1 hotfix - if an installation of 1.1 were later replaced with 2.0, the 1.1 files would be "newer" so would be considered up to date. This patch removes the last-modified value by only returning -1 when requested. We could go further and return an etag that actually maps to the release version or the contents of the file, but that is left for future work. Fixes #3011 --- .../server/jetty/ControlledCacheResource.java | 122 ++++++++++++++++++ .../server/jetty/JettyBackedGrpcServer.java | 7 +- 2 files changed, 124 insertions(+), 5 deletions(-) create mode 100644 server/jetty/src/main/java/io/deephaven/server/jetty/ControlledCacheResource.java diff --git a/server/jetty/src/main/java/io/deephaven/server/jetty/ControlledCacheResource.java b/server/jetty/src/main/java/io/deephaven/server/jetty/ControlledCacheResource.java new file mode 100644 index 00000000000..39ce578353d --- /dev/null +++ b/server/jetty/src/main/java/io/deephaven/server/jetty/ControlledCacheResource.java @@ -0,0 +1,122 @@ +package io.deephaven.server.jetty; + +import org.eclipse.jetty.util.resource.Resource; + +import java.io.File; +import java.io.IOException; +import java.io.InputStream; +import java.net.MalformedURLException; +import java.net.URI; +import java.nio.channels.ReadableByteChannel; + +/** + * Simple wrapper around the Jetty Resource type, to grant us control over caching features. The current implementation + * only removes the last-modified value, but a future version could provide a "real" weak/strong etag. + */ +public class ControlledCacheResource extends Resource { + private final Resource wrapped; + + public ControlledCacheResource(Resource wrapped) { + this.wrapped = wrapped; + } + + @Override + public boolean isContainedIn(Resource r) throws MalformedURLException { + return wrapped.isContainedIn(r); + } + + @Override + public void close() { + wrapped.close(); + } + + @Override + public boolean exists() { + return wrapped.exists(); + } + + @Override + public boolean isDirectory() { + return wrapped.isDirectory(); + } + + @Override + public long lastModified() { + // Always return -1, so that we don't get the build system timestamp. In theory we could return the app startup + // time as well, so that clients that connect don't need to revalidate quite as often, but this could have other + // side effects such as in load balancing with a short-lived old build against a seconds-older new build. + return -1; + } + + @Override + public long length() { + return wrapped.length(); + } + + @Override + public URI getURI() { + return wrapped.getURI(); + } + + @Override + public File getFile() throws IOException { + return wrapped.getFile(); + } + + @Override + public String getName() { + return wrapped.getName(); + } + + @Override + public InputStream getInputStream() throws IOException { + return wrapped.getInputStream(); + } + + @Override + public ReadableByteChannel getReadableByteChannel() throws IOException { + return wrapped.getReadableByteChannel(); + } + + @Override + public boolean delete() throws SecurityException { + return wrapped.delete(); + } + + @Override + public boolean renameTo(Resource dest) throws SecurityException { + return wrapped.renameTo(dest); + } + + @Override + public String[] list() { + return wrapped.list(); + } + + @Override + public Resource addPath(String path) throws IOException, MalformedURLException { + // Re-wrap any instance that might be returned + return new ControlledCacheResource(wrapped.addPath(path)); + } + + @Override + public String toString() { + // Jetty's CachedContentFactory.CachedHttpContent requires that toString return the underlying URL found on + // disk, or else the mime lookup from content type won't resolve anything. + return wrapped.toString(); + } + + @Override + public int hashCode() { + // As with toString, delegating this to the wrapped instance, just in case there is some specific, expected + // behavior + return wrapped.hashCode(); + } + + @Override + public boolean equals(Object obj) { + // As with toString, delegating this to the wrapped instance, just in case there is some specific, expected + // behavior + return wrapped.equals(obj); + } +} diff --git a/server/jetty/src/main/java/io/deephaven/server/jetty/JettyBackedGrpcServer.java b/server/jetty/src/main/java/io/deephaven/server/jetty/JettyBackedGrpcServer.java index b5964a258b4..c9401fafe2b 100644 --- a/server/jetty/src/main/java/io/deephaven/server/jetty/JettyBackedGrpcServer.java +++ b/server/jetty/src/main/java/io/deephaven/server/jetty/JettyBackedGrpcServer.java @@ -3,7 +3,6 @@ */ package io.deephaven.server.jetty; -import io.deephaven.server.config.ServerConfig; import io.deephaven.server.runner.GrpcServer; import io.deephaven.ssl.config.CiphersIntermediate; import io.deephaven.ssl.config.ProtocolsIntermediate; @@ -12,7 +11,6 @@ import io.deephaven.ssl.config.impl.KickstartUtils; import io.grpc.servlet.web.websocket.GrpcWebsocket; import io.grpc.servlet.web.websocket.MultiplexedWebSocketServerStream; -import io.grpc.servlet.web.websocket.MultiplexedWebsocketStreamImpl; import io.grpc.servlet.web.websocket.WebSocketServerStream; import io.grpc.servlet.jakarta.web.GrpcWebFilter; import jakarta.servlet.DispatcherType; @@ -43,8 +41,6 @@ import java.io.UncheckedIOException; import java.net.URL; import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collections; import java.util.EnumSet; import java.util.HashMap; import java.util.Map; @@ -71,7 +67,8 @@ public JettyBackedGrpcServer( try { String knownFile = "/ide/index.html"; URL ide = JettyBackedGrpcServer.class.getResource(knownFile); - context.setBaseResource(Resource.newResource(ide.toExternalForm().replace("!" + knownFile, "!/"))); + Resource jarContents = Resource.newResource(ide.toExternalForm().replace("!" + knownFile, "!/")); + context.setBaseResource(new ControlledCacheResource(jarContents)); } catch (IOException ioException) { throw new UncheckedIOException(ioException); }