Skip to content

Commit

Permalink
Wrap Jetty's Resource type to force an invalid last-modified header
Browse files Browse the repository at this point in the history
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 deephaven#3011
  • Loading branch information
niloc132 committed Oct 31, 2022
1 parent 76b42f9 commit 5938ca6
Show file tree
Hide file tree
Showing 2 changed files with 124 additions and 5 deletions.
Original file line number Diff line number Diff line change
@@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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);
}
Expand Down

0 comments on commit 5938ca6

Please sign in to comment.