From 011694cf0e9832854c12fd0269c912485225ee30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20Jourdan-Weil?= Date: Wed, 31 Dec 2025 15:55:10 +0100 Subject: [PATCH 1/4] test: safer assertions by checking the Jackson node class and not only the string representation --- .../scala/play/api/libs/json/JsonSpec.scala | 36 +++++++++++++------ 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/play-json/jvm/src/test/scala/play/api/libs/json/JsonSpec.scala b/play-json/jvm/src/test/scala/play/api/libs/json/JsonSpec.scala index d554dbcd..506a0256 100644 --- a/play-json/jvm/src/test/scala/play/api/libs/json/JsonSpec.scala +++ b/play-json/jvm/src/test/scala/play/api/libs/json/JsonSpec.scala @@ -14,6 +14,7 @@ import java.util.TimeZone import com.fasterxml.jackson.databind.JsonNode import com.fasterxml.jackson.databind.ObjectMapper +import com.fasterxml.jackson.databind.node.{ ArrayNode, NumericNode, ObjectNode } import play.api.libs.functional.syntax._ import play.api.libs.json.Json._ import play.api.libs.json.jackson.JacksonJson @@ -481,36 +482,51 @@ class JsonSpec extends org.specs2.mutable.Specification { } "Serialize and deserialize Jackson ObjectNodes" in { - val on = mapper + val on: ObjectNode = mapper .createObjectNode() .put("foo", 1) .put("bar", "two") - val json = Json.obj("foo" -> 1, "bar" -> "two") + val json = Json.obj("foo" -> 1, "bar" -> "two") + val deserialized: JsResult[JsonNode] = fromJson[JsonNode](json) toJson(on).must_==(json) and ( - fromJson[JsonNode](json).map(_.toString).must_==(JsSuccess(on.toString)) + deserialized.map(_.isInstanceOf[ObjectNode]).must_==(JsSuccess(true)) + ) and ( + deserialized.map(_.toString).must_==(JsSuccess(on.toString)) ) } "Serialize and deserialize Jackson ArrayNodes" in { - val an = mapper + val an: ArrayNode = mapper .createArrayNode() .add("one") .add(2) - val json = Json.arr("one", 2) + val json = Json.arr("one", 2) + val deserialized: JsResult[JsonNode] = fromJson[JsonNode](json) + toJson(an).must(equalTo(json)) and ( - fromJson[JsonNode](json).map(_.toString).must_==(JsSuccess(an.toString)) + deserialized.map(_.isInstanceOf[ArrayNode]).must_==(JsSuccess(true)) + ) and ( + deserialized.map(_.toString).must_==(JsSuccess(an.toString)) ) } "Deserialize integer JsNumber as Jackson number node" in { - val jsNum = JsNumber(new java.math.BigDecimal("50")) - fromJson[JsonNode](jsNum).map(_.toString).must_==(JsSuccess("50")) + val jsNum = JsNumber(new java.math.BigDecimal("50")) + val deserialized: JsResult[JsonNode] = fromJson[JsonNode](jsNum) + + deserialized.map(_.isInstanceOf[NumericNode]).must_==(JsSuccess(true)) and ( + deserialized.map(_.toString).must_==(JsSuccess("50")) + ) } "Deserialize float JsNumber as Jackson number node" in { - val jsNum = JsNumber(new java.math.BigDecimal("12.345")) - fromJson[JsonNode](jsNum).map(_.toString).must_==(JsSuccess("12.345")) + val jsNum = JsNumber(new java.math.BigDecimal("12.345")) + val deserialized: JsResult[JsonNode] = fromJson[JsonNode](jsNum) + + deserialized.map(_.isInstanceOf[NumericNode]).must_==(JsSuccess(true)) and ( + deserialized.map(_.toString).must_==(JsSuccess("12.345")) + ) } "Serialize JsNumbers with integers correctly" in { From 769936d0241efacba87f65d5f100ff996faebd69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20Jourdan-Weil?= Date: Sat, 3 Jan 2026 18:36:34 +0100 Subject: [PATCH 2/4] refactor: remove intermediate JsonGenerator and StringWriter for prettyPrint and stringify methods --- .../api/libs/json/jackson/JacksonJson.scala | 121 +++++++----------- 1 file changed, 48 insertions(+), 73 deletions(-) diff --git a/play-json/jvm/src/main/scala/play/api/libs/json/jackson/JacksonJson.scala b/play-json/jvm/src/main/scala/play/api/libs/json/jackson/JacksonJson.scala index 2cad502d..f6156212 100644 --- a/play-json/jvm/src/main/scala/play/api/libs/json/jackson/JacksonJson.scala +++ b/play-json/jvm/src/main/scala/play/api/libs/json/jackson/JacksonJson.scala @@ -6,7 +6,6 @@ package play.api.libs.json.jackson import java.io.InputStream import java.io.OutputStream -import java.io.StringWriter import scala.annotation.switch import scala.annotation.tailrec @@ -20,7 +19,6 @@ import com.fasterxml.jackson.core.JsonParser import com.fasterxml.jackson.core.JsonTokenId import com.fasterxml.jackson.core.Version import com.fasterxml.jackson.core.json.JsonWriteFeature -import com.fasterxml.jackson.core.util.DefaultPrettyPrinter import com.fasterxml.jackson.databind.Module.SetupContext import com.fasterxml.jackson.databind._ @@ -283,43 +281,56 @@ private[play] object JacksonJson { } private[play] case class JacksonJson(defaultMapperJsonConfig: JsonConfig) { - private var currentMapper: ObjectMapper = null - private val defaultMapper: ObjectMapper = JsonMapper - .builder( - new JsonFactoryBuilder() - .streamReadConstraints(defaultMapperJsonConfig.streamReadConstraints) - .streamWriteConstraints(defaultMapperJsonConfig.streamWriteConstraints) - .build() - ) - .addModules( - new ParameterNamesModule(), - new Jdk8Module(), - new JavaTimeModule(), - new DefaultScalaModule(), - new PlayJsonMapperModule(defaultMapperJsonConfig), - ) - .disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES) - .disable(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS) - .disable(SerializationFeature.WRITE_DURATIONS_AS_TIMESTAMPS) - .disable(SerializationFeature.FAIL_ON_EMPTY_BEANS) - .build() - - private[play] def mapper(): ObjectMapper = if (currentMapper == null) { - defaultMapper - } else { - currentMapper + private var currentMapper: ObjectMapper = null + private var currentMapperWithEscapeNonAscii: ObjectMapper = null + + private val defaultMapper: ObjectMapper = buildMapper(false) + private val defaultMapperWithEscapeNonAscii: ObjectMapper = buildMapper(true) + + private def buildMapper(escapeNonAscii: Boolean): ObjectMapper = { + JsonMapper + .builder( + new JsonFactoryBuilder() + .streamReadConstraints(defaultMapperJsonConfig.streamReadConstraints) + .streamWriteConstraints(defaultMapperJsonConfig.streamWriteConstraints) + .build() + ) + .addModules( + new ParameterNamesModule(), + new Jdk8Module(), + new JavaTimeModule(), + new DefaultScalaModule(), + new PlayJsonMapperModule(defaultMapperJsonConfig), + ) + .disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES) + .disable(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS) + .disable(SerializationFeature.WRITE_DURATIONS_AS_TIMESTAMPS) + .disable(SerializationFeature.FAIL_ON_EMPTY_BEANS) + .configure(JsonWriteFeature.ESCAPE_NON_ASCII, escapeNonAscii) + .build() + } + + private[play] def mapper(escapeNonAscii: Boolean = false): ObjectMapper = { + if (escapeNonAscii) { + if (currentMapperWithEscapeNonAscii == null) { + defaultMapperWithEscapeNonAscii + } else { + currentMapperWithEscapeNonAscii + } + } else { + if (currentMapper == null) { + defaultMapper + } else { + currentMapper + } + } } private[play] def setObjectMapper(mapper: ObjectMapper): Unit = { this.currentMapper = mapper + this.currentMapperWithEscapeNonAscii = mapper.copy().enable(JsonWriteFeature.ESCAPE_NON_ASCII.mappedFeature) } - private def stringJsonGenerator(out: StringWriter) = - mapper().getFactory.createGenerator(out) - - private def stringJsonGenerator(out: OutputStream) = - mapper().getFactory.createGenerator(out) - def parseJsValue(data: Array[Byte]): JsValue = mapper().readValue(data, classOf[JsValue]) @@ -329,53 +340,17 @@ private[play] case class JacksonJson(defaultMapperJsonConfig: JsonConfig) { def parseJsValue(stream: InputStream): JsValue = mapper().readValue(stream, classOf[JsValue]) - private def withStringWriter[T](f: StringWriter => T): T = { - val sw = new StringWriter() - - try { - f(sw) - } catch { - case err: Throwable => throw err - } finally { - if (sw != null) try { - sw.close() - } catch { - case _: Throwable => () - } - } - } - def generateFromJsValue(jsValue: JsValue, escapeNonASCII: Boolean): String = - withStringWriter { sw => - val gen = stringJsonGenerator(sw) - - if (escapeNonASCII) { - gen.enable(JsonWriteFeature.ESCAPE_NON_ASCII.mappedFeature) - } + mapper(escapeNonASCII).writeValueAsString(jsValue) - mapper().writeValue(gen, jsValue) - sw.flush() - sw.getBuffer.toString - } - - def prettyPrint(jsValue: JsValue): String = withStringWriter { sw => - val gen = stringJsonGenerator(sw).setPrettyPrinter( - new DefaultPrettyPrinter() - ) + def prettyPrint(jsValue: JsValue): String = { val writer: ObjectWriter = mapper().writerWithDefaultPrettyPrinter() - - writer.writeValue(gen, jsValue) - sw.flush() - sw.getBuffer.toString + writer.writeValueAsString(jsValue) } def prettyPrintToStream(jsValue: JsValue, stream: OutputStream): Unit = { - val gen = stringJsonGenerator(stream).setPrettyPrinter( - new DefaultPrettyPrinter() - ) val writer: ObjectWriter = mapper().writerWithDefaultPrettyPrinter() - - writer.writeValue(gen, jsValue) + writer.writeValue(stream, jsValue) } def jsValueToBytes(jsValue: JsValue): Array[Byte] = From 181f1002efc8cfda11cb4e02921df5f478075153 Mon Sep 17 00:00:00 2001 From: Matthias Kurz Date: Wed, 15 Jul 2026 12:36:16 +0200 Subject: [PATCH 3/4] Avoid copying custom ObjectMappers Configure non-ASCII escaping on a per-call ObjectWriter so custom ObjectMapper subclasses do not need to implement copy(). This also keeps mapper configuration live and preserves the zero-argument mapper accessor and null reset behavior. --- .../api/libs/json/jackson/JacksonJson.scala | 81 ++++++++----------- .../scala/play/api/libs/json/JsonSpec.scala | 17 +++- 2 files changed, 51 insertions(+), 47 deletions(-) diff --git a/play-json/jvm/src/main/scala/play/api/libs/json/jackson/JacksonJson.scala b/play-json/jvm/src/main/scala/play/api/libs/json/jackson/JacksonJson.scala index f6156212..5d091e4b 100644 --- a/play-json/jvm/src/main/scala/play/api/libs/json/jackson/JacksonJson.scala +++ b/play-json/jvm/src/main/scala/play/api/libs/json/jackson/JacksonJson.scala @@ -281,54 +281,35 @@ private[play] object JacksonJson { } private[play] case class JacksonJson(defaultMapperJsonConfig: JsonConfig) { - private var currentMapper: ObjectMapper = null - private var currentMapperWithEscapeNonAscii: ObjectMapper = null - - private val defaultMapper: ObjectMapper = buildMapper(false) - private val defaultMapperWithEscapeNonAscii: ObjectMapper = buildMapper(true) - - private def buildMapper(escapeNonAscii: Boolean): ObjectMapper = { - JsonMapper - .builder( - new JsonFactoryBuilder() - .streamReadConstraints(defaultMapperJsonConfig.streamReadConstraints) - .streamWriteConstraints(defaultMapperJsonConfig.streamWriteConstraints) - .build() - ) - .addModules( - new ParameterNamesModule(), - new Jdk8Module(), - new JavaTimeModule(), - new DefaultScalaModule(), - new PlayJsonMapperModule(defaultMapperJsonConfig), - ) - .disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES) - .disable(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS) - .disable(SerializationFeature.WRITE_DURATIONS_AS_TIMESTAMPS) - .disable(SerializationFeature.FAIL_ON_EMPTY_BEANS) - .configure(JsonWriteFeature.ESCAPE_NON_ASCII, escapeNonAscii) - .build() - } - - private[play] def mapper(escapeNonAscii: Boolean = false): ObjectMapper = { - if (escapeNonAscii) { - if (currentMapperWithEscapeNonAscii == null) { - defaultMapperWithEscapeNonAscii - } else { - currentMapperWithEscapeNonAscii - } - } else { - if (currentMapper == null) { - defaultMapper - } else { - currentMapper - } - } + private var currentMapper: ObjectMapper = null + private val defaultMapper: ObjectMapper = JsonMapper + .builder( + new JsonFactoryBuilder() + .streamReadConstraints(defaultMapperJsonConfig.streamReadConstraints) + .streamWriteConstraints(defaultMapperJsonConfig.streamWriteConstraints) + .build() + ) + .addModules( + new ParameterNamesModule(), + new Jdk8Module(), + new JavaTimeModule(), + new DefaultScalaModule(), + new PlayJsonMapperModule(defaultMapperJsonConfig), + ) + .disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES) + .disable(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS) + .disable(SerializationFeature.WRITE_DURATIONS_AS_TIMESTAMPS) + .disable(SerializationFeature.FAIL_ON_EMPTY_BEANS) + .build() + + private[play] def mapper(): ObjectMapper = if (currentMapper == null) { + defaultMapper + } else { + currentMapper } private[play] def setObjectMapper(mapper: ObjectMapper): Unit = { this.currentMapper = mapper - this.currentMapperWithEscapeNonAscii = mapper.copy().enable(JsonWriteFeature.ESCAPE_NON_ASCII.mappedFeature) } def parseJsValue(data: Array[Byte]): JsValue = @@ -340,8 +321,16 @@ private[play] case class JacksonJson(defaultMapperJsonConfig: JsonConfig) { def parseJsValue(stream: InputStream): JsValue = mapper().readValue(stream, classOf[JsValue]) - def generateFromJsValue(jsValue: JsValue, escapeNonASCII: Boolean): String = - mapper(escapeNonASCII).writeValueAsString(jsValue) + def generateFromJsValue(jsValue: JsValue, escapeNonASCII: Boolean): String = { + val writer = mapper().writer() + val configuredWriter = if (escapeNonASCII) { + writer.`with`(JsonWriteFeature.ESCAPE_NON_ASCII) + } else { + writer + } + + configuredWriter.writeValueAsString(jsValue) + } def prettyPrint(jsValue: JsValue): String = { val writer: ObjectWriter = mapper().writerWithDefaultPrettyPrinter() diff --git a/play-json/jvm/src/test/scala/play/api/libs/json/JsonSpec.scala b/play-json/jvm/src/test/scala/play/api/libs/json/JsonSpec.scala index 506a0256..89e02f7b 100644 --- a/play-json/jvm/src/test/scala/play/api/libs/json/JsonSpec.scala +++ b/play-json/jvm/src/test/scala/play/api/libs/json/JsonSpec.scala @@ -17,7 +17,7 @@ import com.fasterxml.jackson.databind.ObjectMapper import com.fasterxml.jackson.databind.node.{ ArrayNode, NumericNode, ObjectNode } import play.api.libs.functional.syntax._ import play.api.libs.json.Json._ -import play.api.libs.json.jackson.JacksonJson +import play.api.libs.json.jackson.{ JacksonJson, PlayJsonMapperModule } class JsonSpec extends org.specs2.mutable.Specification { @@ -529,6 +529,21 @@ class JsonSpec extends org.specs2.mutable.Specification { ) } + "Use a custom ObjectMapper subclass for ASCII serialization" in { + val jacksonJson = JacksonJson(JsonConfig.settings) + val customMapper = new ObjectMapper() {} + + jacksonJson.setObjectMapper(customMapper) + customMapper.registerModule(new PlayJsonMapperModule()) + + jacksonJson + .generateFromJsValue(JsString("é"), escapeNonASCII = true) + .mustEqual("\"\\u00E9\"") + + jacksonJson.setObjectMapper(null) + jacksonJson.mapper().eq(customMapper).mustEqual(false) + } + "Serialize JsNumbers with integers correctly" in { val numStrings = Seq( "0", From 25b2545cfb9bca384088631ff246c9047095d25a Mon Sep 17 00:00:00 2001 From: Matthias Kurz Date: Wed, 15 Jul 2026 12:38:08 +0200 Subject: [PATCH 4/4] Keep pretty-print output streams open Disable Jackson's AUTO_CLOSE_TARGET feature for pretty printing so the caller retains ownership of the output stream. Document the contract and cover it with a close-tracking stream. --- .../play/api/libs/json/jackson/JacksonJson.scala | 5 ++++- .../src/main/scala/play/api/libs/json/Json.scala | 2 +- .../play/api/libs/json/JsonSharedSpec.scala | 16 ++++++++++++++-- 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/play-json/jvm/src/main/scala/play/api/libs/json/jackson/JacksonJson.scala b/play-json/jvm/src/main/scala/play/api/libs/json/jackson/JacksonJson.scala index 5d091e4b..1114a590 100644 --- a/play-json/jvm/src/main/scala/play/api/libs/json/jackson/JacksonJson.scala +++ b/play-json/jvm/src/main/scala/play/api/libs/json/jackson/JacksonJson.scala @@ -17,6 +17,7 @@ import com.fasterxml.jackson.core.JsonFactoryBuilder import com.fasterxml.jackson.core.JsonGenerator import com.fasterxml.jackson.core.JsonParser import com.fasterxml.jackson.core.JsonTokenId +import com.fasterxml.jackson.core.StreamWriteFeature import com.fasterxml.jackson.core.Version import com.fasterxml.jackson.core.json.JsonWriteFeature @@ -338,7 +339,9 @@ private[play] case class JacksonJson(defaultMapperJsonConfig: JsonConfig) { } def prettyPrintToStream(jsValue: JsValue, stream: OutputStream): Unit = { - val writer: ObjectWriter = mapper().writerWithDefaultPrettyPrinter() + val writer: ObjectWriter = mapper() + .writerWithDefaultPrettyPrinter() + .without(StreamWriteFeature.AUTO_CLOSE_TARGET) writer.writeValue(stream, jsValue) } 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 25440a64..2b5bde40 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 @@ -160,7 +160,7 @@ sealed trait JsonFacade { * writes the result to an output stream. * * $jsonParam - * @param stream the stream to write to. + * @param stream the stream to write to; it is not closed by this method. */ def prettyPrintToStream(json: JsValue, stream: OutputStream): Unit 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 a479045d..62cb66be 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 @@ -21,6 +21,17 @@ class JsonSharedSpec with org.scalatest.TryValues with org.scalatestplus.scalacheck.ScalaCheckPropertyChecks { + private class CloseTrackingOutputStream extends ByteArrayOutputStream { + private var closed = false + + def isClosed: Boolean = closed + + override def close(): Unit = { + closed = true + super.close() + } + } + case class User(id: Long, name: String, friends: List[User]) implicit val UserFormat: Format[User] = ( @@ -344,15 +355,16 @@ class JsonSharedSpec }""") } - "JSON pretty print to stream" in json { js => + "JSON pretty print to stream without closing it" in json { js => def jo = js.obj( "key1" -> "toto", "key2" -> js.obj("key21" -> "tata", "key22" -> 123), "key3" -> js.arr(1, "tutu") ) - val stream = new ByteArrayOutputStream() + val stream = new CloseTrackingOutputStream() js.prettyPrintToStream(jo, stream) + stream.isClosed.mustEqual(false) stream .toString("UTF-8") .mustEqual("""{