Skip to content

Commit 97c256f

Browse files
amarzialiamarziali
committed
Ensure servlet.path starts with /
1 parent 4dc68bb commit 97c256f

File tree

3 files changed

+75
-62
lines changed

3 files changed

+75
-62
lines changed

‎dd-java-agent/instrumentation/cxf-2.1/src/latestDepTest/groovy/CxfContextPropagationTest.groovy

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import datadog.trace.agent.test.AgentTestRunner
22
import datadog.trace.agent.test.utils.OkHttpUtils
33
import datadog.trace.agent.test.utils.TraceUtils
44
import datadog.trace.api.DDSpanTypes
5+
import datadog.trace.bootstrap.instrumentation.api.InstrumentationTags
56
import datadog.trace.bootstrap.instrumentation.api.Tags
67
import okhttp3.Request
78
import org.apache.cxf.endpoint.Server
@@ -70,7 +71,8 @@ class CxfContextPropagationTest extends AgentTestRunner {
7071
"$Tags.HTTP_METHOD" "GET"
7172
"$Tags.HTTP_STATUS" 200
7273
"$Tags.HTTP_ROUTE" String
73-
"servlet.path" "/test"
74+
"$InstrumentationTags.SERVLET_CONTEXT" "/"
75+
"$InstrumentationTags.SERVLET_PATH" "/test"
7476
"$Tags.HTTP_USER_AGENT" String
7577
"$Tags.HTTP_CLIENT_IP" "127.0.0.1"
7678
defaultTags()

‎dd-java-agent/instrumentation/jetty-12/src/main/java17/datadog/trace/instrumentation/jetty12/SetContextPathAdvice.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,16 @@ public static void updateContextPath(
3636
((AgentSpan) span).setTag(SERVLET_CONTEXT, servletContext);
3737
req.setAttribute(DD_CONTEXT_PATH_ATTRIBUTE, servletContext);
3838
if (pathInContext != null) {
39-
final String relativePath =
39+
// the following can be cached however than can be issues for application having
40+
// dynamically generated URL
41+
// since a bounded cache might collide
42+
String relativePath =
4043
pathInContext.startsWith(servletContext)
4144
? pathInContext.substring(servletContext.length())
4245
: pathInContext;
46+
if (!relativePath.isEmpty() && relativePath.charAt(0) != '/') {
47+
relativePath = "/" + relativePath;
48+
}
4349
((AgentSpan) span).setTag(SERVLET_PATH, relativePath);
4450
req.setAttribute(DD_SERVLET_PATH_ATTRIBUTE, relativePath);
4551
}

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

Lines changed: 65 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,44 @@
11
package datadog.trace.agent.test.base
22

3+
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_JSON
4+
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_MULTIPART
5+
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_URLENCODED
6+
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.CREATED
7+
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.CREATED_IS
8+
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.CUSTOM_EXCEPTION
9+
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR
10+
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION
11+
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.FORWARDED
12+
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.NOT_FOUND
13+
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.PATH_PARAM
14+
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.QUERY_ENCODED_BOTH
15+
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.QUERY_ENCODED_QUERY
16+
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.QUERY_PARAM
17+
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.REDIRECT
18+
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SESSION_ID
19+
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCESS
20+
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.TIMEOUT
21+
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.TIMEOUT_ERROR
22+
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.UNKNOWN
23+
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.USER_BLOCK
24+
import static datadog.trace.agent.test.utils.TraceUtils.basicSpan
25+
import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace
26+
import static datadog.trace.api.config.TraceInstrumentationConfig.HTTP_SERVER_RAW_QUERY_STRING
27+
import static datadog.trace.api.config.TraceInstrumentationConfig.HTTP_SERVER_RAW_RESOURCE
28+
import static datadog.trace.api.config.TraceInstrumentationConfig.HTTP_SERVER_TAG_QUERY_STRING
29+
import static datadog.trace.api.config.TraceInstrumentationConfig.SERVLET_ASYNC_TIMEOUT_ERROR
30+
import static datadog.trace.api.config.TracerConfig.HEADER_TAGS
31+
import static datadog.trace.api.config.TracerConfig.REQUEST_HEADER_TAGS
32+
import static datadog.trace.api.config.TracerConfig.RESPONSE_HEADER_TAGS
33+
import static datadog.trace.bootstrap.blocking.BlockingActionHelper.TemplateType.JSON
34+
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activeScope
35+
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activeSpan
36+
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.get
37+
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpan
38+
import static datadog.trace.bootstrap.instrumentation.decorator.HttpServerDecorator.SERVER_PATHWAY_EDGE_TAGS
39+
import static java.nio.charset.StandardCharsets.UTF_8
40+
import static org.junit.Assume.assumeTrue
41+
342
import ch.qos.logback.classic.Level
443
import datadog.appsec.api.blocking.Blocking
544
import datadog.appsec.api.blocking.BlockingContentType
@@ -28,6 +67,7 @@ import datadog.trace.api.iast.IastContext
2867
import datadog.trace.api.normalize.SimpleHttpPathNormalizer
2968
import datadog.trace.bootstrap.blocking.BlockingActionHelper
3069
import datadog.trace.bootstrap.instrumentation.api.AgentTracer
70+
import datadog.trace.bootstrap.instrumentation.api.InstrumentationTags
3171
import datadog.trace.bootstrap.instrumentation.api.Tags
3272
import datadog.trace.bootstrap.instrumentation.api.URIDataAdapter
3373
import datadog.trace.bootstrap.instrumentation.api.URIUtils
@@ -50,45 +90,6 @@ import java.util.function.BiFunction
5090
import java.util.function.Function
5191
import java.util.function.Supplier
5292

53-
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_JSON
54-
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_MULTIPART
55-
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_URLENCODED
56-
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.CREATED
57-
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.CREATED_IS
58-
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.CUSTOM_EXCEPTION
59-
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR
60-
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION
61-
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.FORWARDED
62-
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.NOT_FOUND
63-
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.PATH_PARAM
64-
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.QUERY_ENCODED_BOTH
65-
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.QUERY_ENCODED_QUERY
66-
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.QUERY_PARAM
67-
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.REDIRECT
68-
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SESSION_ID
69-
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCESS
70-
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.TIMEOUT
71-
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.TIMEOUT_ERROR
72-
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.UNKNOWN
73-
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.USER_BLOCK
74-
import static datadog.trace.agent.test.utils.TraceUtils.basicSpan
75-
import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace
76-
import static datadog.trace.api.config.TraceInstrumentationConfig.HTTP_SERVER_RAW_QUERY_STRING
77-
import static datadog.trace.api.config.TraceInstrumentationConfig.HTTP_SERVER_RAW_RESOURCE
78-
import static datadog.trace.api.config.TraceInstrumentationConfig.HTTP_SERVER_TAG_QUERY_STRING
79-
import static datadog.trace.api.config.TraceInstrumentationConfig.SERVLET_ASYNC_TIMEOUT_ERROR
80-
import static datadog.trace.api.config.TracerConfig.HEADER_TAGS
81-
import static datadog.trace.api.config.TracerConfig.REQUEST_HEADER_TAGS
82-
import static datadog.trace.api.config.TracerConfig.RESPONSE_HEADER_TAGS
83-
import static datadog.trace.bootstrap.blocking.BlockingActionHelper.TemplateType.JSON
84-
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activeScope
85-
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activeSpan
86-
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.get
87-
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpan
88-
import static datadog.trace.bootstrap.instrumentation.decorator.HttpServerDecorator.SERVER_PATHWAY_EDGE_TAGS
89-
import static java.nio.charset.StandardCharsets.UTF_8
90-
import static org.junit.Assume.assumeTrue
91-
9293
abstract class HttpServerTest<SERVER> extends WithHttpServer<SERVER> {
9394

9495
public static final Logger SERVER_LOGGER = LoggerFactory.getLogger("http-server")
@@ -208,9 +209,9 @@ abstract class HttpServerTest<SERVER> extends WithHttpServer<SERVER> {
208209
// Only used if hasExtraErrorInformation is true
209210
Map<String, Serializable> expectedExtraErrorInformation(ServerEndpoint endpoint) {
210211
if (endpoint.errored) {
211-
["error.message" : { it == null || it == EXCEPTION.body },
212-
"error.type" : { it == null || it == Exception.name },
213-
"error.stack": { it == null || it instanceof String }]
212+
["error.message": { it == null || it == EXCEPTION.body },
213+
"error.type" : { it == null || it == Exception.name },
214+
"error.stack" : { it == null || it instanceof String }]
214215
} else {
215216
Collections.emptyMap()
216217
}
@@ -492,8 +493,8 @@ abstract class HttpServerTest<SERVER> extends WithHttpServer<SERVER> {
492493
}
493494

494495
private static final Map<String, ServerEndpoint> PATH_MAP = {
495-
Map<String, ServerEndpoint> map = values().collectEntries { [it.path, it]}
496-
map.putAll(values().collectEntries { [it.rawPath, it]})
496+
Map<String, ServerEndpoint> map = values().collectEntries { [it.path, it] }
497+
map.putAll(values().collectEntries { [it.rawPath, it] })
497498
map
498499
}.call()
499500

@@ -701,9 +702,9 @@ abstract class HttpServerTest<SERVER> extends WithHttpServer<SERVER> {
701702
}
702703

703704
where:
704-
method | body | header | value | tags
705-
'GET' | null | 'x-datadog-test-both-header' | 'foo' | [ 'both_header_tag': 'foo' ]
706-
'GET' | null | 'x-datadog-test-request-header' | 'bar' | [ 'request_header_tag': 'bar' ]
705+
method | body | header | value | tags
706+
'GET' | null | 'x-datadog-test-both-header' | 'foo' | ['both_header_tag': 'foo']
707+
'GET' | null | 'x-datadog-test-request-header' | 'bar' | ['request_header_tag': 'bar']
707708
}
708709

709710
@Flaky(value = "https://github.com/DataDog/dd-trace-java/issues/4690", suites = ["MuleHttpServerForkedTest"])
@@ -715,7 +716,7 @@ abstract class HttpServerTest<SERVER> extends WithHttpServer<SERVER> {
715716
def body = null
716717
def header = IG_RESPONSE_HEADER
717718
def mapping = 'mapped_response_header_tag'
718-
def tags = ['mapped_response_header_tag': "$IG_RESPONSE_HEADER_VALUE" ]
719+
def tags = ['mapped_response_header_tag': "$IG_RESPONSE_HEADER_VALUE"]
719720

720721
injectSysConfig(HTTP_SERVER_TAG_QUERY_STRING, "true")
721722
injectSysConfig(RESPONSE_HEADER_TAGS, "$header:$mapping")
@@ -798,13 +799,13 @@ abstract class HttpServerTest<SERVER> extends WithHttpServer<SERVER> {
798799
}
799800

800801
where:
801-
rawQuery | endpoint | encoded
802-
true | SUCCESS | false
803-
true | QUERY_PARAM | false
804-
true | QUERY_ENCODED_QUERY | true
805-
false | SUCCESS | false
806-
false | QUERY_PARAM | false
807-
false | QUERY_ENCODED_QUERY | true
802+
rawQuery | endpoint | encoded
803+
true | SUCCESS | false
804+
true | QUERY_PARAM | false
805+
true | QUERY_ENCODED_QUERY | true
806+
false | SUCCESS | false
807+
false | QUERY_PARAM | false
808+
false | QUERY_ENCODED_QUERY | true
808809

809810
method = "GET"
810811
body = null
@@ -917,7 +918,7 @@ abstract class HttpServerTest<SERVER> extends WithHttpServer<SERVER> {
917918
}
918919

919920
then:
920-
DDSpan span = TEST_WRITER.flatten().find {it.operationName =='appsec-span' }
921+
DDSpan span = TEST_WRITER.flatten().find { it.operationName == 'appsec-span' }
921922
span.getTag(IG_PATH_PARAMS_TAG) == expectedIGPathParams()
922923

923924
and:
@@ -1611,7 +1612,7 @@ abstract class HttpServerTest<SERVER> extends WithHttpServer<SERVER> {
16111612
then:
16121613
TEST_WRITER.waitForTraces(1)
16131614
def trace = TEST_WRITER.get(0)
1614-
assert trace.find {it.isError() } == null
1615+
assert trace.find { it.isError() } == null
16151616
}
16161617

16171618
def 'test blocking of request for path parameters'() {
@@ -1692,7 +1693,8 @@ abstract class HttpServerTest<SERVER> extends WithHttpServer<SERVER> {
16921693
spans.find { it.tags['appsec.blocked'] == 'true' } != null
16931694
spans.find {
16941695
it.error &&
1695-
it.tags['error.type'] == BlockingException.name } != null
1696+
it.tags['error.type'] == BlockingException.name
1697+
} != null
16961698

16971699
and:
16981700
if (isDataStreamsEnabled()) {
@@ -1872,7 +1874,7 @@ abstract class HttpServerTest<SERVER> extends WithHttpServer<SERVER> {
18721874
if (isDataStreamsEnabled()) {
18731875
TEST_DATA_STREAMS_WRITER.waitForGroups(1)
18741876
}
1875-
DDSpan span = TEST_WRITER.flatten().find {it.operationName =='appsec-span' }
1877+
DDSpan span = TEST_WRITER.flatten().find { it.operationName == 'appsec-span' }
18761878
span != null
18771879
final sessionId = span.tags[IG_SESSION_ID_TAG]
18781880
sessionId != null
@@ -1964,7 +1966,10 @@ abstract class HttpServerTest<SERVER> extends WithHttpServer<SERVER> {
19641966
"$Tags.HTTP_FORWARDED_IP" endpoint.body
19651967
}
19661968
if (null != expectedServerSpanRoute) {
1967-
"$Tags.HTTP_ROUTE" expectedServerSpanRoute
1969+
"$Tags.HTTP_ROUTE".expectedServerSpanRoute
1970+
}
1971+
if (span.getTag(InstrumentationTags.SERVLET_PATH) != null) {
1972+
assert span.getTag(InstrumentationTags.SERVLET_PATH).toString().startsWith("/")
19681973
}
19691974
if (null != expectedExtraErrorInformation) {
19701975
addTags(expectedExtraErrorInformation)

0 commit comments

Comments
 (0)