Json validator hardening

If the json validator encounters a schema which exists, but has the wrong meta-schema, the current implementation will crash hard. This pull request fixes that issue by wrapping all calls to the underlying library in Either.catchNonFatal
snowplow · Apr 21, 2023 · f2b3195 · f2b3195
1 parent c4864bb
commit f2b3195
Show file tree

Hide file tree

Showing 2 changed files with 68 additions and 22 deletions.
diff --git a/modules/core/src/main/scala/com.snowplowanalytics.iglu/client/validator/CirceValidator.scala b/modules/core/src/main/scala/com.snowplowanalytics.iglu/client/validator/CirceValidator.scala
@@ -92,16 +92,28 @@ object CirceValidator extends Validator[Json] {
     schema: JsonSchema,
     instance: Json
   ): EitherNel[ValidatorReport, Unit] = {
-    val messages = schema
-      .validate(circeToJackson(instance))
-      .asScala
-      .toList
-      .map(fromValidationMessage)
-
-    messages match {
-      case x :: xs => NonEmptyList(x, xs).asLeft
-      case Nil     => ().asRight
-    }
+    val messages = Either
+      .catchNonFatal(
+        schema
+          .validate(circeToJackson(instance))
+          .asScala
+          .toList
+          .map(fromValidationMessage)
+      )
+      .leftMap(ValidatorError.schemaIssue)
+
+    messages.fold(
+      err =>
+        Left(
+          NonEmptyList.one(
+            ValidatorReport(s"Invalid schema ${err.issues.toList.mkString(",")}", None, Nil, None)
+          )
+        ),
+      {
+        case x :: xs => NonEmptyList(x, xs).asLeft
+        case Nil     => ().asRight
+      }
+    )
   }
 
   private def fromValidationMessage(m: ValidationMessage): ValidatorReport =
@@ -119,11 +131,16 @@ object CirceValidator extends Validator[Json] {
   }
 
   private def validateSchemaAgainstV4(schema: JsonNode): List[ValidatorError.SchemaIssue] = {
-    V4Schema
-      .validate(schema)
-      .asScala
-      .toList
-      .map(m => ValidatorError.SchemaIssue(m.getPath, m.getMessage))
+    Either
+      .catchNonFatal(
+        V4Schema
+          .validate(schema)
+          .asScala
+          .toList
+          .map(m => ValidatorError.SchemaIssue(m.getPath, m.getMessage))
+      )
+      .leftMap(ValidatorError.schemaIssue)
+      .fold(_.issues.toList, identity)
   }
 
   private[client] object WithCaching {
@@ -180,13 +197,17 @@ object CirceValidator extends Validator[Json] {
     }
 
     private def validateSchema(schema: JsonNode): Either[ValidatorError.InvalidSchema, Unit] = {
-      val issues = V4Schema
-        .validate(schema)
-        .asScala
-        .toList
-        .map(m => ValidatorError.SchemaIssue(m.getPath, m.getMessage))
-
-      issues match {
+      val issues = Either
+        .catchNonFatal(
+          V4Schema
+            .validate(schema)
+            .asScala
+            .toList
+            .map(m => ValidatorError.SchemaIssue(m.getPath, m.getMessage))
+        )
+        .leftMap(ValidatorError.schemaIssue)
+
+      issues.flatMap {
         case Nil          => Right(())
         case head :: tail => Left(ValidatorError.InvalidSchema(NonEmptyList(head, tail)))
       }

diff --git a/...ore/src/test/scala/com.snowplowanalytics.iglu.client/validator/SchemaValidationSpec.scala b/...ore/src/test/scala/com.snowplowanalytics.iglu.client/validator/SchemaValidationSpec.scala
@@ -13,7 +13,10 @@
 package com.snowplowanalytics.iglu.client.validator
 
 // circe
+import cats.effect.IO
 import cats.effect.testing.specs2.CatsEffect
+import com.snowplowanalytics.iglu.client.Client
+import com.snowplowanalytics.iglu.client.resolver.Resolver
 import io.circe.literal._
 
 // Specs2
@@ -28,6 +31,7 @@ class SchemaValidationSpec extends Specification with CatsEffect {
 
   validating a correct self-desc JSON should return the JSON in a Success $e1
   validating a correct self-desc JSON with JSON Schema with incorrect $$schema property should return Failure $e2
+  validating a unexpected self-desc JSON with valid JSON Schema should return Failure $e3
   """
 
   val validJson =
@@ -52,6 +56,17 @@ class SchemaValidationSpec extends Specification with CatsEffect {
       json"""{"schema": "iglu://jsonschema/1-0-0", "data": { "id": 0 } }"""
     )
 
+  val validJsonWithSchemaWithInvalidMetaSchema =
+    SelfDescribingData(
+      SchemaKey(
+        "com.snowplowanalytics.self-desc",
+        "schema",
+        "jsonschema",
+        SchemaVer.Full(1, 0, 0)
+      ),
+      json"""{"schema": "http://iglucentral.com/schemas/com.snowplowanalytics.self-desc/schema/jsonschema/1-0-0#", "data": { "id": 0 } }"""
+    )
+
   val testResolver = SpecHelpers.TestResolver
 
   def e1 = {
@@ -67,4 +82,14 @@ class SchemaValidationSpec extends Specification with CatsEffect {
       result <- client.check(validJsonWithInvalidSchema).value
     } yield result must beLeft
   }
+
+  def e3 = {
+    // todo: Improve this
+    val TestResolver = Resolver.init[IO](10, None, SpecHelpers.IgluCentral)
+    val TestClient   = for { resolver <- TestResolver } yield Client(resolver, CirceValidator)
+    for {
+      client <- TestClient
+      result <- client.check(validJsonWithSchemaWithInvalidMetaSchema).value
+    } yield result must beLeft
+  }
 }