Skip to content

Commit 428b50e

Browse files
jandro996devflow.devflow-routing-intake
andauthored
Add server.request.body.files_content AppSec address for Vert.x 3/4/5 (#11724)
feat(appsec): fire server.request.body.files_content for Vert.x 3/4/5 Extends RoutingContextFilenamesAdvice in vertx-web-3.4, vertx-web-4.0, and vertx-web-5.0 to also collect and fire the requestFilesContent() IG callback alongside the existing requestFilesFilenames() callback. Content is read from disk using FileInputStream and MultipartContentDecoder since Vert.x BodyHandler always persists uploads to disk. The filenames callback fires first; the content callback only fires if filenames did not trigger a block (sequential guard). Muzzle references are updated to include uploadedFileName() and contentType() on FileUpload. Tests enable testBodyFilesContent() for all three Vert.x versions. fix(appsec): use Vert.x FileUpload.charSet() when decoding file content FileUpload.contentType() in Vert.x returns only the MIME type without the charset parameter. charSet() exposes it separately. Combine both into a full content-type string before passing to MultipartContentDecoder.readInputStream so extractCharset finds the declared charset instead of falling back to the JVM default. Add charSet() to FILE_UPLOAD_REF muzzle references in all three Vert.x modules. fix(appsec): extract FileUploadHelper to fix muzzle validation for Vert.x 3/4/5 Private static helpers in @advice classes generate INVOKESTATIC references to the advice class itself. Muzzle resolves the advice class by name and validates all its INVOKESTATIC targets against the library classpath, which does not contain our advice classes, causing "Missing class" failures. Extract readUploadContent and commitBlockingResponse into FileUploadHelper per module and declare each in helperClassNames() so ByteBuddy injects them into the target classloader and muzzle validates their references normally. fix: make FileUploadHelper public and fix max files limit test for Vert.x 3/4/5 Helper classes injected via helperClassNames() are loaded into the app classloader. When package-private, they cause IllegalAccessError from instrumented classes in different packages within the same unnamed module. The max files limit test also needed an override because Vert.x 3.4 returns fileUploads() as a HashSet, whose iteration order depends on identity hash codes. Adding helperClassNames() shifts object allocation counts and therefore hash codes, changing which file is excluded. The override checks the count of inspected files instead of which specific file was excluded. style: replace em-dashes with colons in test comments fix: skip ordering-dependent max files limit test for Vert.x HashSet fileUploads() returns a HashSet in Vert.x so iteration order is JVM- version-dependent. Add testBodyFilesContentOrdering() flag (defaults true) to HttpServerTest to allow skipping the specific-file assertion. Vert.x subclasses return false and add a count-based test instead. fix: skip files_content test in VertxRxCircuitBreakerHttpServerForkedTest The circuit breaker test server does not configure BodyHandler for the multipart endpoint, so fileUploads() is never populated and the files_content instrumentation does not fire. Consistent with the existing testBodyFilenames() override in the same class. Co-authored-by: devflow.devflow-routing-intake <devflow.devflow-routing-intake@kubernetes.us1.ddbuild.io>
1 parent b848caf commit 428b50e

15 files changed

Lines changed: 467 additions & 66 deletions

File tree

dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -387,6 +387,11 @@ abstract class HttpServerTest<SERVER> extends WithHttpServer<SERVER> {
387387
false
388388
}
389389

390+
/** Override to false when the multipart implementation uses a set with non-deterministic ordering (e.g. HashSet). */
391+
boolean testBodyFilesContentOrdering() {
392+
true
393+
}
394+
390395
boolean testBodyJson() {
391396
false
392397
}
@@ -1767,7 +1772,7 @@ abstract class HttpServerTest<SERVER> extends WithHttpServer<SERVER> {
17671772

17681773
def 'test instrumentation gateway file upload content max files limit'() {
17691774
setup:
1770-
assumeTrue(testBodyFilesContent())
1775+
assumeTrue(testBodyFilesContent() && testBodyFilesContentOrdering())
17711776
def maxFilesToInspect = Config.get().getAppSecMaxFileContentCount()
17721777
def bodyBuilder = new MultipartBody.Builder().setType(MultipartBody.FORM)
17731778
(1..maxFilesToInspect + 1).each {

dd-java-agent/instrumentation/vertx/vertx-rx-3.5/src/test/groovy/server/VertxRxCircuitBreakerHttpServerForkedTest.groovy

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,11 @@ class VertxRxCircuitBreakerHttpServerForkedTest extends VertxHttpServerForkedTes
4444
false
4545
}
4646

47+
@Override
48+
boolean testBodyFilesContent() {
49+
false
50+
}
51+
4752
@Override
4853
boolean testBodyJson() {
4954
false
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
package datadog.trace.instrumentation.vertx_3_4.server;
2+
3+
import datadog.appsec.api.blocking.BlockingException;
4+
import datadog.trace.api.gateway.BlockResponseFunction;
5+
import datadog.trace.api.gateway.Flow;
6+
import datadog.trace.api.gateway.RequestContext;
7+
import datadog.trace.api.http.MultipartContentDecoder;
8+
import io.vertx.ext.web.FileUpload;
9+
import java.io.FileInputStream;
10+
import java.util.List;
11+
import java.util.function.BiFunction;
12+
13+
public class FileUploadHelper {
14+
15+
public static BlockingException commitBlockingResponse(
16+
BiFunction<RequestContext, List<String>, Flow<Void>> cb,
17+
RequestContext reqCtx,
18+
List<String> data,
19+
String reason) {
20+
Flow<Void> flow = cb.apply(reqCtx, data);
21+
Flow.Action action = flow.getAction();
22+
if (action instanceof Flow.Action.RequestBlockingAction) {
23+
BlockResponseFunction brf = reqCtx.getBlockResponseFunction();
24+
if (brf != null) {
25+
brf.tryCommitBlockingResponse(
26+
reqCtx.getTraceSegment(), (Flow.Action.RequestBlockingAction) action);
27+
return new BlockingException(reason);
28+
}
29+
}
30+
return null;
31+
}
32+
33+
public static String readUploadContent(FileUpload upload, int maxBytes) {
34+
try {
35+
String path = upload.uploadedFileName();
36+
if (path == null || path.isEmpty()) {
37+
return "";
38+
}
39+
String charSet = upload.charSet();
40+
String contentType =
41+
charSet != null && !charSet.isEmpty()
42+
? upload.contentType() + "; charset=" + charSet
43+
: upload.contentType();
44+
try (FileInputStream fis = new FileInputStream(path)) {
45+
return MultipartContentDecoder.readInputStream(fis, maxBytes, contentType);
46+
}
47+
} catch (Exception ignored) {
48+
return "";
49+
}
50+
}
51+
}

dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/RoutingContextFilenamesAdvice.java

Lines changed: 37 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,9 @@
22

33
import static datadog.trace.api.gateway.Events.EVENTS;
44

5-
import datadog.appsec.api.blocking.BlockingException;
65
import datadog.trace.advice.ActiveRequestContext;
76
import datadog.trace.advice.RequiresRequestContext;
8-
import datadog.trace.api.gateway.BlockResponseFunction;
7+
import datadog.trace.api.Config;
98
import datadog.trace.api.gateway.CallbackProvider;
109
import datadog.trace.api.gateway.Flow;
1110
import datadog.trace.api.gateway.RequestContext;
@@ -38,35 +37,52 @@ static void after(
3837
return;
3938
}
4039

41-
List<String> filenames = new ArrayList<>(uploads.size());
40+
CallbackProvider cbp = AgentTracer.get().getCallbackProvider(RequestContextSlot.APPSEC);
41+
BiFunction<RequestContext, List<String>, Flow<Void>> filenamesCb =
42+
cbp.getCallback(EVENTS.requestFilesFilenames());
43+
BiFunction<RequestContext, List<String>, Flow<Void>> contentCb =
44+
cbp.getCallback(EVENTS.requestFilesContent());
45+
if (filenamesCb == null && contentCb == null) {
46+
return;
47+
}
48+
49+
int maxFiles = Config.get().getAppSecMaxFileContentCount();
50+
int maxBytes = Config.get().getAppSecMaxFileContentBytes();
51+
List<String> filenames = null;
52+
List<String> filesContent = null;
53+
4254
for (FileUpload upload : uploads) {
4355
String name = upload.fileName();
44-
if (name != null && !name.isEmpty()) {
56+
if (filenamesCb != null && name != null && !name.isEmpty()) {
57+
if (filenames == null) {
58+
filenames = new ArrayList<>();
59+
}
4560
filenames.add(name);
4661
}
62+
if (contentCb != null
63+
&& maxFiles > 0
64+
&& (filesContent == null || filesContent.size() < maxFiles)) {
65+
if (filesContent == null) {
66+
filesContent = new ArrayList<>();
67+
}
68+
filesContent.add(FileUploadHelper.readUploadContent(upload, maxBytes));
69+
}
4770
}
48-
if (filenames.isEmpty()) {
49-
return;
71+
72+
if (filenamesCb != null && filenames != null) {
73+
throwable =
74+
FileUploadHelper.commitBlockingResponse(
75+
filenamesCb, reqCtx, filenames, "Blocked request (multipart file upload)");
5076
}
5177

52-
CallbackProvider cbp = AgentTracer.get().getCallbackProvider(RequestContextSlot.APPSEC);
53-
BiFunction<RequestContext, List<String>, Flow<Void>> cb =
54-
cbp.getCallback(EVENTS.requestFilesFilenames());
55-
if (cb == null) {
78+
if (throwable != null) {
5679
return;
5780
}
5881

59-
Flow<Void> flow = cb.apply(reqCtx, filenames);
60-
Flow.Action action = flow.getAction();
61-
if (action instanceof Flow.Action.RequestBlockingAction) {
62-
BlockResponseFunction brf = reqCtx.getBlockResponseFunction();
63-
if (brf != null) {
64-
brf.tryCommitBlockingResponse(
65-
reqCtx.getTraceSegment(), (Flow.Action.RequestBlockingAction) action);
66-
if (throwable == null) {
67-
throwable = new BlockingException("Blocked request (multipart file upload)");
68-
}
69-
}
82+
if (contentCb != null && filesContent != null) {
83+
throwable =
84+
FileUploadHelper.commitBlockingResponse(
85+
contentCb, reqCtx, filesContent, "Blocked request (file content)");
7086
}
7187
}
7288
}

dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/RoutingContextImplInstrumentation.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,20 @@ public class RoutingContextImplInstrumentation extends InstrumenterModule.AppSec
1818
private static final Reference FILE_UPLOAD_REF =
1919
new Reference.Builder("io.vertx.ext.web.FileUpload")
2020
.withMethod(new String[0], 0, "fileName", "Ljava/lang/String;")
21+
.withMethod(new String[0], 0, "uploadedFileName", "Ljava/lang/String;")
22+
.withMethod(new String[0], 0, "contentType", "Ljava/lang/String;")
23+
.withMethod(new String[0], 0, "charSet", "Ljava/lang/String;")
2124
.build();
2225

2326
public RoutingContextImplInstrumentation() {
2427
super("vertx", "vertx-3.4");
2528
}
2629

30+
@Override
31+
public String[] helperClassNames() {
32+
return new String[] {packageName + ".FileUploadHelper"};
33+
}
34+
2735
@Override
2836
public String instrumentedType() {
2937
return "io.vertx.ext.web.impl.RoutingContextImpl";

dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-3.4/src/test/groovy/server/VertxHttpServerForkedTest.groovy

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package server
22

3+
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_MULTIPART
34
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR
5+
import static org.junit.jupiter.api.Assumptions.assumeTrue
46
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION
57
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.LOGIN
68
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.NOT_FOUND
@@ -10,6 +12,10 @@ import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCES
1012
import datadog.trace.agent.test.asserts.TraceAssert
1113
import datadog.trace.agent.test.base.HttpServer
1214
import datadog.trace.agent.test.base.HttpServerTest
15+
import datadog.trace.api.Config
16+
import okhttp3.MediaType
17+
import okhttp3.MultipartBody
18+
import okhttp3.RequestBody
1319
import datadog.trace.api.DDSpanTypes
1420
import datadog.trace.bootstrap.instrumentation.api.Tags
1521
import datadog.trace.instrumentation.netty41.server.NettyHttpServerDecorator
@@ -87,6 +93,42 @@ class VertxHttpServerForkedTest extends HttpServerTest<Vertx> {
8793
true
8894
}
8995

96+
@Override
97+
boolean testBodyFilesContent() {
98+
true
99+
}
100+
101+
@Override
102+
boolean testBodyFilesContentOrdering() {
103+
false
104+
}
105+
106+
// fileUploads() returns a HashSet in Vert.x: check count instead of which specific file is excluded
107+
def 'test instrumentation gateway file upload content max files limit count'() {
108+
setup:
109+
assumeTrue(testBodyFilesContent())
110+
def maxFilesToInspect = Config.get().getAppSecMaxFileContentCount()
111+
def bodyBuilder = new MultipartBody.Builder().setType(MultipartBody.FORM)
112+
(1..maxFilesToInspect + 1).each { i ->
113+
bodyBuilder.addFormDataPart("file${i}", "file${i}.bin",
114+
RequestBody.create(MediaType.parse('application/octet-stream'), "content_of_file_${i}"))
115+
}
116+
def httpRequest = request(BODY_MULTIPART, 'POST', bodyBuilder.build()).build()
117+
def response = client.newCall(httpRequest).execute()
118+
119+
when:
120+
TEST_WRITER.waitForTraces(1)
121+
122+
then:
123+
TEST_WRITER.get(0).any { span ->
124+
def tag = span.getTag('request.body.files_content') as String
125+
tag != null && tag.count('content_of_file_') == maxFilesToInspect
126+
}
127+
128+
cleanup:
129+
response.close()
130+
}
131+
90132
@Override
91133
boolean testBodyJson() {
92134
true

dd-java-agent/instrumentation/vertx/vertx-web/vertx-web-4.0/src/latestDepTest/groovy/server/VertxHttpServerForkedTest.groovy

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package server
22

3+
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_MULTIPART
34
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR
5+
import static org.junit.jupiter.api.Assumptions.assumeTrue
46
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION
57
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.LOGIN
68
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.NOT_FOUND
@@ -10,6 +12,10 @@ import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCES
1012
import datadog.trace.agent.test.asserts.TraceAssert
1113
import datadog.trace.agent.test.base.HttpServer
1214
import datadog.trace.agent.test.base.HttpServerTest
15+
import datadog.trace.api.Config
16+
import okhttp3.MediaType
17+
import okhttp3.MultipartBody
18+
import okhttp3.RequestBody
1319
import datadog.trace.api.DDSpanTypes
1420
import datadog.trace.bootstrap.instrumentation.api.Tags
1521
import datadog.trace.instrumentation.netty41.server.NettyHttpServerDecorator
@@ -82,6 +88,42 @@ class VertxHttpServerForkedTest extends HttpServerTest<Vertx> {
8288
true
8389
}
8490

91+
@Override
92+
boolean testBodyFilesContent() {
93+
true
94+
}
95+
96+
@Override
97+
boolean testBodyFilesContentOrdering() {
98+
false
99+
}
100+
101+
// fileUploads() returns a HashSet in Vert.x: check count instead of which specific file is excluded
102+
def 'test instrumentation gateway file upload content max files limit count'() {
103+
setup:
104+
assumeTrue(testBodyFilesContent())
105+
def maxFilesToInspect = Config.get().getAppSecMaxFileContentCount()
106+
def bodyBuilder = new MultipartBody.Builder().setType(MultipartBody.FORM)
107+
(1..maxFilesToInspect + 1).each { i ->
108+
bodyBuilder.addFormDataPart("file${i}", "file${i}.bin",
109+
RequestBody.create(MediaType.parse('application/octet-stream'), "content_of_file_${i}"))
110+
}
111+
def httpRequest = request(BODY_MULTIPART, 'POST', bodyBuilder.build()).build()
112+
def response = client.newCall(httpRequest).execute()
113+
114+
when:
115+
TEST_WRITER.waitForTraces(1)
116+
117+
then:
118+
TEST_WRITER.get(0).any { span ->
119+
def tag = span.getTag('request.body.files_content') as String
120+
tag != null && tag.count('content_of_file_') == maxFilesToInspect
121+
}
122+
123+
cleanup:
124+
response.close()
125+
}
126+
85127
@Override
86128
boolean testBodyJson() {
87129
true
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
package datadog.trace.instrumentation.vertx_4_0.server;
2+
3+
import datadog.appsec.api.blocking.BlockingException;
4+
import datadog.trace.api.gateway.BlockResponseFunction;
5+
import datadog.trace.api.gateway.Flow;
6+
import datadog.trace.api.gateway.RequestContext;
7+
import datadog.trace.api.http.MultipartContentDecoder;
8+
import io.vertx.ext.web.FileUpload;
9+
import java.io.FileInputStream;
10+
import java.util.List;
11+
import java.util.function.BiFunction;
12+
13+
public class FileUploadHelper {
14+
15+
public static BlockingException commitBlockingResponse(
16+
BiFunction<RequestContext, List<String>, Flow<Void>> cb,
17+
RequestContext reqCtx,
18+
List<String> data,
19+
String reason) {
20+
Flow<Void> flow = cb.apply(reqCtx, data);
21+
Flow.Action action = flow.getAction();
22+
if (action instanceof Flow.Action.RequestBlockingAction) {
23+
BlockResponseFunction brf = reqCtx.getBlockResponseFunction();
24+
if (brf != null) {
25+
brf.tryCommitBlockingResponse(
26+
reqCtx.getTraceSegment(), (Flow.Action.RequestBlockingAction) action);
27+
return new BlockingException(reason);
28+
}
29+
}
30+
return null;
31+
}
32+
33+
public static String readUploadContent(FileUpload upload, int maxBytes) {
34+
try {
35+
String path = upload.uploadedFileName();
36+
if (path == null || path.isEmpty()) {
37+
return "";
38+
}
39+
String charSet = upload.charSet();
40+
String contentType =
41+
charSet != null && !charSet.isEmpty()
42+
? upload.contentType() + "; charset=" + charSet
43+
: upload.contentType();
44+
try (FileInputStream fis = new FileInputStream(path)) {
45+
return MultipartContentDecoder.readInputStream(fis, maxBytes, contentType);
46+
}
47+
} catch (Exception ignored) {
48+
return "";
49+
}
50+
}
51+
}

0 commit comments

Comments
 (0)