Skip to content

Commit abddcfa

Browse files
committed
Add BODY_MULTIPART_COMBINED test to cover GetFilenamesFromMultiPartAdvice path
- New BODY_MULTIPART_COMBINED endpoint: calls getParameterMap() first (triggers GetFilenamesFromMultiPartAdvice via extractContentParameters -> getParts(MultiMap)), then getParts() explicitly (GetFilenamesAdvice must not double-fire since _contentParameters is already set) - New test 'file upload filenames called once via parameter map' verifies the callback fires exactly once across both advice paths - Enabled in Jetty 9.0, 9.0.4, 9.3, 9.4.21, 10.0 and 11.0
1 parent 30bb769 commit abddcfa

9 files changed

Lines changed: 78 additions & 1 deletion

File tree

  • dd-java-agent
    • instrumentation-testing/src/main/groovy/datadog/trace/agent/test/base
    • instrumentation
      • jetty/jetty-server
      • servlet
        • jakarta-servlet-5.0/src/testFixtures/groovy/datadog/trace/instrumentation/servlet5
        • javax-servlet/javax-servlet-3.0/src/testFixtures/groovy/datadog/trace/instrumentation/servlet3

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

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -372,6 +372,10 @@ abstract class HttpServerTest<SERVER> extends WithHttpServer<SERVER> {
372372
false
373373
}
374374

375+
boolean testBodyFilenamesCalledOnceCombined() {
376+
false
377+
}
378+
375379
boolean testBodyFilenames() {
376380
false
377381
}
@@ -481,6 +485,7 @@ abstract class HttpServerTest<SERVER> extends WithHttpServer<SERVER> {
481485
BODY_URLENCODED("body-urlencoded?ignore=pair", 200, '[a:[x]]'),
482486
BODY_MULTIPART("body-multipart?ignore=pair", 200, '[a:[x]]'),
483487
BODY_MULTIPART_REPEATED("body-multipart-repeated", 200, "ok"),
488+
BODY_MULTIPART_COMBINED("body-multipart-combined", 200, "ok"),
484489
BODY_JSON("body-json", 200, '{"a":"x"}'),
485490
BODY_XML("body-xml", 200, '<foo attr="attr_value">mytext<bar/></foo>'),
486491
REDIRECT("redirect", 302, "/redirected"),
@@ -1675,6 +1680,30 @@ abstract class HttpServerTest<SERVER> extends WithHttpServer<SERVER> {
16751680
response.close()
16761681
}
16771682

1683+
def 'test instrumentation gateway file upload filenames called once via parameter map'() {
1684+
setup:
1685+
assumeTrue(testBodyFilenamesCalledOnceCombined())
1686+
RequestBody fileBody = RequestBody.create(MediaType.parse('application/octet-stream'), 'file content')
1687+
def body = new MultipartBody.Builder()
1688+
.setType(MultipartBody.FORM)
1689+
.addFormDataPart('file', 'evil.php', fileBody)
1690+
.build()
1691+
def httpRequest = request(BODY_MULTIPART_COMBINED, 'POST', body).build()
1692+
def response = client.newCall(httpRequest).execute()
1693+
1694+
when:
1695+
TEST_WRITER.waitForTraces(1)
1696+
1697+
then:
1698+
TEST_WRITER.get(0).any {
1699+
it.getTag('request.body.filenames') == "[evil.php]"
1700+
&& it.getTag('_dd.appsec.filenames.cb.calls') == 1
1701+
}
1702+
1703+
cleanup:
1704+
response.close()
1705+
}
1706+
16781707
def 'test instrumentation gateway json request body'() {
16791708
setup:
16801709
assumeTrue(testBodyJson())

dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-10.0/src/test/groovy/datadog/trace/instrumentation/jetty10/Jetty10Test.groovy

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,11 @@ abstract class Jetty10Test extends HttpServerTest<Server> {
9595
true
9696
}
9797

98+
@Override
99+
boolean testBodyFilenamesCalledOnceCombined() {
100+
true
101+
}
102+
98103
@Override
99104
boolean testSessionId() {
100105
true

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
@@ -77,6 +77,11 @@ abstract class Jetty11Test extends HttpServerTest<Server> {
7777
true
7878
}
7979

80+
@Override
81+
boolean testBodyFilenamesCalledOnceCombined() {
82+
true
83+
}
84+
8085
@Override
8186
boolean testBlocking() {
8287
true

dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.0.4/src/test/groovy/datadog/trace/instrumentation/jetty9/Jetty9Test.groovy

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,11 @@ abstract class Jetty9Test extends HttpServerTest<Server> {
9595
true
9696
}
9797

98+
@Override
99+
boolean testBodyFilenamesCalledOnceCombined() {
100+
true
101+
}
102+
98103
@Override
99104
boolean testSessionId() {
100105
true

dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.0/src/test/groovy/datadog/trace/instrumentation/jetty9/Jetty9Test.groovy

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,11 @@ abstract class Jetty9Test extends HttpServerTest<Server> {
9494
true
9595
}
9696

97+
@Override
98+
boolean testBodyFilenamesCalledOnceCombined() {
99+
true
100+
}
101+
97102
@Override
98103
boolean testSessionId() {
99104
true

dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.3/src/test/groovy/datadog/trace/instrumentation/jetty9/Jetty9Test.groovy

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,11 @@ abstract class Jetty9Test extends HttpServerTest<Server> {
9494
true
9595
}
9696

97+
@Override
98+
boolean testBodyFilenamesCalledOnceCombined() {
99+
true
100+
}
101+
97102
@Override
98103
boolean testSessionId() {
99104
true

dd-java-agent/instrumentation/jetty/jetty-server/jetty-server-9.4.21/src/test/groovy/datadog/trace/instrumentation/jetty9/Jetty9Test.groovy

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,11 @@ abstract class Jetty9Test extends HttpServerTest<Server> {
9595
true
9696
}
9797

98+
@Override
99+
boolean testBodyFilenamesCalledOnceCombined() {
100+
true
101+
}
102+
98103
@Override
99104
boolean testSessionId() {
100105
true

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

Lines changed: 10 additions & 1 deletion
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_COMBINED
1415
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_MULTIPART_REPEATED
1516
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_URLENCODED
1617
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.CREATED
@@ -71,12 +72,20 @@ class TestServlet5 extends HttpServlet {
7172
break
7273
case BODY_MULTIPART_REPEATED:
7374
resp.status = endpoint.status
74-
// Call getParts() 3 times to verify the filenames callback fires only once
75+
// Call getParts() 3 times to verify the filenames callback fires only once
7576
req.getParts()
7677
req.getParts()
7778
req.getParts()
7879
resp.writer.print(endpoint.body)
7980
break
81+
case BODY_MULTIPART_COMBINED:
82+
resp.status = endpoint.status
83+
// Call getParameterMap() first (exercises GetFilenamesFromMultiPartAdvice via extractContentParameters),
84+
// then getParts() explicitly (GetFilenamesAdvice must not double-fire since map is already set)
85+
req.parameterMap
86+
req.getParts()
87+
resp.writer.print(endpoint.body)
88+
break
8089
case BODY_MULTIPART:
8190
case BODY_URLENCODED:
8291
resp.status = endpoint.status

dd-java-agent/instrumentation/servlet/javax-servlet/javax-servlet-3.0/src/testFixtures/groovy/datadog/trace/instrumentation/servlet3/TestServlet3.groovy

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

1616
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_URLENCODED
1717
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_MULTIPART
18+
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_MULTIPART_COMBINED
1819
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_MULTIPART_REPEATED
1920
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.CREATED
2021
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.CREATED_IS
@@ -104,6 +105,14 @@ class TestServlet3 {
104105
req.getParts()
105106
resp.writer.print(endpoint.body)
106107
break
108+
case BODY_MULTIPART_COMBINED:
109+
resp.status = endpoint.status
110+
// Call getParameterMap() first (exercises GetFilenamesFromMultiPartAdvice via extractContentParameters),
111+
// then getParts() explicitly (GetFilenamesAdvice must not double-fire since map is already set)
112+
req.parameterMap
113+
req.getParts()
114+
resp.writer.print(endpoint.body)
115+
break
107116
case BODY_URLENCODED:
108117
case BODY_MULTIPART:
109118
resp.status = endpoint.status

0 commit comments

Comments
 (0)