diff --git a/CHANGES.md b/CHANGES.md index e4249d78b1..eb4160e3f0 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -6,6 +6,10 @@ - Allow supply extra K8s manifests to deploy via `magda-core` `.Values.extraObjects` - Stop releasing helm charts to charts.magda.io. Since v2, we started to release charts to Github container OCI registry +## v3.0.3 + +- #3193 auto-remove Null byte from JSON input so it won't be rejected by registry + ## v3.0.2 - #3513 Fixed Indexer reindex trigger job doesn't start diff --git a/magda-registry-api/src/main/scala/au/csiro/data61/magda/registry/AspectsService.scala b/magda-registry-api/src/main/scala/au/csiro/data61/magda/registry/AspectsService.scala index bdc5027305..cc22291b41 100644 --- a/magda-registry-api/src/main/scala/au/csiro/data61/magda/registry/AspectsService.scala +++ b/magda-registry-api/src/main/scala/au/csiro/data61/magda/registry/AspectsService.scala @@ -23,8 +23,10 @@ import scalikejdbc._ import spray.json.JsObject import scala.util.{Failure, Success} - -import au.csiro.data61.magda.directives.CommonDirectives.onCompleteBlockingTask +import au.csiro.data61.magda.directives.CommonDirectives.{ + onCompleteBlockingTask, + sanitizedJsonEntity +} class AspectsService( config: Config, @@ -101,7 +103,7 @@ class AspectsService( pathEnd { requireUserId { userId => requiresSpecifiedTenantId { tenantId => - entity(as[AspectDefinition]) { aspect => + sanitizedJsonEntity(as[AspectDefinition]) { aspect => requirePermission( authClient, "object/aspect/create", @@ -223,7 +225,7 @@ class AspectsService( { requireUserId { userId => requiresSpecifiedTenantId { tenantId => - entity(as[AspectDefinition]) { aspect => + sanitizedJsonEntity(as[AspectDefinition]) { aspect => requireAspectUpdateOrCreateWhenNonExistPermission( authClient, id, @@ -345,7 +347,7 @@ class AspectsService( path(Segment) { id: String => requireUserId { userId => requiresSpecifiedTenantId { tenantId => - entity(as[JsonPatch]) { aspectPatch => + sanitizedJsonEntity(as[JsonPatch]) { aspectPatch => requireAspectUpdateOrCreateWhenNonExistPermission( authClient, id, diff --git a/magda-registry-api/src/main/scala/au/csiro/data61/magda/registry/RecordAspectsService.scala b/magda-registry-api/src/main/scala/au/csiro/data61/magda/registry/RecordAspectsService.scala index 76b3ee7077..7a0fc7f893 100644 --- a/magda-registry-api/src/main/scala/au/csiro/data61/magda/registry/RecordAspectsService.scala +++ b/magda-registry-api/src/main/scala/au/csiro/data61/magda/registry/RecordAspectsService.scala @@ -6,14 +6,12 @@ import akka.http.scaladsl.model.headers.RawHeader import akka.http.scaladsl.server.Directives._ import akka.http.scaladsl.server.Route import akka.stream.Materializer -import au.csiro.data61.magda.directives.AuthDirectives.{requireUserId} -import au.csiro.data61.magda.directives.TenantDirectives.{ - requiresSpecifiedTenantId -} +import au.csiro.data61.magda.directives.AuthDirectives.requireUserId +import au.csiro.data61.magda.directives.TenantDirectives.requiresSpecifiedTenantId import au.csiro.data61.magda.model.Registry._ import au.csiro.data61.magda.registry.Directives.{ - requireRecordAspectUpdatePermission, - requireDeleteRecordAspectPermission + requireDeleteRecordAspectPermission, + requireRecordAspectUpdatePermission } import com.typesafe.config.Config import gnieh.diffson.sprayJson._ @@ -24,8 +22,10 @@ import scalikejdbc.DB import spray.json.JsObject import scala.util.{Failure, Success} - -import au.csiro.data61.magda.directives.CommonDirectives.onCompleteBlockingTask +import au.csiro.data61.magda.directives.CommonDirectives.{ + onCompleteBlockingTask, + sanitizedJsonEntity +} @Path("/records/{recordId}/aspects") @io.swagger.annotations.Api( @@ -154,7 +154,7 @@ class RecordAspectsService( (recordId: String, aspectId: String) => requireUserId { userId => requiresSpecifiedTenantId { tenantId => - entity(as[JsObject]) { aspect => + sanitizedJsonEntity(as[JsObject]) { aspect => parameters( 'merge.as[Boolean].? ) { merge => @@ -392,7 +392,7 @@ class RecordAspectsService( (recordId: String, aspectId: String) => requireUserId { userId => requiresSpecifiedTenantId { tenantId => - entity(as[JsonPatch]) { aspectPatch => + sanitizedJsonEntity(as[JsonPatch]) { aspectPatch => requireRecordAspectUpdatePermission( authClient, recordId, diff --git a/magda-registry-api/src/main/scala/au/csiro/data61/magda/registry/RecordsService.scala b/magda-registry-api/src/main/scala/au/csiro/data61/magda/registry/RecordsService.scala index 67594bcdd6..8754072324 100644 --- a/magda-registry-api/src/main/scala/au/csiro/data61/magda/registry/RecordsService.scala +++ b/magda-registry-api/src/main/scala/au/csiro/data61/magda/registry/RecordsService.scala @@ -35,10 +35,10 @@ import spray.json.JsObject import scala.concurrent.{Await, ExecutionContext, Future} import scala.concurrent.duration._ import scala.util.{Failure, Success} - import au.csiro.data61.magda.directives.CommonDirectives.{ onCompleteBlockingTask, - onCompleteBlockingTaskIn + onCompleteBlockingTaskIn, + sanitizedJsonEntity } @Path("/records") @@ -418,7 +418,7 @@ class RecordsService( path(Segment) { id: String => requireUserId { userId => requiresSpecifiedTenantId { tenantId => - entity(as[Record]) { recordIn => + sanitizedJsonEntity(as[Record]) { recordIn => parameters( 'merge.as[Boolean].? ) { merge => @@ -577,7 +577,7 @@ class RecordsService( path(Segment) { id: String => requireUserId { userId => requiresSpecifiedTenantId { tenantId => - entity(as[JsonPatch]) { recordPatch => + sanitizedJsonEntity(as[JsonPatch]) { recordPatch => requireRecordUpdateOrCreateWhenNonExistPermission( authClient, id, @@ -707,7 +707,7 @@ class RecordsService( def patchRecords: Route = patch { requireUserId { userId => requiresSpecifiedTenantId { tenantId => - entity(as[PatchRecordsRequest]) { requestData => + sanitizedJsonEntity(as[PatchRecordsRequest]) { requestData => withAuthDecision( authClient, AuthDecisionReqConfig("object/record/update") @@ -831,7 +831,7 @@ class RecordsService( path("aspects" / Segment) { (aspectId: String) => requireUserId { userId => requiresSpecifiedTenantId { tenantId => - entity(as[PutRecordsAspectRequest]) { requestData => + sanitizedJsonEntity(as[PutRecordsAspectRequest]) { requestData => parameters( 'merge.as[Boolean].? ) { merge => @@ -1080,7 +1080,7 @@ class RecordsService( requireUserId { userId => requiresSpecifiedTenantId { tenantId => pathEnd { - entity(as[Record]) { record => + sanitizedJsonEntity(as[Record]) { record => requirePermission( authClient, "object/record/create", diff --git a/magda-registry-api/src/test/scala/au/csiro/data61/magda/registry/RecordsServiceSpec.scala b/magda-registry-api/src/test/scala/au/csiro/data61/magda/registry/RecordsServiceSpec.scala index 5ca0150101..2de062ad63 100644 --- a/magda-registry-api/src/test/scala/au/csiro/data61/magda/registry/RecordsServiceSpec.scala +++ b/magda-registry-api/src/test/scala/au/csiro/data61/magda/registry/RecordsServiceSpec.scala @@ -2,7 +2,8 @@ package au.csiro.data61.magda.registry import java.net.URLEncoder import akka.event.LoggingAdapter -import akka.http.scaladsl.model.StatusCodes +import akka.http.javadsl.model.RequestEntity +import akka.http.scaladsl.model.{ContentTypes, HttpEntity, StatusCodes} import au.csiro.data61.magda.model.Registry._ import au.csiro.data61.magda.model.TenantId._ import gnieh.diffson._ @@ -3913,6 +3914,116 @@ class RecordsServiceSpec extends ApiSpec { } } + it("can add a new record with Null Byte with no error") { param => + val aspectDefinition = AspectDefinition("test", "test", None) + Post("/v0/aspects", aspectDefinition) ~> addUserId() ~> addTenantIdHeader( + TENANT_1 + ) ~> param.api(role).routes ~> check { + status shouldEqual StatusCodes.OK + } + var eventId: Option[String] = None + val record = + Record( + "testId", + "testName", + Map( + "test" -> JsObject( + // null bytes should be removed from field values + "a" -> JsString("abc\u0000edf"), + // null bytes should be removed from field name as well + "b\u0000c" -> JsString("bc\u0000123") + ) + ), + Some("tag") + ) + val expectedRecord = record.copy( + tenantId = Some(TENANT_1), + aspects = Map( + "test" -> JsObject( + "a" -> JsString("abcedf"), + "bc" -> JsString("bc123") + ) + ) + ) + val reqBody = record.toJson.toString() + val reqEntity = HttpEntity(ContentTypes.`application/json`, reqBody) + Post("/v0/records", reqEntity) ~> addUserId() ~> addTenantIdHeader( + TENANT_1 + ) ~> param.api(role).routes ~> check { + status shouldEqual StatusCodes.OK + val r = responseAs[Record] + r shouldEqual expectedRecord + r.aspects + .get("test") + .flatMap(x => x.fields.get("a").map(_.convertTo[String])) + .get shouldBe "abcedf" + r.aspects + .get("test") + .flatMap(x => x.fields.get("bc").map(_.convertTo[String])) + .get shouldBe "bc123" + // --- try to retrieve eventId generated because of this request + eventId = header("x-magda-event-id").map(_.value()) + } + + Get("/v0/records/testId?aspect=test") ~> addTenantIdHeader(TENANT_1) ~> param + .api(role) + .routes ~> check { + status shouldEqual StatusCodes.OK + val r = responseAs[Record] + r shouldEqual expectedRecord + r.aspects + .get("test") + .flatMap(x => x.fields.get("a").map(_.convertTo[String])) + .get shouldBe "abcedf" + r.aspects + .get("test") + .flatMap(x => x.fields.get("bc").map(_.convertTo[String])) + .get shouldBe "bc123" + } + + Get(s"/v0/records/${record.id}/history") ~> addTenantIdHeader(TENANT_1) ~> param + .api(role) + .routes ~> check { + status shouldEqual StatusCodes.OK + + val eventsPage = responseAs[EventsPage] + eventsPage.events.length shouldEqual 2 + eventsPage.events(0).userId shouldEqual Some(USER_ID) + eventsPage.events(0).eventType shouldEqual EventType.CreateRecord + eventsPage + .events(0) + .data + .fields("recordId") + .convertTo[String] shouldEqual record.id + eventsPage.events(1).userId shouldEqual Some(USER_ID) + eventsPage + .events(1) + .eventType shouldEqual EventType.CreateRecordAspect + eventsPage + .events(1) + .data + .fields("aspect") + .asJsObject + .fields("a") + .convertTo[String] shouldEqual "abcedf" + eventsPage + .events(1) + .data + .fields("aspect") + .asJsObject + .fields("bc") + .convertTo[String] shouldEqual "bc123" + // --- check if the event id match the event id in previous POST response header + eventId.get shouldEqual eventsPage.events(1).id.get.toString + } + + Get("/v0/records/testId") ~> addTenantIdHeader(TENANT_2) ~> param + .api(role) + .routes ~> check { + status shouldEqual StatusCodes.NotFound + } + } + it( "can add two new records with the same record IDs by different tenants" ) { implicit param => diff --git a/magda-scala-common/src/main/scala/au/csiro/data61/magda/directives/CommonDirectives.scala b/magda-scala-common/src/main/scala/au/csiro/data61/magda/directives/CommonDirectives.scala index d7df8637e6..5a8cc6eabb 100644 --- a/magda-scala-common/src/main/scala/au/csiro/data61/magda/directives/CommonDirectives.scala +++ b/magda-scala-common/src/main/scala/au/csiro/data61/magda/directives/CommonDirectives.scala @@ -2,7 +2,22 @@ package au.csiro.data61.magda.directives import akka.http.scaladsl.server.Directives._ import akka.http.scaladsl.server.{Directive0, _} + import scala.concurrent.Future +import akka.http.scaladsl.model.{ + ContentTypes, + ExceptionWithErrorInfo, + HttpEntity +} +import akka.http.scaladsl.unmarshalling.Unmarshaller.UnsupportedContentTypeException +import akka.http.scaladsl.unmarshalling.{ + FromRequestUnmarshaller, + Unmarshaller, + Unmarshal +} +import scala.util.{Failure, Success} +import akka.http.scaladsl.server.directives.RouteDirectives.reject +import au.csiro.data61.magda.util.StringUtils.ExtraStringHelperFunctions object CommonDirectives { @@ -21,4 +36,57 @@ object CommonDirectives { } } + /** + * Sanitize Json input to remove any Null bytes in Json string as postgreSQL won't accept it + * + * Example: sanitizedJsonEntity(as[Record]) + * + * @param um Unmarshaller + * @tparam T + * @return + */ + def sanitizedJsonEntity[T](um: FromRequestUnmarshaller[T]): Directive1[T] = + extractRequestContext.flatMap[Tuple1[T]] { ctx => + import ctx.executionContext + import ctx.materializer + + val unmarshalled: Future[T] = Unmarshal(ctx.request).to[String].flatMap { + jsonString => + val ent = HttpEntity( + ContentTypes.`application/json`, + jsonString.removeNullByteFromJsonString + ) + um(ctx.request.mapEntity(_ => ent)) + } + + onComplete(unmarshalled).flatMap { + case Success(value) => + provide(value) + case Failure(RejectionError(r)) => + reject(r) + case Failure(Unmarshaller.NoContentException) => + reject(RequestEntityExpectedRejection) + case Failure(x: UnsupportedContentTypeException) => + reject( + UnsupportedRequestContentTypeRejection( + x.supported, + x.actualContentType + ) + ) + case Failure(x: IllegalArgumentException) => + reject(ValidationRejection(x.getMessage.nullAsEmpty, Some(x))) + case Failure(x: ExceptionWithErrorInfo) => + reject( + MalformedRequestContentRejection( + x.info.format(ctx.settings.verboseErrorMessages), + x + ) + ) + case Failure(x) => + reject(MalformedRequestContentRejection(x.getMessage.nullAsEmpty, x)) + } + } & cancelRejections( + RequestEntityExpectedRejection.getClass, + classOf[UnsupportedRequestContentTypeRejection] + ) } diff --git a/magda-scala-common/src/main/scala/au/csiro/data61/magda/util/StringUtils.scala b/magda-scala-common/src/main/scala/au/csiro/data61/magda/util/StringUtils.scala index fda64b556d..ca6245ed39 100644 --- a/magda-scala-common/src/main/scala/au/csiro/data61/magda/util/StringUtils.scala +++ b/magda-scala-common/src/main/scala/au/csiro/data61/magda/util/StringUtils.scala @@ -12,5 +12,16 @@ object StringUtils { def toUrlSegment = urlSegmentEncoder.encode(s, "utf-8") def toQueryStringVal = urlQsValEncoder.encode(s, "utf-8") + + def removeNullByte: String = { + s.replace("\u0000", "") + } + + def removeNullByteFromJsonString: String = { + s.replace("\u0000", "").replace("\\u0000", "") + } + + def nullAsEmpty: String = + if (s eq null) "" else s } }