Skip to content

Commit 9700e85

Browse files
committed
Fix DD_TRACE_SYMFONY_HTTP_ROUTE=false
DD_TRACE_SYMFONY_HTTP_ROUTE=false incorrectly fully disabled Symfony request handling instrumentation. Properly scoped it to its original intent: the extraction of the Symfony route path for meta tag http.route.
1 parent df4dde8 commit 9700e85

2 files changed

Lines changed: 44 additions & 39 deletions

File tree

appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/Symfony62Tests.groovy

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -125,14 +125,17 @@ class Symfony62Tests {
125125
service apache2 restart''')
126126
assert res.exitCode == 0
127127

128+
// path params are always pushed to AppSec regardless of DD_TRACE_SYMFONY_HTTP_ROUTE,
129+
// so the WAF still blocks based on the path param key 'param01'
128130
HttpRequest req = container.buildReq('/dynamic-path/someValue').GET().build()
129131
def trace = container.traceFromRequest(req, ofString()) { HttpResponse<String> re ->
130-
assert re.statusCode() == 200
131-
assert re.body().contains('Hi someValue!')
132+
assert re.statusCode() == 403
132133
}
133134

134135
Span span = trace.first()
135-
assert span.meta."http.route" != '/dynamic-path/{param01}'
136+
assert span.meta."http.route" == null
137+
assert span.meta."symfony.route.name" != null
138+
assert span.resource == 'app_home_dynamic'
136139
} finally {
137140
def res = CONTAINER.execInContainer(
138141
'bash', '-c',

src/DDTrace/Integrations/Symfony/SymfonyIntegration.php

Lines changed: 38 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -527,50 +527,52 @@ static function() {
527527
$rootSpan->meta[Tag::HTTP_ROUTE] = $path;
528528
}
529529
};
530+
} else {
531+
$handle_http_route = static function() { /* noop */ };
532+
}
530533

531-
\DDTrace\trace_method(
532-
'Symfony\Component\HttpKernel\HttpKernel',
533-
'handle',
534-
static function(SpanData $span, $args, $response) use ($handle_http_route) {
535-
/** @var Request $request */
536-
list($request) = $args;
534+
\DDTrace\trace_method(
535+
'Symfony\Component\HttpKernel\HttpKernel',
536+
'handle',
537+
static function(SpanData $span, $args, $response) use ($handle_http_route) {
538+
/** @var Request $request */
539+
list($request) = $args;
537540

538-
$span->name = 'symfony.kernel.handle';
539-
$span->service = \ddtrace_config_app_name(self::$frameworkPrefix);
540-
$span->type = Type::WEB_SERVLET;
541-
$span->meta[Tag::COMPONENT] = self::NAME;
541+
$span->name = 'symfony.kernel.handle';
542+
$span->service = \ddtrace_config_app_name(self::$frameworkPrefix);
543+
$span->type = Type::WEB_SERVLET;
544+
$span->meta[Tag::COMPONENT] = self::NAME;
542545

543-
$rootSpan = \DDTrace\root_span();
544-
$rootSpan->meta[Tag::HTTP_METHOD] = $request->getMethod();
545-
$rootSpan->meta[Tag::COMPONENT] = self::$frameworkPrefix;
546-
$rootSpan->meta[Tag::SPAN_KIND] = 'server';
547-
self::addTraceAnalyticsIfEnabled($rootSpan);
546+
$rootSpan = \DDTrace\root_span();
547+
$rootSpan->meta[Tag::HTTP_METHOD] = $request->getMethod();
548+
$rootSpan->meta[Tag::COMPONENT] = self::$frameworkPrefix;
549+
$rootSpan->meta[Tag::SPAN_KIND] = 'server';
550+
self::addTraceAnalyticsIfEnabled($rootSpan);
548551

549-
if (!array_key_exists(Tag::HTTP_URL, $rootSpan->meta)) {
550-
$rootSpan->meta[Tag::HTTP_URL] = Normalizer::urlSanitize($request->getUri());
551-
}
552-
if (isset($response)) {
553-
$rootSpan->meta[Tag::HTTP_STATUS_CODE] = $response->getStatusCode();
554-
}
552+
if (!array_key_exists(Tag::HTTP_URL, $rootSpan->meta)) {
553+
$rootSpan->meta[Tag::HTTP_URL] = Normalizer::urlSanitize($request->getUri());
554+
}
555+
if (isset($response)) {
556+
$rootSpan->meta[Tag::HTTP_STATUS_CODE] = $response->getStatusCode();
557+
}
555558

556-
$route_name = $request->get('_route');
557-
if ($route_name !== null) {
558-
if (dd_trace_env_config("DD_HTTP_SERVER_ROUTE_BASED_NAMING")) {
559-
$rootSpan->resource = $route_name;
560-
}
561-
$rootSpan->meta['symfony.route.name'] = $route_name;
562-
$handle_http_route($route_name, $request, $rootSpan);
559+
$route_name = $request->get('_route');
560+
if ($route_name !== null) {
561+
if (dd_trace_env_config("DD_HTTP_SERVER_ROUTE_BASED_NAMING")) {
562+
$rootSpan->resource = $route_name;
563563
}
564+
$rootSpan->meta['symfony.route.name'] = $route_name;
565+
$handle_http_route($route_name, $request, $rootSpan);
566+
}
564567

565-
$parameters = $request->get('_route_params');
566-
if (!empty($parameters) &&
567-
is_array($parameters) &&
568-
function_exists('datadog\appsec\push_addresses')) {
569-
\datadog\appsec\push_addresses(["server.request.path_params" => $parameters]);
570-
}
568+
$parameters = $request->get('_route_params');
569+
if (!empty($parameters) &&
570+
is_array($parameters) &&
571+
function_exists('datadog\appsec\push_addresses')) {
572+
\datadog\appsec\push_addresses(["server.request.path_params" => $parameters]);
571573
}
572-
);
573-
}
574+
}
575+
);
574576

575577
/*
576578
* EventDispatcher v4.3 introduced an arg hack that mutates the arguments.

0 commit comments

Comments
 (0)