From 7966052880faec6c03d6992fed1c020ed5ba7b95 Mon Sep 17 00:00:00 2001 From: Gustavo Lopes Date: Wed, 25 Oct 2023 18:40:08 +0100 Subject: [PATCH] AppSec Xml support for play 2.6 --- .../play26/server/PlayRouters.groovy | 19 +++ .../play26/server/PlayServerTest.groovy | 28 ++++ .../server/latestdep/PlayRouters.groovy | 30 ++++ .../src/latestDepTest/routes/conf/routes | 1 + .../latestdep/ImplicitConversions.scala | 2 - .../server/latestdep/PlayController.scala | 6 + .../server/latestdep/PlayRoutersScala.scala | 8 + .../play26/appsec/BodyParserHelpers.java | 147 ++++++++++++++++++ .../PlayBodyParsersInstrumentation.java | 14 ++ .../appsec/TolerantJsonInstrumentation.java | 3 + .../appsec/TolerantXmlInstrumentation.java | 72 +++++++++ .../agent/test/base/HttpServerTest.groovy | 6 +- 12 files changed, 332 insertions(+), 4 deletions(-) create mode 100644 dd-java-agent/instrumentation/play-2.6/src/main/java/datadog/trace/instrumentation/play26/appsec/TolerantXmlInstrumentation.java diff --git a/dd-java-agent/instrumentation/play-2.6/src/baseTest/groovy/datadog/trace/instrumentation/play26/server/PlayRouters.groovy b/dd-java-agent/instrumentation/play-2.6/src/baseTest/groovy/datadog/trace/instrumentation/play26/server/PlayRouters.groovy index 07beb2adab6..2d3925a7f28 100644 --- a/dd-java-agent/instrumentation/play-2.6/src/baseTest/groovy/datadog/trace/instrumentation/play26/server/PlayRouters.groovy +++ b/dd-java-agent/instrumentation/play-2.6/src/baseTest/groovy/datadog/trace/instrumentation/play26/server/PlayRouters.groovy @@ -15,6 +15,7 @@ import play.routing.Router import play.routing.RoutingDsl import scala.collection.JavaConverters import scala.concurrent.ExecutionContextExecutor +import scala.xml.NodeSeq import java.util.concurrent.CompletableFuture import java.util.concurrent.CompletionStage @@ -26,6 +27,7 @@ import java.util.function.Supplier import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_JSON import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_MULTIPART import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_URLENCODED +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_XML import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.CREATED import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.CUSTOM_EXCEPTION import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR @@ -151,6 +153,13 @@ class PlayRouters { Results.status(BODY_JSON.status, Json$.MODULE$.stringify(json)) } } as Supplier) + .POST(BODY_XML.path).routeTo({ + -> + controller(BODY_XML) { + NodeSeq node = body().asXml().get() + Results.status(BODY_XML.status, node.toString()) + } + } as Supplier) .build() } @@ -281,6 +290,16 @@ class PlayRouters { } }, execContext) } as Supplier) + .POST(BODY_XML.path).routeAsync({ + -> + NodeSeq node = body().asXml().get() + CompletableFuture.supplyAsync({ + -> + controller(BODY_XML) { + Results.status(BODY_XML.status, node.toString()) + } + }, execContext) + } as Supplier) .build() } diff --git a/dd-java-agent/instrumentation/play-2.6/src/baseTest/groovy/datadog/trace/instrumentation/play26/server/PlayServerTest.groovy b/dd-java-agent/instrumentation/play-2.6/src/baseTest/groovy/datadog/trace/instrumentation/play26/server/PlayServerTest.groovy index 4eb4a367c79..26ca37d551b 100644 --- a/dd-java-agent/instrumentation/play-2.6/src/baseTest/groovy/datadog/trace/instrumentation/play26/server/PlayServerTest.groovy +++ b/dd-java-agent/instrumentation/play-2.6/src/baseTest/groovy/datadog/trace/instrumentation/play26/server/PlayServerTest.groovy @@ -8,8 +8,11 @@ import datadog.trace.api.DDTags import datadog.trace.bootstrap.instrumentation.api.Tags import datadog.trace.instrumentation.play26.PlayHttpServerDecorator import groovy.transform.CompileStatic +import okhttp3.MediaType +import okhttp3.RequestBody import play.server.Server +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_XML import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.CUSTOM_EXCEPTION import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.FORWARDED @@ -142,4 +145,29 @@ class PlayServerTest extends HttpServerTest { } } } + + def 'test instrumentation gateway xml request body'() { + setup: + def request = request( + BODY_XML, 'POST', + RequestBody.create(MediaType.get('text/xml'), 'mytext')) + .build() + def response = client.newCall(request).execute() + if (isDataStreamsEnabled()) { + TEST_DATA_STREAMS_WRITER.waitForGroups(1) + } + String body = response.body().charStream().text + + + expect: + body == BODY_XML.body || body == 'mytext' + + when: + TEST_WRITER.waitForTraces(1) + + then: + TEST_WRITER.get(0).any { + it.getTag('request.body.converted') == '[[children:[mytext, [:]], attributes:[attr:attr_value]]]' + } + } } diff --git a/dd-java-agent/instrumentation/play-2.6/src/latestDepTest/groovy/datadog/trace/instrumentation/play26/server/latestdep/PlayRouters.groovy b/dd-java-agent/instrumentation/play-2.6/src/latestDepTest/groovy/datadog/trace/instrumentation/play26/server/latestdep/PlayRouters.groovy index c9773648e1b..4bf05bb12c5 100644 --- a/dd-java-agent/instrumentation/play-2.6/src/latestDepTest/groovy/datadog/trace/instrumentation/play26/server/latestdep/PlayRouters.groovy +++ b/dd-java-agent/instrumentation/play-2.6/src/latestDepTest/groovy/datadog/trace/instrumentation/play26/server/latestdep/PlayRouters.groovy @@ -6,6 +6,7 @@ import datadog.appsec.api.blocking.Blocking import datadog.trace.agent.test.base.HttpServerTest import datadog.trace.instrumentation.play26.server.TestHttpErrorHandler import groovy.transform.CompileStatic +import org.w3c.dom.Document import play.BuiltInComponents import play.libs.concurrent.ClassLoaderExecution import play.mvc.Http @@ -16,6 +17,10 @@ import play.routing.Router import play.routing.RoutingDsl import scala.concurrent.ExecutionContextExecutor +import javax.xml.transform.OutputKeys +import javax.xml.transform.TransformerFactory +import javax.xml.transform.dom.DOMSource +import javax.xml.transform.stream.StreamResult import java.util.concurrent.CompletableFuture import java.util.concurrent.CompletionStage import java.util.concurrent.ExecutorService @@ -23,6 +28,7 @@ import java.util.concurrent.ExecutorService import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_JSON import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_MULTIPART import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_URLENCODED +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_XML import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.CREATED import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.CUSTOM_EXCEPTION import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR @@ -118,6 +124,12 @@ class PlayRouters { Results.status(BODY_JSON.status, new ObjectMapper().writeValueAsString(json)) } } as RequestFunctions.Params0) + .POST(BODY_XML.path).routingTo({ Http.Request req -> + controller(BODY_XML) { + Document xml = req.body().asXml() + Results.status(BODY_XML.status, documentToString(xml)) + } + } as RequestFunctions.Params0) .build() } @@ -246,6 +258,15 @@ class PlayRouters { } }, execContext) } as RequestFunctions.Params0>) + .POST(BODY_XML.path).routingAsync({ Http.Request req -> + Document xml = req.body().asXml() + CompletableFuture.supplyAsync({ + -> + controller(BODY_XML) { + Results.status(BODY_XML.status, documentToString(xml)) + } + }, execContext) + } as RequestFunctions.Params0>) .build() } @@ -263,4 +284,13 @@ class PlayRouters { }) } } + + private static String documentToString(Document doc) { + StringWriter writer = new StringWriter() + TransformerFactory.newInstance().newTransformer().with { + setOutputProperty(OutputKeys.INDENT, "no") + transform(new DOMSource(doc), new StreamResult(writer)) + } + writer.toString() + } } diff --git a/dd-java-agent/instrumentation/play-2.6/src/latestDepTest/routes/conf/routes b/dd-java-agent/instrumentation/play-2.6/src/latestDepTest/routes/conf/routes index 3fa95e8631a..0ba1ae9cff2 100644 --- a/dd-java-agent/instrumentation/play-2.6/src/latestDepTest/routes/conf/routes +++ b/dd-java-agent/instrumentation/play-2.6/src/latestDepTest/routes/conf/routes @@ -14,3 +14,4 @@ POST /created datadog.trace.instrumentation.play26.server. POST /body-urlencoded datadog.trace.instrumentation.play26.server.latestdep.PlayController.bodyUrlencoded() POST /body-multipart datadog.trace.instrumentation.play26.server.latestdep.PlayController.bodyMultipart() POST /body-json datadog.trace.instrumentation.play26.server.latestdep.PlayController.bodyJson() +POST /body-xml datadog.trace.instrumentation.play26.server.latestdep.PlayController.bodyXml() diff --git a/dd-java-agent/instrumentation/play-2.6/src/latestDepTest/scala/datadog/trace/instrumentation/play26/server/latestdep/ImplicitConversions.scala b/dd-java-agent/instrumentation/play-2.6/src/latestDepTest/scala/datadog/trace/instrumentation/play26/server/latestdep/ImplicitConversions.scala index 701f93d0c41..7896e2836ae 100644 --- a/dd-java-agent/instrumentation/play-2.6/src/latestDepTest/scala/datadog/trace/instrumentation/play26/server/latestdep/ImplicitConversions.scala +++ b/dd-java-agent/instrumentation/play-2.6/src/latestDepTest/scala/datadog/trace/instrumentation/play26/server/latestdep/ImplicitConversions.scala @@ -1,7 +1,5 @@ package datadog.trace.instrumentation.play26.server.latestdep -import scala.collection.Seq - object ImplicitConversions { implicit class MapExtensions[A](m: Iterable[(String, A)]) { def toStringAsGroovy: String = { diff --git a/dd-java-agent/instrumentation/play-2.6/src/latestDepTest/scala/datadog/trace/instrumentation/play26/server/latestdep/PlayController.scala b/dd-java-agent/instrumentation/play-2.6/src/latestDepTest/scala/datadog/trace/instrumentation/play26/server/latestdep/PlayController.scala index cb9c82aa748..65a360ad27f 100644 --- a/dd-java-agent/instrumentation/play-2.6/src/latestDepTest/scala/datadog/trace/instrumentation/play26/server/latestdep/PlayController.scala +++ b/dd-java-agent/instrumentation/play-2.6/src/latestDepTest/scala/datadog/trace/instrumentation/play26/server/latestdep/PlayController.scala @@ -10,6 +10,7 @@ import play.api.libs.json.{JsNull, JsValue, Json} import play.api.mvc._ import scala.concurrent.{ExecutionContext, Future} +import scala.xml.NodeSeq class PlayController(cc: ControllerComponents)(implicit ec: ExecutionContext) extends AbstractController(cc) { def success() = controller(ServerEndpoint.SUCCESS) { _ => @@ -81,6 +82,11 @@ class PlayController(cc: ControllerComponents)(implicit ec: ExecutionContext) ex Results.Ok(Json.stringify(body)) } + def bodyXml = controller(ServerEndpoint.BODY_XML) { request => + val body : NodeSeq = request.body.asXml.getOrElse(NodeSeq.Empty) + Results.Ok(body.toString()) + } + private def controller(endpoint: ServerEndpoint)(block: Request[AnyContent] => Result) : Action[AnyContent] = { Action.async { request => Future { diff --git a/dd-java-agent/instrumentation/play-2.6/src/latestDepTest/scala/datadog/trace/instrumentation/play26/server/latestdep/PlayRoutersScala.scala b/dd-java-agent/instrumentation/play-2.6/src/latestDepTest/scala/datadog/trace/instrumentation/play26/server/latestdep/PlayRoutersScala.scala index 6a319eee403..aa3256e96ed 100644 --- a/dd-java-agent/instrumentation/play-2.6/src/latestDepTest/scala/datadog/trace/instrumentation/play26/server/latestdep/PlayRoutersScala.scala +++ b/dd-java-agent/instrumentation/play-2.6/src/latestDepTest/scala/datadog/trace/instrumentation/play26/server/latestdep/PlayRoutersScala.scala @@ -15,6 +15,7 @@ import play.api.routing.sird._ import java.util.concurrent.ExecutorService import scala.concurrent.{ExecutionContext, Future} +import scala.xml.NodeSeq object PlayRoutersScala { @@ -149,6 +150,13 @@ object PlayRoutersScala { Results.Ok(Json.stringify(body)) } } + + case POST(p"/body-xml") => defaultActionBuilder.async(parser) { request => + controller(BODY_XML) { + val body: NodeSeq = request.body.asXml.getOrElse(NodeSeq.Empty) + Results.Ok(body.toString()) + } + } } } diff --git a/dd-java-agent/instrumentation/play-2.6/src/main/java/datadog/trace/instrumentation/play26/appsec/BodyParserHelpers.java b/dd-java-agent/instrumentation/play-2.6/src/main/java/datadog/trace/instrumentation/play26/appsec/BodyParserHelpers.java index 8ace882c0a3..87afd198a69 100644 --- a/dd-java-agent/instrumentation/play-2.6/src/main/java/datadog/trace/instrumentation/play26/appsec/BodyParserHelpers.java +++ b/dd-java-agent/instrumentation/play-2.6/src/main/java/datadog/trace/instrumentation/play26/appsec/BodyParserHelpers.java @@ -20,12 +20,18 @@ import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.bootstrap.instrumentation.api.AgentTracer; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.function.BiFunction; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.w3c.dom.Attr; +import org.w3c.dom.Document; +import org.w3c.dom.Element; +import org.w3c.dom.NamedNodeMap; +import org.w3c.dom.NodeList; import play.api.libs.json.JsArray; import play.api.libs.json.JsBoolean; import play.api.libs.json.JsNumber; @@ -39,6 +45,10 @@ import scala.collection.Iterator; import scala.collection.Seq; import scala.compat.java8.JFunction1; +import scala.xml.Elem; +import scala.xml.Node; +import scala.xml.NodeSeq; +import scala.xml.Text; public class BodyParserHelpers { @@ -54,6 +64,7 @@ public class BodyParserHelpers { private static JFunction1, MultipartFormData> HANDLE_MULTIPART_FORM_DATA = BodyParserHelpers::handleMultipartFormData; private static JFunction1 HANDLE_JSON = BodyParserHelpers::handleJson; + private static JFunction1 HANDLE_XML = BodyParserHelpers::handleXml; private BodyParserHelpers() {} @@ -126,6 +137,10 @@ private static JsValue handleJson(JsValue data) { if (data == null) { return null; } + if (!isAppsecActiveForRequest()) { + // skip potentially expensive transformation + return data; + } try { Object conv = jsValueToJavaObject(data, MAX_RECURSION); @@ -136,6 +151,28 @@ private static JsValue handleJson(JsValue data) { return data; } + public static Function1 getHandleXmlF() { + return HANDLE_XML; + } + + private static NodeSeq handleXml(NodeSeq data) { + if (data == null) { + return null; + } + if (!isAppsecActiveForRequest()) { + // skip potentially expensive transformation + return data; + } + + try { + Object conv = nodeSeqToJavaObject(data, MAX_RECURSION); + handleArbitraryPostData(conv, "xml"); + } catch (Exception e) { + handleException(e, "Error handling result of xml BodyParser"); + } + return data; + } + private static void executeCallback( RequestContext reqCtx, BiFunction> callback, @@ -190,6 +227,15 @@ public static void handleJsonNode(JsonNode n, String source) { handleArbitraryPostDataWithSpanError(o, source); } + private static boolean isAppsecActiveForRequest() { + AgentSpan span = activeSpan(); + RequestContext reqCtx; + + return span != null + && (reqCtx = span.getRequestContext()) != null + && reqCtx.getData(RequestContextSlot.APPSEC) != null; + } + public static void handleArbitraryPostDataWithSpanError(Object o, String source) { AgentSpan span = activeSpan(); try { @@ -300,4 +346,105 @@ private static Object jsNodeToJavaObject(JsonNode value, int maxRecursion) { return value.asText(""); } } + + private static Object nodeSeqToJavaObject(NodeSeq nodeSeq, int maxRecursion) { + if (nodeSeq.length() == 0 || maxRecursion <= 0) { + return null; + } + + List ret = new ArrayList<>(nodeSeq.length()); + Iterator iterator = nodeSeq.iterator(); + while (iterator.hasNext()) { + Node node = iterator.next(); + ret.add(nodeToJavaObject(node, maxRecursion)); + } + return ret; + } + + // we don't preserve element names + private static Object nodeToJavaObject(Node node, int maxRecursion) { + if (node == null || maxRecursion <= 0) { + return null; + } + + if (node instanceof Elem) { + scala.collection.immutable.Map attributes = node.attributes().asAttrMap(); + + int size = node.child().size(); + List childList = Collections.emptyList(); + if (size > 0) { + childList = new ArrayList<>(); + Iterator iterator = node.child().iterator(); + while (iterator.hasNext()) { + Node childNode = iterator.next(); + childList.add(nodeToJavaObject(childNode, maxRecursion - 1)); + } + } + + Map nodeRepr = new HashMap<>(); + if (!attributes.isEmpty()) { + nodeRepr.put("attributes", tryConvertingScalaContainers(attributes, 1)); + } + if (!childList.isEmpty()) { + nodeRepr.put("children", childList); + } + return nodeRepr; + } else if (node instanceof Text) { + return node.text().trim(); + } else { + return null; + } + } + + public static void handleXmlDocument(Document ret, String source) { + if (ret == null) { + return; + } + + Element documentElement = ret.getDocumentElement(); + Object obj = convertW3cNode(documentElement, MAX_RECURSION); + + // pass wrapped in a list for consistency with NodeSeq (scala) variant + handleArbitraryPostDataWithSpanError(Collections.singletonList(obj), source); + } + + private static Object convertW3cNode(org.w3c.dom.Node node, int maxRecursion) { + if (node == null || maxRecursion <= 0) { + return null; + } + + if (node instanceof Element) { + Map attributes = Collections.emptyMap(); + if (node.hasAttributes()) { + attributes = new HashMap<>(); + NamedNodeMap attrMap = node.getAttributes(); + for (int i = 0; i < attrMap.getLength(); i++) { + org.w3c.dom.Attr item = (Attr) attrMap.item(i); + attributes.put(item.getName(), item.getValue()); + } + } + + List children = Collections.emptyList(); + if (node.hasChildNodes()) { + NodeList childNodes = node.getChildNodes(); + children = new ArrayList<>(childNodes.getLength()); + for (int i = 0; i < childNodes.getLength(); i++) { + org.w3c.dom.Node item = childNodes.item(i); + children.add(convertW3cNode(item, maxRecursion - 1)); + } + } + + Map repr = new HashMap<>(); + if (!attributes.isEmpty()) { + repr.put("attributes", attributes); + } + if (!children.isEmpty()) { + repr.put("children", children); + } + return repr; + } else if (node instanceof org.w3c.dom.Text) { + return node.getTextContent(); + } + return null; + } } diff --git a/dd-java-agent/instrumentation/play-2.6/src/main/java/datadog/trace/instrumentation/play26/appsec/PlayBodyParsersInstrumentation.java b/dd-java-agent/instrumentation/play-2.6/src/main/java/datadog/trace/instrumentation/play26/appsec/PlayBodyParsersInstrumentation.java index 7394b343309..0b74d0be928 100644 --- a/dd-java-agent/instrumentation/play-2.6/src/main/java/datadog/trace/instrumentation/play26/appsec/PlayBodyParsersInstrumentation.java +++ b/dd-java-agent/instrumentation/play-2.6/src/main/java/datadog/trace/instrumentation/play26/appsec/PlayBodyParsersInstrumentation.java @@ -17,6 +17,7 @@ import play.core.Execution; import scala.collection.Seq; import scala.collection.immutable.Map; +import scala.xml.NodeSeq; /** @see play.api.mvc.PlayBodyParsers$class#tolerantFormUrlEncoded(PlayBodyParsers, int) */ @AutoService(Instrumenter.class) @@ -81,6 +82,10 @@ public void adviceTransformations(AdviceTransformation transformation) { isTraitMethod(TRAIT_NAME, "tolerantJson", is(int.class).or(is(long.class))) .and(returns(named("play.api.mvc.BodyParser"))), PlayBodyParsersInstrumentation.class.getName() + "$JsonAdvice"); + transformation.applyAdvice( + isTraitMethod(TRAIT_NAME, "tolerantXml", is(int.class).or(is(long.class))) + .and(returns(named("play.api.mvc.BodyParser"))), + PlayBodyParsersInstrumentation.class.getName() + "$XmlAdvice"); } static class UrlEncodedAdvice { @@ -134,4 +139,13 @@ static void after(@Advice.Return(readOnly = false) BodyParser parser) { parser.map(BodyParserHelpers.getHandleJsonF(), Execution.Implicits$.MODULE$.trampoline()); } } + + static class XmlAdvice { + @Advice.OnMethodExit(suppress = Throwable.class) + static void after(@Advice.Return(readOnly = false) BodyParser parser) { + + parser = + parser.map(BodyParserHelpers.getHandleXmlF(), Execution.Implicits$.MODULE$.trampoline()); + } + } } diff --git a/dd-java-agent/instrumentation/play-2.6/src/main/java/datadog/trace/instrumentation/play26/appsec/TolerantJsonInstrumentation.java b/dd-java-agent/instrumentation/play-2.6/src/main/java/datadog/trace/instrumentation/play26/appsec/TolerantJsonInstrumentation.java index 291b0f6fdb7..7d959f0cbfd 100644 --- a/dd-java-agent/instrumentation/play-2.6/src/main/java/datadog/trace/instrumentation/play26/appsec/TolerantJsonInstrumentation.java +++ b/dd-java-agent/instrumentation/play-2.6/src/main/java/datadog/trace/instrumentation/play26/appsec/TolerantJsonInstrumentation.java @@ -9,8 +9,10 @@ import com.fasterxml.jackson.databind.JsonNode; import com.google.auto.service.AutoService; import datadog.appsec.api.blocking.BlockingException; +import datadog.trace.advice.RequiresRequestContext; import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.muzzle.Reference; +import datadog.trace.api.gateway.RequestContextSlot; import datadog.trace.instrumentation.play26.MuzzleReferences; import net.bytebuddy.asm.Advice; import play.mvc.Http; @@ -56,6 +58,7 @@ public void adviceTransformations(AdviceTransformation transformation) { TolerantJsonInstrumentation.class.getName() + "$ParseAdvice"); } + @RequiresRequestContext(RequestContextSlot.APPSEC) static class ParseAdvice { @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class) static void after(@Advice.Return JsonNode ret, @Advice.Thrown(readOnly = false) Throwable t) { diff --git a/dd-java-agent/instrumentation/play-2.6/src/main/java/datadog/trace/instrumentation/play26/appsec/TolerantXmlInstrumentation.java b/dd-java-agent/instrumentation/play-2.6/src/main/java/datadog/trace/instrumentation/play26/appsec/TolerantXmlInstrumentation.java new file mode 100644 index 00000000000..878403c43ce --- /dev/null +++ b/dd-java-agent/instrumentation/play-2.6/src/main/java/datadog/trace/instrumentation/play26/appsec/TolerantXmlInstrumentation.java @@ -0,0 +1,72 @@ +package datadog.trace.instrumentation.play26.appsec; + +import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.returns; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; +import static net.bytebuddy.matcher.ElementMatchers.takesArguments; + +import akka.util.ByteString; +import com.google.auto.service.AutoService; +import datadog.appsec.api.blocking.BlockingException; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.agent.tooling.muzzle.Reference; +import datadog.trace.instrumentation.play26.MuzzleReferences; +import net.bytebuddy.asm.Advice; +import play.mvc.Http; + +/** @see play.mvc.BodyParser.TolerantXml#parse(Http.RequestHeader, ByteString) */ +@AutoService(Instrumenter.class) +public class TolerantXmlInstrumentation extends Instrumenter.AppSec + implements Instrumenter.ForSingleType { + public TolerantXmlInstrumentation() { + super("play"); + } + + @Override + public String muzzleDirective() { + return "play26Plus"; + } + + @Override + public Reference[] additionalMuzzleReferences() { + return MuzzleReferences.PLAY_26_PLUS; + } + + @Override + public String instrumentedType() { + return "play.mvc.BodyParser$TolerantXml"; + } + + @Override + public String[] helperClassNames() { + return new String[] { + packageName + ".BodyParserHelpers", + }; + } + + @Override + public void adviceTransformations(AdviceTransformation transformation) { + transformation.applyAdvice( + named("parse") + .and(takesArguments(2)) + .and(takesArgument(0, named("play.mvc.Http$RequestHeader"))) + .and(takesArgument(1, named("akka.util.ByteString"))) + .and(returns(named("org.w3c.dom.Document"))), + TolerantXmlInstrumentation.class.getName() + "$ParseAdvice"); + } + + static class ParseAdvice { + @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class) + static void after( + @Advice.Return org.w3c.dom.Document ret, @Advice.Thrown(readOnly = false) Throwable t) { + if (t != null) { + return; + } + try { + BodyParserHelpers.handleXmlDocument(ret, "TolerantXml#parse"); + } catch (BlockingException be) { + t = be; + } + } + } +} diff --git a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy index dff515786ca..97131a0380d 100644 --- a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy +++ b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy @@ -388,6 +388,7 @@ abstract class HttpServerTest extends WithHttpServer { BODY_URLENCODED("body-urlencoded?ignore=pair", 200, '[a:[x]]'), BODY_MULTIPART("body-multipart?ignore=pair", 200, '[a:[x]]'), BODY_JSON("body-json", 200, '{"a":"x"}'), + BODY_XML("body-xml", 200, 'mytext'), REDIRECT("redirect", 302, "/redirected"), FORWARDED("forwarded", 200, "1.2.3.4"), ERROR("error-status", 500, "controller error"), // "error" is a special path for some frameworks @@ -2095,8 +2096,9 @@ abstract class HttpServerTest extends WithHttpServer { [ it.key, (it.value instanceof Iterable || it.value instanceof String[]) ? it.value : [it.value] - ]} - } else if (!(obj instanceof String)) { + ] + } + } else if (!(obj instanceof String) && !(obj instanceof List)) { obj = obj.properties .findAll { it.key != 'class' } .collectEntries { [it.key, it.value instanceof Iterable ? it.value : [it.value]] }