Skip to content

Commit d8a92f8

Browse files
jandro996jandro996
committed
spotless
1 parent b1ec26b commit d8a92f8

File tree

5 files changed

+128
-5
lines changed

5 files changed

+128
-5
lines changed

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

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -368,6 +368,10 @@ abstract class HttpServerTest<SERVER> extends WithHttpServer<SERVER> {
368368
false
369369
}
370370

371+
boolean testBodyFilenamesCalledOnce() {
372+
false
373+
}
374+
371375
boolean testBodyFilenames() {
372376
false
373377
}
@@ -476,6 +480,7 @@ abstract class HttpServerTest<SERVER> extends WithHttpServer<SERVER> {
476480
CREATED_IS("created_input_stream", 201, "created"),
477481
BODY_URLENCODED("body-urlencoded?ignore=pair", 200, '[a:[x]]'),
478482
BODY_MULTIPART("body-multipart?ignore=pair", 200, '[a:[x]]'),
483+
BODY_MULTIPART_REPEATED("body-multipart-repeated", 200, "ok"),
479484
BODY_JSON("body-json", 200, '{"a":"x"}'),
480485
BODY_XML("body-xml", 200, '<foo attr="attr_value">mytext<bar/></foo>'),
481486
REDIRECT("redirect", 302, "/redirected"),
@@ -1646,6 +1651,30 @@ abstract class HttpServerTest<SERVER> extends WithHttpServer<SERVER> {
16461651
response.close()
16471652
}
16481653

1654+
def 'test instrumentation gateway file upload filenames called once'() {
1655+
setup:
1656+
assumeTrue(testBodyFilenamesCalledOnce())
1657+
RequestBody fileBody = RequestBody.create(MediaType.parse('application/octet-stream'), 'file content')
1658+
def body = new MultipartBody.Builder()
1659+
.setType(MultipartBody.FORM)
1660+
.addFormDataPart('file', 'evil.php', fileBody)
1661+
.build()
1662+
def httpRequest = request(BODY_MULTIPART_REPEATED, 'POST', body).build()
1663+
def response = client.newCall(httpRequest).execute()
1664+
1665+
when:
1666+
TEST_WRITER.waitForTraces(1)
1667+
1668+
then:
1669+
TEST_WRITER.get(0).any {
1670+
it.getTag('request.body.filenames') == "[evil.php]"
1671+
&& it.getTag('_dd.appsec.filenames.cb.calls') == 1
1672+
}
1673+
1674+
cleanup:
1675+
response.close()
1676+
}
1677+
16491678
def 'test instrumentation gateway json request body'() {
16501679
setup:
16511680
assumeTrue(testBodyJson())
@@ -2581,6 +2610,7 @@ abstract class HttpServerTest<SERVER> extends WithHttpServer<SERVER> {
25812610
boolean responseBodyTag
25822611
Object responseBody
25832612
List<String> uploadedFilenames
2613+
int uploadedFilenamesCallCount = 0
25842614
}
25852615

25862616
static final String stringOrEmpty(String string) {
@@ -2754,6 +2784,8 @@ abstract class HttpServerTest<SERVER> extends WithHttpServer<SERVER> {
27542784
rqCtxt.traceSegment.setTagTop('request.body.filenames', filenames as String)
27552785
Context context = rqCtxt.getData(RequestContextSlot.APPSEC)
27562786
context.uploadedFilenames = filenames
2787+
context.uploadedFilenamesCallCount++
2788+
rqCtxt.traceSegment.setTagTop('_dd.appsec.filenames.cb.calls', context.uploadedFilenamesCallCount)
27572789
Flow.ResultFlow.empty()
27582790
} as BiFunction<RequestContext, List<String>, Flow<Void>>)
27592791

‎dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.2/src/main/java/datadog/trace/instrumentation/jetty92/RequestExtractContentParametersInstrumentation.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -144,8 +144,7 @@ static void after(
144144
@RequiresRequestContext(RequestContextSlot.APPSEC)
145145
public static class GetFilenamesAdvice {
146146
@Advice.OnMethodEnter(suppress = Throwable.class)
147-
static boolean before(
148-
@Advice.FieldValue("_contentParameters") final MultiMap<String> map) {
147+
static boolean before(@Advice.FieldValue("_contentParameters") final MultiMap<String> map) {
149148
final int callDepth = CallDepthThreadLocalMap.incrementCallDepth(Collection.class);
150149
return callDepth == 0 && map == null;
151150
}

‎dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-9.3/src/main/java/datadog/trace/instrumentation/jetty93/RequestExtractContentParametersInstrumentation.java‎

Lines changed: 81 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,12 @@ public void methodAdvice(MethodTransformer transformer) {
4646
transformer.applyAdvice(
4747
named("extractContentParameters").and(takesArguments(0)).or(named("getParts")),
4848
getClass().getName() + "$ExtractContentParametersAdvice");
49-
transformer.applyAdvice(named("getParts"), getClass().getName() + "$GetFilenamesAdvice");
49+
transformer.applyAdvice(
50+
named("getParts").and(takesArguments(0)),
51+
getClass().getName() + "$GetFilenamesAdvice");
52+
transformer.applyAdvice(
53+
named("getParts").and(takesArguments(1)),
54+
getClass().getName() + "$GetFilenamesFromMultiPartAdvice");
5055
}
5156

5257
private static final Reference REQUEST_REFERENCE =
@@ -105,11 +110,15 @@ static void after(
105110
}
106111
}
107112

113+
/**
114+
* Fires the {@code requestFilesFilenames} event when the application calls public {@code
115+
* getParts()}. The {@code _contentParameters == null} guard ensures the WAF is invoked only on
116+
* the first call — subsequent calls return the cached result without re-processing.
117+
*/
108118
@RequiresRequestContext(RequestContextSlot.APPSEC)
109119
public static class GetFilenamesAdvice {
110120
@Advice.OnMethodEnter(suppress = Throwable.class)
111-
static boolean before(
112-
@Advice.FieldValue("_contentParameters") final MultiMap<String> map) {
121+
static boolean before(@Advice.FieldValue("_contentParameters") final MultiMap<String> map) {
113122
final int callDepth = CallDepthThreadLocalMap.incrementCallDepth(Collection.class);
114123
return callDepth == 0 && map == null;
115124
}
@@ -166,4 +175,73 @@ static void after(
166175
}
167176
}
168177
}
178+
179+
/**
180+
* Fires the {@code requestFilesFilenames} event when multipart content is parsed via the
181+
* internal {@code getParts(MultiMap)} path triggered by {@code getParameter*()} /
182+
* {@code getParameterMap()} — i.e. when the application never calls public {@code getParts()}.
183+
* In Jetty 9.3+, {@code extractContentParameters()} assigns {@code _contentParameters} before
184+
* calling this method, so {@code map == null} cannot be used as a "first parse" guard here;
185+
* the call-depth guard prevents double-firing when {@code getParts()} internally delegates to
186+
* this method.
187+
*/
188+
@RequiresRequestContext(RequestContextSlot.APPSEC)
189+
public static class GetFilenamesFromMultiPartAdvice {
190+
@Advice.OnMethodEnter(suppress = Throwable.class)
191+
static boolean before() {
192+
return CallDepthThreadLocalMap.incrementCallDepth(Collection.class) == 0;
193+
}
194+
195+
@Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class)
196+
static void after(
197+
@Advice.Enter boolean proceed,
198+
@Advice.Return Collection parts,
199+
@ActiveRequestContext RequestContext reqCtx,
200+
@Advice.Thrown(readOnly = false) Throwable t) {
201+
CallDepthThreadLocalMap.decrementCallDepth(Collection.class);
202+
if (!proceed || t != null || parts == null || parts.isEmpty()) {
203+
return;
204+
}
205+
Method getSubmittedFileName = null;
206+
try {
207+
getSubmittedFileName = parts.iterator().next().getClass().getMethod("getSubmittedFileName");
208+
} catch (Exception ignored) {
209+
}
210+
if (getSubmittedFileName == null) {
211+
return;
212+
}
213+
List<String> filenames = new ArrayList<>();
214+
for (Object part : parts) {
215+
try {
216+
String name = (String) getSubmittedFileName.invoke(part);
217+
if (name != null && !name.isEmpty()) {
218+
filenames.add(name);
219+
}
220+
} catch (Exception ignored) {
221+
}
222+
}
223+
if (filenames.isEmpty()) {
224+
return;
225+
}
226+
CallbackProvider cbp = AgentTracer.get().getCallbackProvider(RequestContextSlot.APPSEC);
227+
BiFunction<RequestContext, List<String>, Flow<Void>> callback =
228+
cbp.getCallback(EVENTS.requestFilesFilenames());
229+
if (callback == null) {
230+
return;
231+
}
232+
Flow<Void> flow = callback.apply(reqCtx, filenames);
233+
Flow.Action action = flow.getAction();
234+
if (action instanceof Flow.Action.RequestBlockingAction) {
235+
Flow.Action.RequestBlockingAction rba = (Flow.Action.RequestBlockingAction) action;
236+
BlockResponseFunction brf = reqCtx.getBlockResponseFunction();
237+
if (brf != null) {
238+
brf.tryCommitBlockingResponse(reqCtx.getTraceSegment(), rba);
239+
if (t == null) {
240+
t = new BlockingException("Blocked request (multipart file upload)");
241+
reqCtx.getTraceSegment().effectivelyBlocked();
242+
}
243+
}
244+
}
245+
}
246+
}
169247
}

‎dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-11.0/src/test/groovy/Jetty11Test.groovy‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,11 @@ abstract class Jetty11Test extends HttpServerTest<Server> {
7272
true
7373
}
7474

75+
@Override
76+
boolean testBodyFilenamesCalledOnce() {
77+
true
78+
}
79+
7580
@Override
7681
boolean testBlocking() {
7782
true

‎dd-java-agent/instrumentation/servlet/jakarta-servlet-5.0/src/testFixtures/groovy/datadog/trace/instrumentation/servlet5/TestServlet5.groovy‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import java.lang.reflect.Field
1111
import java.lang.reflect.Modifier
1212

1313
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_MULTIPART
14+
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_MULTIPART_REPEATED
1415
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_URLENCODED
1516
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.CREATED
1617
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.CREATED_IS
@@ -68,6 +69,14 @@ class TestServlet5 extends HttpServlet {
6869
resp.status = endpoint.status
6970
resp.writer.print(req.getHeader("x-forwarded-for"))
7071
break
72+
case BODY_MULTIPART_REPEATED:
73+
resp.status = endpoint.status
74+
// Call getParts() 3 times to verify the filenames callback fires only once
75+
req.getParts()
76+
req.getParts()
77+
req.getParts()
78+
resp.writer.print(endpoint.body)
79+
break
7180
case BODY_MULTIPART:
7281
case BODY_URLENCODED:
7382
resp.status = endpoint.status

0 commit comments

Comments
 (0)