Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce caching of ResteasyUriInfo.InitData #4563

Closed
wants to merge 1 commit into from
Closed

Introduce caching of ResteasyUriInfo.InitData #4563

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions .../deployment/src/main/java/io/quarkus/resteasy/deployment/ResteasyStandaloneBuildStep.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ public class ResteasyStandaloneBuildStep {
protected static final String META_INF_RESOURCES_SLASH = "META-INF/resources/";
protected static final String META_INF_RESOURCES = "META-INF/resources";

private static final int URI_INFO_CACHE_MAX_SIZE = 100; // TODO we should set this via config

public static final class ResteasyStandaloneBuildItem extends SimpleBuildItem {

final String deploymentRootPath;
Expand Down Expand Up @@ -88,10 +90,10 @@ public void staticInit(ResteasyStandaloneRecorder recorder,
}
rootPath += deploymentRootPath;
}
recorder.staticInit(deployment.getDeployment(), rootPath, knownPaths);
recorder.staticInit(deployment.getDeployment(), rootPath, knownPaths, URI_INFO_CACHE_MAX_SIZE);

} else if (!knownPaths.isEmpty()) {
recorder.staticInit(null, rootPath, knownPaths);
recorder.staticInit(null, rootPath, knownPaths, URI_INFO_CACHE_MAX_SIZE);
}

if (deployment != null || !knownPaths.isEmpty()) {
Expand Down
7 changes: 5 additions & 2 deletions ...time/src/main/java/io/quarkus/resteasy/runtime/standalone/ResteasyStandaloneRecorder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,14 +78,16 @@ public static void setHotDeploymentResources(List<Path> resources) {
private static ResteasyDeployment deployment;
private static Set<String> knownPaths;
private static String contextPath;
private static int resteasyUriInfoCacheMaxSize;

public void staticInit(ResteasyDeployment dep, String path, Set<String> known) {
public void staticInit(ResteasyDeployment dep, String path, Set<String> known, int uriInfoCacheMaxSize) {
if (dep != null) {
deployment = dep;
deployment.start();
}
knownPaths = known;
contextPath = path;
resteasyUriInfoCacheMaxSize = uriInfoCacheMaxSize;
}

public Consumer<Route> start(RuntimeValue<Vertx> vertx,
Expand Down Expand Up @@ -163,7 +165,8 @@ public void accept(Route route) {
public Handler<RoutingContext> vertxRequestHandler(RuntimeValue<Vertx> vertx,
BeanContainer beanContainer) {
if (deployment != null) {
return new VertxRequestHandler(vertx.getValue(), beanContainer, deployment, contextPath, ALLOCATOR);
return new VertxRequestHandler(vertx.getValue(), beanContainer, deployment, contextPath,
resteasyUriInfoCacheMaxSize, ALLOCATOR);
}
return null;
}
Expand Down
38 changes: 37 additions & 1 deletion ...asy/runtime/src/main/java/io/quarkus/resteasy/runtime/standalone/VertxRequestHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

import java.io.IOException;
import java.io.InputStream;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;

import javax.enterprise.inject.Instance;
import javax.enterprise.inject.spi.CDI;
Expand Down Expand Up @@ -35,20 +37,25 @@ public class VertxRequestHandler implements Handler<RoutingContext> {
protected final Vertx vertx;
protected final RequestDispatcher dispatcher;
protected final String rootPath;
protected final int resteasyUriInfoCacheMaxSize;
protected final BufferAllocator allocator;
protected final BeanContainer beanContainer;
protected final CurrentIdentityAssociation association;

private final Map<String, ResteasyUriInfo.InitData> resteasyUriInfoInitDataMap = new ConcurrentHashMap<>();

public VertxRequestHandler(Vertx vertx,
BeanContainer beanContainer,
ResteasyDeployment deployment,
String rootPath,
int resteasyUriInfoCacheMaxSize,
BufferAllocator allocator) {
this.vertx = vertx;
this.beanContainer = beanContainer;
this.dispatcher = new RequestDispatcher((SynchronousDispatcher) deployment.getDispatcher(),
deployment.getProviderFactory(), null);
this.rootPath = rootPath;
this.resteasyUriInfoCacheMaxSize = resteasyUriInfoCacheMaxSize;
this.allocator = allocator;
Instance<CurrentIdentityAssociation> association = CDI.current().select(CurrentIdentityAssociation.class);
this.association = association.isResolvable() ? association.get() : null;
Expand Down Expand Up @@ -90,7 +97,23 @@ private void dispatch(RoutingContext routingContext, InputStream is, VertxOutput
try {
Context ctx = vertx.getOrCreateContext();
HttpServerRequest request = routingContext.request();
ResteasyUriInfo uriInfo = VertxUtil.extractUriInfo(request, rootPath);

String uriString = VertxUtil.getUriString(request);
ResteasyUriInfo.InitData resteasyUriInfoCachedInitData = null;
boolean setInitDataUponSuccess = false;
if (ResteasyUriInfo.InitData.canBeCached(uriString) && resteasyUriInfoCacheMaxSize > 0) {
String cacheKey = ResteasyUriInfo.InitData.getCacheKey(uriString, rootPath);
resteasyUriInfoCachedInitData = resteasyUriInfoInitDataMap.get(cacheKey);
if (resteasyUriInfoCachedInitData == null) {
setInitDataUponSuccess = true;
}
}
if (resteasyUriInfoCachedInitData == null) {
resteasyUriInfoCachedInitData = new ResteasyUriInfo.InitData(uriString, rootPath);
}

ResteasyUriInfo uriInfo = new ResteasyUriInfo(uriString, rootPath, resteasyUriInfoCachedInitData);

ResteasyHttpHeaders headers = VertxUtil.extractHttpHeaders(request);
HttpServerResponse response = request.response();
VertxHttpResponse vertxResponse = new VertxHttpResponse(request, dispatcher.getProviderFactory(),
Expand All @@ -114,6 +137,19 @@ private void dispatch(RoutingContext routingContext, InputStream is, VertxOutput
if (!vertxRequest.getAsyncContext().isSuspended()) {
try {
vertxResponse.finish();
// the reason we only cache successful responses is to ensure that a torrent of erroneous URLs
// doesn't fill up the cache and cause a DoS
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you should cache unsuccesfull responses as well, especially if we go for having a proper cache.

DDoS are caused by the system being overwhelmed; hitting the "bad response cache" would be the best way to avoid it as the cache would help fighting load.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree 💯%. If we have a proper cache than all this nastiness can go away.

if (setInitDataUponSuccess && vertxResponse.getStatus() >= 200 && vertxResponse.getStatus() <= 300) {
// we don't want the cache to grow unbounded since values path params could potentially be infinite
// and if left unchecked would take up all memory if the application is up for long enough
if (resteasyUriInfoInitDataMap.size() > resteasyUriInfoCacheMaxSize) {
resteasyUriInfoInitDataMap.clear(); // this is super lame and should probably be revisited
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes it's dangerous. See my other comments on the design discussion.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we have the RESTEasy part of the PR in which puts in the necessary hooks, we can absolutely improve this based on what you mention about the existence or not of Caffeine.
Note that the way I have done the RESTEasy PR (and as of course you have seen from looking at this PR), the caching itself is handled exclusively in Quarkus so we can probably get as creative as we like.

}
// this could potentially be written multiple times for initial requests but it doesn't matter
// since the data is always the same
resteasyUriInfoInitDataMap.put(ResteasyUriInfo.InitData.getCacheKey(uriString, rootPath),
resteasyUriInfoCachedInitData);
}
} catch (IOException e) {
log.error("Unexpected failure", e);
}
Expand Down
16 changes: 10 additions & 6 deletions ...ions/resteasy/runtime/src/main/java/io/quarkus/resteasy/runtime/standalone/VertxUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public class VertxUtil {

private static final Pattern COMMA_PATTERN = Pattern.compile(",");

public static ResteasyUriInfo extractUriInfo(HttpServerRequest req, String contextPath) {
public static String getUriString(HttpServerRequest req) {
String uri = req.absoluteURI();
String protocol = req.scheme();

Expand All @@ -44,12 +44,16 @@ public static ResteasyUriInfo extractUriInfo(HttpServerRequest req, String conte
uriString = protocol + "://" + host + uri;
}

// ResteasyUriInfo expects a context path to start with a "/" character
if (!contextPath.startsWith("/")) {
contextPath = "/" + contextPath;
}
return uriString;
}

return new ResteasyUriInfo(uriString, contextPath);
public static ResteasyUriInfo.InitData extractUriInfoInitData(String uriString, String contextPath,
Map<String, ResteasyUriInfo.InitData> resteasyUriInfoInitDataMap) {
if (ResteasyUriInfo.InitData.canBeCached(uriString)) {
String cacheKey = ResteasyUriInfo.InitData.getCacheKey(uriString, contextPath);
return resteasyUriInfoInitDataMap.get(cacheKey);
}
return null;
}

public static ResteasyHttpHeaders extractHttpHeaders(HttpServerRequest request) {
Expand Down