Skip to content

Commit

Permalink
Merge pull request #23552 from cescoffier/restrict-access-to-vertx-co…
Browse files Browse the repository at this point in the history
…ntext-locals

Restrict access to some method from the Vert.x Context which are considered risky
  • Loading branch information
cescoffier authored Feb 18, 2022
2 parents 96b586c + 70bcd82 commit 3493e06
Show file tree
Hide file tree
Showing 24 changed files with 552 additions and 99 deletions.
2 changes: 1 addition & 1 deletion bom/application/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
<microprofile-rest-client.version>2.0</microprofile-rest-client.version>
<microprofile-jwt.version>1.2</microprofile-jwt.version>
<microprofile-lra.version>1.0</microprofile-lra.version>
<smallrye-common.version>1.9.0</smallrye-common.version>
<smallrye-common.version>1.10.0</smallrye-common.version>
<smallrye-config.version>2.9.0</smallrye-config.version>
<smallrye-health.version>3.2.0</smallrye-health.version>
<smallrye-metrics.version>3.0.4</smallrye-metrics.version>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import io.quarkus.grpc.GrpcClient;
import io.quarkus.grpc.server.services.HelloService;
import io.quarkus.test.QuarkusUnitTest;
import io.smallrye.common.vertx.VertxContext;
import io.smallrye.mutiny.Uni;
import io.vertx.core.impl.ContextInternal;
import io.vertx.core.impl.EventLoopContext;
Expand Down Expand Up @@ -80,7 +81,7 @@ public String invokeFromIoThread(String s) {

public String invokeFromDuplicatedContext(String s) {
Context root = vertx.getOrCreateContext();
ContextInternal duplicate = ((ContextInternal) root.getDelegate()).duplicate();
ContextInternal duplicate = (ContextInternal) VertxContext.getOrCreateDuplicatedContext(root.getDelegate());
return Uni.createFrom().<String> emitter(e -> {
duplicate.runOnContext(x -> {
service.sayHello(HelloRequest.newBuilder().setName(s).build())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@
import io.quarkus.grpc.GrpcClient;
import io.quarkus.grpc.server.services.HelloService;
import io.quarkus.test.QuarkusUnitTest;
import io.smallrye.common.vertx.VertxContext;
import io.smallrye.mutiny.Uni;
import io.vertx.core.impl.ContextInternal;
import io.vertx.core.impl.EventLoopContext;
import io.vertx.core.impl.WorkerContext;
import io.vertx.mutiny.core.Context;
Expand Down Expand Up @@ -88,7 +88,7 @@ public String invokeFromIoThread(String s) {

public String invokeFromDuplicatedContext(String s) {
Context root = vertx.getOrCreateContext();
ContextInternal duplicate = ((ContextInternal) root.getDelegate()).duplicate();
io.vertx.core.Context duplicate = VertxContext.getOrCreateDuplicatedContext(root.getDelegate());
return Uni.createFrom().<String> emitter(e -> {
duplicate.runOnContext(x -> {
service.sayHello(HelloRequest.newBuilder().setName(s).build())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,9 @@
import static org.assertj.core.api.Assertions.assertThat;

import io.quarkus.arc.Arc;
import io.smallrye.common.vertx.VertxContext;
import io.vertx.core.Context;
import io.vertx.core.Vertx;
import io.vertx.core.impl.EventLoopContext;
import io.vertx.core.impl.WorkerContext;

public class AssertHelper {

Expand All @@ -16,20 +15,15 @@ public static void assertThatTheRequestScopeIsActive() {

public static void assertRunOnEventLoop() {
assertThat(Vertx.currentContext()).isNotNull();
assertThat(Vertx.currentContext().isEventLoopContext());
assertThat(Vertx.currentContext().isEventLoopContext()).isTrue();
assertThat(Thread.currentThread().getName()).contains("eventloop");
}

public static Context assertRunOnDuplicatedContext() {
assertThat(Vertx.currentContext()).isNotNull();
assertThat(isRootContext(Vertx.currentContext())).isFalse();
assertThat(VertxContext.isOnDuplicatedContext()).isTrue();
return Vertx.currentContext();
}

private static boolean isRootContext(Context context) {
return context instanceof EventLoopContext || context instanceof WorkerContext;
}

public static void assertRunOnWorker() {
assertThat(Vertx.currentContext()).isNotNull();
assertThat(Thread.currentThread().getName()).contains("executor");
Expand Down
4 changes: 4 additions & 0 deletions extensions/grpc/runtime/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,10 @@
<groupId>io.smallrye.common</groupId>
<artifactId>smallrye-common-annotation</artifactId>
</dependency>
<dependency>
<groupId>io.smallrye.common</groupId>
<artifactId>smallrye-common-vertx-context</artifactId>
</dependency>

<!-- Test dependencies -->
<dependency>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,9 @@
import io.grpc.ServerCallHandler;
import io.grpc.ServerInterceptor;
import io.quarkus.grpc.GlobalInterceptor;
import io.smallrye.common.vertx.VertxContext;
import io.vertx.core.Context;
import io.vertx.core.Vertx;
import io.vertx.core.impl.ContextInternal;
import io.vertx.core.impl.EventLoopContext;
import io.vertx.core.impl.WorkerContext;

@ApplicationScoped
@GlobalInterceptor
Expand All @@ -27,7 +25,7 @@ public GrpcDuplicatedContextGrpcInterceptor() {
}

private static boolean isRootContext(Context context) {
return context instanceof EventLoopContext || context instanceof WorkerContext;
return !VertxContext.isDuplicatedContext(context);
}

@Override
Expand All @@ -40,10 +38,7 @@ public <ReqT, RespT> ServerCall.Listener<ReqT> interceptCall(ServerCall<ReqT, Re

if (capturedVertxContext != null) {
// If we are not on a duplicated context, create and switch.
Context local = capturedVertxContext;
if (isRootContext(capturedVertxContext)) {
local = ((ContextInternal) capturedVertxContext).duplicate();
}
Context local = VertxContext.getOrCreateDuplicatedContext(capturedVertxContext);

// Must be sure to call next.startCall on the right context
return new ListenedOnDuplicatedContext<>(() -> next.startCall(call, headers), local);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
import io.quarkus.runtime.LaunchMode;
import io.quarkus.runtime.RuntimeValue;
import io.quarkus.vertx.core.deployment.CoreVertxBuildItem;
import io.quarkus.vertx.deployment.CopyVertxContextDataBuildItem;

public class OpenTelemetryProcessor {
static class OpenTelemetryEnabled implements BooleanSupplier {
Expand Down Expand Up @@ -138,11 +137,6 @@ void storeVertxOnContextStorage(OpenTelemetryRecorder recorder, CoreVertxBuildIt
recorder.storeVertxOnContextStorage(vertx.getVertx());
}

@BuildStep
CopyVertxContextDataBuildItem copyVertxContextData() {
return new CopyVertxContextDataBuildItem(QuarkusContextStorage.ACTIVE_CONTEXT);
}

public static boolean isClassPresent(String classname) {
try {
Class.forName(classname, false, Thread.currentThread().getContextClassLoader());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,18 @@

import static io.opentelemetry.api.trace.SpanKind.INTERNAL;
import static io.opentelemetry.api.trace.SpanKind.SERVER;
import static java.net.HttpURLConnection.HTTP_OK;
import static org.junit.jupiter.api.Assertions.assertEquals;

import java.util.List;

import javax.enterprise.context.ApplicationScoped;
import javax.enterprise.event.Observes;
import javax.inject.Inject;
import javax.ws.rs.client.ClientBuilder;
import javax.ws.rs.client.WebTarget;
import javax.ws.rs.core.Response;
import javax.ws.rs.core.UriBuilder;

import org.jboss.shrinkwrap.api.ShrinkWrap;
import org.jboss.shrinkwrap.api.spec.JavaArchive;
Expand All @@ -19,7 +25,10 @@
import io.opentelemetry.extension.annotations.SpanAttribute;
import io.opentelemetry.extension.annotations.WithSpan;
import io.opentelemetry.sdk.trace.data.SpanData;
import io.quarkus.runtime.StartupEvent;
import io.quarkus.test.QuarkusUnitTest;
import io.smallrye.config.SmallRyeConfig;
import io.vertx.ext.web.Router;

public class WithSpanInterceptorTest {
@RegisterExtension
Expand Down Expand Up @@ -79,6 +88,15 @@ void spanChild() {
assertEquals(spanItems.get(0).getParentSpanId(), spanItems.get(1).getSpanId());
}

@Test
void spanCdiRest() {
spanBean.spanRestClient();
List<SpanData> spanItems = spanExporter.getFinishedSpanItems(4);
assertEquals(spanItems.get(0).getTraceId(), spanItems.get(1).getTraceId());
assertEquals(spanItems.get(0).getTraceId(), spanItems.get(2).getTraceId());
assertEquals(spanItems.get(0).getTraceId(), spanItems.get(3).getTraceId());
}

@ApplicationScoped
public static class SpanBean {
@WithSpan
Expand Down Expand Up @@ -108,6 +126,14 @@ public void spanArgs(@SpanAttribute(value = "arg") String arg) {
public void spanChild() {
spanChildBean.spanChild();
}

@Inject
SpanRestClient spanRestClient;

@WithSpan
public void spanRestClient() {
spanRestClient.spanRestClient();
}
}

@ApplicationScoped
Expand All @@ -117,4 +143,28 @@ public void spanChild() {

}
}

@ApplicationScoped
public static class SpanRestClient {
@Inject
SmallRyeConfig config;

@WithSpan
public void spanRestClient() {
WebTarget target = ClientBuilder.newClient()
.target(UriBuilder.fromUri(config.getRawValue("test.url")).path("hello"));
Response response = target.request().get();
assertEquals(HTTP_OK, response.getStatus());
}
}

@ApplicationScoped
public static class HelloRouter {
@Inject
Router router;

public void register(@Observes StartupEvent ev) {
router.get("/hello").handler(rc -> rc.response().end("hello"));
}
}
}
4 changes: 4 additions & 0 deletions extensions/opentelemetry/opentelemetry/runtime/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@
<optional>true</optional>
</dependency>

<dependency>
<groupId>io.smallrye.common</groupId>
<artifactId>smallrye-common-vertx-context</artifactId>
</dependency>
<dependency>
<groupId>io.opentelemetry</groupId>
<artifactId>opentelemetry-sdk</artifactId>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,66 +1,113 @@
package io.quarkus.opentelemetry.runtime;

import static io.smallrye.common.vertx.VertxContext.getOrCreateDuplicatedContext;
import static io.smallrye.common.vertx.VertxContext.isDuplicatedContext;

import org.jboss.logging.Logger;

import io.opentelemetry.context.Context;
import io.opentelemetry.context.ContextStorage;
import io.opentelemetry.context.Scope;
import io.vertx.core.Vertx;
import io.vertx.core.impl.ContextInternal;

/**
* Bridges the OpenTelemetry ContextStorage with the Vert.x Context. The default OpenTelemetry ContextStorage (based in
* ThreadLocals) is not suitable for Vert.x. In this case, the OpenTelemetry Context piggybacks on top of the Vert.x
* Context. If the Vert.x Context is not available, fallbacks to the default OpenTelemetry ContextStorage.
*/
public enum QuarkusContextStorage implements ContextStorage {
INSTANCE;

private static final Logger log = Logger.getLogger(QuarkusContextStorage.class);
private static final String OTEL_CONTEXT = QuarkusContextStorage.class.getName() + ".otelContext";

public static final String ACTIVE_CONTEXT = QuarkusContextStorage.class.getName() + ".activeContext";

private static final ContextStorage DEFAULT_CONTEXT_STORAGE = ContextStorage.defaultStorage();
static Vertx vertx;

/**
* Attach the OpenTelemetry Context to the current Context. If a Vert.x Context is available, and it is a duplicated
* Vert.x Context the OpenTelemetry Context is attached to the Vert.x Context. Otherwise, fallback to the
* OpenTelemetry default ContextStorage.
*
* @param toAttach the OpenTelemetry Context to attach
* @return the Scope of the OpenTelemetry Context
*/
@Override
public Scope attach(Context toAttach) {
return attach(getVertxContext(), toAttach);
io.vertx.core.Context vertxContext = getVertxContext();
return vertxContext != null && isDuplicatedContext(vertxContext) ? attach(vertxContext, toAttach)
: DEFAULT_CONTEXT_STORAGE.attach(toAttach);
}

/**
* Attach the OpenTelemetry Context in the Vert.x Context if it is a duplicated Vert.x Context.
*
* @param vertxContext the Vert.x Context to attach the OpenTelemetry Context
* @param toAttach the OpenTelemetry Context to attach
* @return the Scope of the OpenTelemetry Context
*/
public Scope attach(io.vertx.core.Context vertxContext, Context toAttach) {
if (toAttach == null) {
// Not allowed
if (vertxContext == null || toAttach == null) {
return Scope.noop();
}

// We don't allow to attach the OpenTelemetry Context to a Vert.x Context that is not a duplicate.
if (!isDuplicatedContext(vertxContext)) {
throw new IllegalArgumentException(
"The Vert.x Context to attach the OpenTelemetry Context must be a duplicated Context");
}

Context beforeAttach = getContext(vertxContext);
if (toAttach == beforeAttach) {
return Scope.noop();
}

if (vertxContext != null) {
vertxContext.putLocal(ACTIVE_CONTEXT, toAttach);
return () -> {
if (getContext(vertxContext) != toAttach) {
log.warn("Context in storage not the expected context, Scope.close was not called correctly");
}
if (beforeAttach == null) {
vertxContext.removeLocal(ACTIVE_CONTEXT);
((ContextInternal) vertxContext).unwrap().removeLocal(ACTIVE_CONTEXT);
} else {
vertxContext.putLocal(ACTIVE_CONTEXT, beforeAttach);
}
};
}
vertxContext.putLocal(OTEL_CONTEXT, toAttach);

return () -> {
if (getContext(vertxContext) != toAttach) {
log.warn("Context in storage not the expected context, Scope.close was not called correctly");
}

return Scope.noop();
if (beforeAttach == null) {
vertxContext.removeLocal(OTEL_CONTEXT);
} else {
vertxContext.putLocal(OTEL_CONTEXT, beforeAttach);
}
};
}

/**
* Gets the current OpenTelemetry Context from the current Vert.x Context if one exists or from the default
* ContextStorage. The current Vert.x Context must be a duplicated Context.
*
* @return the current OpenTelemetry Context or null.
*/
@Override
public Context current() {
return getContext(getVertxContext());
return Vertx.currentContext() != null ? getOrCreateDuplicatedContext(vertx).getLocal(OTEL_CONTEXT)
: DEFAULT_CONTEXT_STORAGE.current();
}

private Context getContext(io.vertx.core.Context vertxContext) {
return vertxContext != null ? vertxContext.getLocal(ACTIVE_CONTEXT) : null;
/**
* Gets the OpenTelemetry Context in a Vert.x Context. The Vert.x Context has to be a duplicate context.
*
* @param vertxContext a Vert.x Context.
* @return the OpenTelemetry Context if exists in the Vert.x Context or null.
*/
public static Context getContext(io.vertx.core.Context vertxContext) {
return vertxContext != null && isDuplicatedContext(vertxContext) ? vertxContext.getLocal(OTEL_CONTEXT) : null;
}

private io.vertx.core.Context getVertxContext() {
return vertx.getOrCreateContext();
/**
* Gets the current duplicated context or a new duplicated context if a Vert.x Context exists. Multiple invocations
* of this method may return the same or different context. If the current context is a duplicate one, multiple
* invocations always return the same context. If the current context is not duplicated, a new instance is returned
* with each method invocation.
*
* @return a duplicated Vert.x Context or null.
*/
private static io.vertx.core.Context getVertxContext() {
return Vertx.currentContext() != null ? getOrCreateDuplicatedContext(vertx) : null;
}
}
Loading

0 comments on commit 3493e06

Please sign in to comment.