From 9132c8da30db9ca799511851d1ca0704656d2ff0 Mon Sep 17 00:00:00 2001 From: colin-lamed <9568290+colin-lamed@users.noreply.github.com> Date: Wed, 30 Mar 2022 17:25:14 +0100 Subject: [PATCH] BDOG-1512 Move truncation log out of http-verbs into play-auditing (to respect auditing enabled) --- .../hmrc/http/client/HttpClientV2Impl.scala | 8 ++------ .../uk/gov/hmrc/http/hooks/HttpHook.scala | 2 -- .../uk/gov/hmrc/play/http/BodyCaptor.scala | 20 +++++-------------- 3 files changed, 7 insertions(+), 23 deletions(-) diff --git a/http-verbs-common/src/main/scala/uk/gov/hmrc/http/client/HttpClientV2Impl.scala b/http-verbs-common/src/main/scala/uk/gov/hmrc/http/client/HttpClientV2Impl.scala index 3ebb4c63..cf8bb327 100644 --- a/http-verbs-common/src/main/scala/uk/gov/hmrc/http/client/HttpClientV2Impl.scala +++ b/http-verbs-common/src/main/scala/uk/gov/hmrc/http/client/HttpClientV2Impl.scala @@ -136,7 +136,6 @@ final class RequestBuilderImpl( override def withBody[B : BodyWritable : TypeTag](body: B): RequestBuilderImpl = { val hookDataP = Promise[Body[Option[HookData]]]() val maxBodyLength = config.get[Int]("http-verbs.auditing.maxBodyLength") - val loggingContext = s"outgoing ${request.method} ${request.url} request" transform { req => val req2 = req.withBody(body) req2.body match { @@ -148,7 +147,7 @@ final class RequestBuilderImpl( case (IsMap(m), _ ) => hookDataP.success(Body.Complete(Some(HookData.FromMap(m)))) case (_ , Some("application/x-www-form-urlencoded")) => hookDataP.success(Body.Complete(Some(HookData.FromMap(FormUrlEncodedParser.parse(bytes.utf8String))))) case _ => val body = - BodyCaptor.bodyUpto(bytes, maxBodyLength, loggingContext, isStream = false) + BodyCaptor.bodyUpto(bytes, maxBodyLength, isStream = false) hookDataP.success(body.map(bytes => Some(HookData.FromString(bytes.utf8String)))) } req2 @@ -156,7 +155,6 @@ final class RequestBuilderImpl( source .alsoTo( BodyCaptor.sink( - loggingContext = loggingContext, maxBodyLength = maxBodyLength, withCapturedBody = body => hookDataP.success(body.map(bytes => Some(HookData.FromString(bytes.utf8String)))) ) @@ -235,7 +233,6 @@ class ExecutorImpl( )(implicit ec: ExecutionContext ): (Future[HttpResponse], Future[ResponseData]) = { val auditResponseF = Promise[ResponseData]() - val loggingContext = s"outgoing ${request.method} ${request.url} response" // play returns scala.collection, but default for Scala 2.13 is scala.collection.immutable def forScala2_13(m: scala.collection.Map[String, scala.collection.Seq[String]]): Map[String, Seq[String]] = @@ -252,7 +249,6 @@ class ExecutorImpl( response.bodyAsSource .alsoTo( BodyCaptor.sink( - loggingContext = loggingContext, maxBodyLength = maxBodyLength, withCapturedBody = body => auditResponseF.success(ResponseData( body = body.map(_.utf8String), @@ -272,7 +268,7 @@ class ExecutorImpl( } else { auditResponseF.success(ResponseData( body = BodyCaptor - .bodyUpto(ByteString(response.body), maxBodyLength, loggingContext, isStream = false) + .bodyUpto(ByteString(response.body), maxBodyLength, isStream = false) .map(_.utf8String), status = response.status, headers = headers diff --git a/http-verbs-common/src/main/scala/uk/gov/hmrc/http/hooks/HttpHook.scala b/http-verbs-common/src/main/scala/uk/gov/hmrc/http/hooks/HttpHook.scala index 5d4ec3e4..86e70ef0 100644 --- a/http-verbs-common/src/main/scala/uk/gov/hmrc/http/hooks/HttpHook.scala +++ b/http-verbs-common/src/main/scala/uk/gov/hmrc/http/hooks/HttpHook.scala @@ -22,8 +22,6 @@ import uk.gov.hmrc.http.{HeaderCarrier, HttpResponse} import scala.concurrent.{ExecutionContext, Future} -// TODO requires major version change of http-verbs/play-auditing/bootstrap-play - trait HttpHook { def apply( verb : String, diff --git a/http-verbs-common/src/main/scala/uk/gov/hmrc/play/http/BodyCaptor.scala b/http-verbs-common/src/main/scala/uk/gov/hmrc/play/http/BodyCaptor.scala index 57d8d4f8..c1f1b277 100644 --- a/http-verbs-common/src/main/scala/uk/gov/hmrc/play/http/BodyCaptor.scala +++ b/http-verbs-common/src/main/scala/uk/gov/hmrc/play/http/BodyCaptor.scala @@ -20,13 +20,11 @@ import akka.stream.{Attributes, FlowShape, Inlet, Outlet} import akka.stream.scaladsl.{Flow, Sink} import akka.stream.stage._ import akka.util.ByteString -import org.slf4j.LoggerFactory import uk.gov.hmrc.http.hooks.Body // based on play.filters.csrf.CSRFAction#BodyHandler private class BodyCaptorFlow( - loggingContext : String, maxBodyLength : Int, withCapturedBody: Body[ByteString] => Unit ) extends GraphStage[FlowShape[ByteString, ByteString]] { @@ -53,7 +51,7 @@ private class BodyCaptorFlow( } override def onUpstreamFinish(): Unit = { - withCapturedBody(BodyCaptor.bodyUpto(buffer, maxBodyLength, loggingContext, isStream = true)) + withCapturedBody(BodyCaptor.bodyUpto(buffer, maxBodyLength, isStream = true)) completeStage() } } @@ -62,33 +60,25 @@ private class BodyCaptorFlow( } object BodyCaptor { - private val logger = LoggerFactory.getLogger(getClass) - def flow( - loggingContext : String, maxBodyLength : Int, withCapturedBody: Body[ByteString] => Unit // provide a callback since a Materialized value would be not be available until the flow has been run ): Flow[ByteString, ByteString, akka.NotUsed] = Flow.fromGraph(new BodyCaptorFlow( - loggingContext = loggingContext, maxBodyLength = maxBodyLength, withCapturedBody = withCapturedBody )) def sink( - loggingContext : String, maxBodyLength : Int, withCapturedBody: Body[ByteString] => Unit ): Sink[ByteString, akka.NotUsed] = - flow(loggingContext, maxBodyLength, withCapturedBody) + flow(maxBodyLength, withCapturedBody) .to(Sink.ignore) - def bodyUpto(body: ByteString, maxBodyLength: Int, loggingContext: String, isStream: Boolean): Body[ByteString] = - if (body.length > maxBodyLength) { - logger.warn( - s"$loggingContext ${if (isStream) "streamed body" else "body " + body.length} exceeds maxLength $maxBodyLength - truncating for audit" - ) + def bodyUpto(body: ByteString, maxBodyLength: Int, isStream: Boolean): Body[ByteString] = + if (body.length > maxBodyLength) Body.Truncated(body.take(maxBodyLength)) - } else + else Body.Complete(body) }