From edfab174ab4cf1fe41c2f73516e67e85bab94545 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Chantepie?= Date: Sun, 25 Feb 2024 22:02:34 +0100 Subject: [PATCH] Close #700 - Safe Json.parse alternatives --- build.sbt | 18 +++++- .../main/scala/play/api/libs/json/Json.scala | 60 +++++++++++++++---- .../play/api/libs/json/JsonSharedSpec.scala | 21 +++++-- 3 files changed, 80 insertions(+), 19 deletions(-) diff --git a/build.sbt b/build.sbt index 941f0ab44..29ccab646 100644 --- a/build.sbt +++ b/build.sbt @@ -45,8 +45,22 @@ val previousVersion: Option[String] = Some("3.0.0") // Do not check for previous JS artifacts for upgrade to Scala.js 1.0 because no sjs1 artifacts exist def playJsonMimaSettings = Seq( mimaPreviousArtifacts := previousVersion.map(organization.value %%% moduleName.value % _).toSet, - mimaBinaryIssueFilters ++= Seq( - ), + mimaBinaryIssueFilters ++= { + val missingMethodInOld: ProblemFilter = { + case ReversedAbstractMethodProblem(_) | ReversedMissingMethodProblem(_) => + false + + case DirectMissingMethodProblem(old) => old.nonAccessible + case InheritedNewAbstractMethodProblem(_, _) => false + case IncompatibleResultTypeProblem(old, _) => old.nonAccessible + case IncompatibleMethTypeProblem(old, _) => old.nonAccessible + case MissingClassProblem(old) => !old.isPublic + case AbstractClassProblem(old) => !old.isPublic + case _ => true + } + + Seq(missingMethodInOld) + } ) val javacSettings = Seq( diff --git a/play-json/shared/src/main/scala/play/api/libs/json/Json.scala b/play-json/shared/src/main/scala/play/api/libs/json/Json.scala index a6425ee85..96c9d78bd 100644 --- a/play-json/shared/src/main/scala/play/api/libs/json/Json.scala +++ b/play-json/shared/src/main/scala/play/api/libs/json/Json.scala @@ -4,33 +4,46 @@ package play.api.libs.json +import java.io.InputStream + import scala.collection.mutable.{ Builder => MBuilder } -import java.io.InputStream +import scala.util.Try /** * @define jsonParam @param json the JsValue to convert - * @define returnStringRepr A String with the json representation + * @define parseDescription Parses the input JSON, and returns it as a [[JsValue]] + * @define stringRepr String with the json representation */ sealed trait JsonFacade { /** - * Parses a String representing a JSON input, and returns it as a [[JsValue]]. + * $parseDescription (use `tryParse` to be safe). * - * @param input the String to parse + * @param input the $stringRepr */ def parse(input: String): JsValue /** - * Parses a stream representing a JSON input, and returns it as a [[JsValue]]. + * $parseDescription. + * @param input the $stringRepr + */ + def tryParse(input: String): Try[JsValue] + + /** + * $parseDescription (use `tryParse` to be safe). * * @param input the InputStream to parse */ def parse(input: InputStream): JsValue /** - * Parses some bytes representing a JSON input, - * and returns it as a [[JsValue]]. + * $parseDescription. + */ + def tryParse(input: InputStream): Try[JsValue] + + /** + * $parseDescription (use `tryParse` to be safe). * * The character encoding used will be automatically detected as UTF-8, * UTF-16 or UTF-32, as per the heuristics in RFC-4627. @@ -39,6 +52,11 @@ sealed trait JsonFacade { */ def parse(input: Array[Byte]): JsValue + /** + * $parseDescription. + */ + def tryParse(input: Array[Byte]): Try[JsValue] + /** * Converts a [[JsValue]] to its string representation. * @@ -88,7 +106,7 @@ sealed trait JsonFacade { * }}} * * $jsonParam - * $returnStringRepr with all non-ascii characters escaped. + * @return A $stringRepr with all non-ascii characters escaped. */ def asciiStringify(json: JsValue): String @@ -118,7 +136,7 @@ sealed trait JsonFacade { * }}} * * $jsonParam - * $returnStringRepr. + * @return A $stringRepr. */ def prettyPrint(json: JsValue): String @@ -174,10 +192,16 @@ sealed trait JsonFacade { object Json extends JsonFacade with JsMacros with JsValueMacros { def parse(input: String): JsValue = StaticBinding.parseJsValue(input) + def tryParse(input: String): Try[JsValue] = Try(StaticBinding.parseJsValue(input)) + def parse(input: InputStream): JsValue = StaticBinding.parseJsValue(input) + def tryParse(input: InputStream): Try[JsValue] = Try(StaticBinding.parseJsValue(input)) + def parse(input: Array[Byte]): JsValue = StaticBinding.parseJsValue(input) + def tryParse(input: Array[Byte]): Try[JsValue] = Try(StaticBinding.parseJsValue(input)) + def stringify(json: JsValue): String = StaticBinding.generateFromJsValue(json, false) @@ -321,10 +345,20 @@ object Json extends JsonFacade with JsMacros with JsValueMacros { def this() = this(JsonConfiguration.default) - @inline def parse(input: String): JsValue = Json.parse(input) - @inline def parse(input: InputStream): JsValue = Json.parse(input) - @inline def parse(input: Array[Byte]): JsValue = Json.parse(input) - @inline def stringify(json: JsValue): String = Json.stringify(json) + @inline def parse(input: String): JsValue = Json.parse(input) + + @inline def tryParse(input: String): Try[JsValue] = Json.tryParse(input) + + @inline def parse(input: InputStream): JsValue = Json.parse(input) + + @inline def tryParse(input: InputStream): Try[JsValue] = Json.tryParse(input) + + @inline def parse(input: Array[Byte]): JsValue = Json.parse(input) + + @inline def tryParse(input: Array[Byte]): Try[JsValue] = Json.tryParse(input) + + @inline def stringify(json: JsValue): String = Json.stringify(json) + @inline def toBytes(json: JsValue): Array[Byte] = Json.toBytes(json) @inline def asciiStringify(json: JsValue): String = diff --git a/play-json/shared/src/test/scala/play/api/libs/json/JsonSharedSpec.scala b/play-json/shared/src/test/scala/play/api/libs/json/JsonSharedSpec.scala index bdcba06aa..939c8bdf2 100644 --- a/play-json/shared/src/test/scala/play/api/libs/json/JsonSharedSpec.scala +++ b/play-json/shared/src/test/scala/play/api/libs/json/JsonSharedSpec.scala @@ -9,10 +9,16 @@ import play.api.libs.functional.syntax._ import scala.collection.immutable.ListMap import org.scalacheck.Gen + import org.scalatest.matchers.must.Matchers import org.scalatest.wordspec.AnyWordSpec -class JsonSharedSpec extends AnyWordSpec with Matchers with org.scalatestplus.scalacheck.ScalaCheckPropertyChecks { +class JsonSharedSpec + extends AnyWordSpec + with Matchers + with org.scalatest.TryValues + with org.scalatestplus.scalacheck.ScalaCheckPropertyChecks { + case class User(id: Long, name: String, friends: List[User]) implicit val UserFormat: Format[User] = ( @@ -159,12 +165,18 @@ class JsonSharedSpec extends AnyWordSpec with Matchers with org.scalatestplus.sc | "price": "2.5 €" |} """.stripMargin) + val bytes = js.toBytes(json) val string = new String(bytes, "UTF-8") - val parsedJson = js.parse(string) + val parsedJson = js.tryParse(string) + + parsedJson.isSuccess.mustEqual(true) - (parsedJson \ "symbol").mustEqual(JsDefined(JsString("☕"))) - (parsedJson \ "price").mustEqual(JsDefined(JsString("2.5 €"))) + val success = parsedJson.success.value + + (success \ "symbol").mustEqual(JsDefined(JsString("☕"))) + + (success \ "price").mustEqual(JsDefined(JsString("2.5 €"))) } } @@ -175,6 +187,7 @@ class JsonSharedSpec extends AnyWordSpec with Matchers with org.scalatestplus.sc val jsonM = js.toJson(m) (jsonM \ "timestamp").as[Long].mustEqual(t) + jsonM.toString.mustEqual("""{"timestamp":1330950829160}""") }