From 07ed891d69800b5120f5e08db61e3f4e8245c6ff Mon Sep 17 00:00:00 2001 From: He-Pin Date: Sat, 4 Jul 2026 18:46:49 +0800 Subject: [PATCH] fix: reject unpaired JSON surrogates Motivation: std.parseJson and strict .json import fast paths accepted or normalized JSON strings containing unpaired UTF-16 surrogate escapes. That allowed invalid JSON Unicode to become Jsonnet strings, diverging from cpp-jsonnet and jrsonnet behavior and from the intent of rejecting malformed JSON input. Modification: Add a shared surrogate validator for JSON visitors, enable it for std.parseJson, and convert strict .json import surrogate failures into normal Jsonnet errors instead of falling back or producing raw invalid strings. Keep the import fast path allocation-conscious with a private nullable return contract, leaving any reusable OptionVal abstraction for a separate PR. Result: Lone high/low surrogate values and keys now fail with Invalid JSON: unpaired surrogate in string, valid surrogate pairs still materialize as codepoint strings, escaped literal text such as \\uD800 remains ordinary text, and loose Jsonnet .json files can still fall back through the Jsonnet parser. References: https://jsonnet.org/ref/stdlib.html#std-parseJson https://github.com/google/go-jsonnet/issues/885 https://github.com/com-lihaoyi/upickle/issues/722 --- sjsonnet/src/sjsonnet/Importer.scala | 159 +++++++++++++++--- sjsonnet/src/sjsonnet/Preloader.scala | 22 ++- sjsonnet/src/sjsonnet/ValVisitor.scala | 35 +++- .../src/sjsonnet/stdlib/ManifestModule.scala | 7 +- ...rror.parsejson_lone_high_surrogate.jsonnet | 1 + ...rsejson_lone_high_surrogate.jsonnet.golden | 2 + ...error.parsejson_lone_low_surrogate.jsonnet | 1 + ...arsejson_lone_low_surrogate.jsonnet.golden | 2 + ...error.parsejson_lone_surrogate_key.jsonnet | 1 + ...arsejson_lone_surrogate_key.jsonnet.golden | 2 + .../parsejson_surrogate_pairs.jsonnet | 6 + .../parsejson_surrogate_pairs.jsonnet.golden | 1 + .../sjsonnet/JsonImportFastPathTests.scala | 54 ++++++ 13 files changed, 258 insertions(+), 35 deletions(-) create mode 100644 sjsonnet/test/resources/new_test_suite/error.parsejson_lone_high_surrogate.jsonnet create mode 100644 sjsonnet/test/resources/new_test_suite/error.parsejson_lone_high_surrogate.jsonnet.golden create mode 100644 sjsonnet/test/resources/new_test_suite/error.parsejson_lone_low_surrogate.jsonnet create mode 100644 sjsonnet/test/resources/new_test_suite/error.parsejson_lone_low_surrogate.jsonnet.golden create mode 100644 sjsonnet/test/resources/new_test_suite/error.parsejson_lone_surrogate_key.jsonnet create mode 100644 sjsonnet/test/resources/new_test_suite/error.parsejson_lone_surrogate_key.jsonnet.golden create mode 100644 sjsonnet/test/resources/new_test_suite/parsejson_surrogate_pairs.jsonnet create mode 100644 sjsonnet/test/resources/new_test_suite/parsejson_surrogate_pairs.jsonnet.golden diff --git a/sjsonnet/src/sjsonnet/Importer.scala b/sjsonnet/src/sjsonnet/Importer.scala index f5a2257d5..147588da8 100644 --- a/sjsonnet/src/sjsonnet/Importer.scala +++ b/sjsonnet/src/sjsonnet/Importer.scala @@ -302,24 +302,29 @@ class CachedResolver( def parse(path: Path, content: ResolvedFile)(implicit ev: EvalErrorScope): Either[Error, (Expr, FileScope)] = { - parseCache.getOrElseUpdate( - (path, content.contentHash()), { - val parsed: Either[Error, (Expr, FileScope)] = content.preParsedAst match { - case Some(pre) => Right(pre) - case None => - CachedResolver.parseJsonImport( - path, - content, - internedStrings, - settings - ) match { - case Some(parsedJson) => Right(parsedJson) - case None => parseJsonnet(path, content) - } + try { + parseCache.getOrElseUpdate( + (path, content.contentHash()), { + val parsed: Either[Error, (Expr, FileScope)] = content.preParsedAst match { + case Some(pre) => Right(pre) + case None => + CachedResolver.parseJsonImportOrNull( + path, + content, + internedStrings, + settings + ) match { + case null => parseJsonnet(path, content) + case parsedJson => Right(parsedJson) + } + } + parsed.flatMap { case (e, fs) => process(e, fs) } } - parsed.flatMap { case (e, fs) => process(e, fs) } - } - ) + ) + } catch { + case e: CachedResolver.InvalidJsonUnicode => + Left(new Error(CachedResolver.InvalidJsonUnicodeMessage).addFrame(e.fileScope.noOffsetPos)) + } } private def parseJsonnet(path: Path, content: ResolvedFile)(implicit @@ -364,25 +369,128 @@ object CachedResolver { private final class DuplicateJsonKey extends RuntimeException(null, null, false, false) private final class InvalidJsonNumber extends RuntimeException(null, null, false, false) private final class JsonParseDepthExceeded extends RuntimeException(null, null, false, false) + private[sjsonnet] final class InvalidJsonUnicode(val fileScope: FileScope) + extends RuntimeException(null, null, false, false) - private[sjsonnet] def parseJsonImport( + private[sjsonnet] val InvalidJsonUnicodeMessage = "Invalid JSON: unpaired surrogate in string" + + /** + * Parses strict `.json` imports through ujson's parser. Returns null when this fast path should + * fall back to the Jsonnet parser; the nullable result avoids Some/None allocations in the import + * hot path without adding a reusable OptionVal abstraction to this semantic fix. + */ + private[sjsonnet] def parseJsonImportOrNull( path: Path, content: ResolvedFile, internedStrings: mutable.HashMap[String, String], - settings: Settings): Option[(Expr, FileScope)] = { - if (!path.last.endsWith(".json")) return None + settings: Settings): (Expr, FileScope) = { + if (!path.last.endsWith(".json")) return null val fileScope = new FileScope(path) try { + val bytes = content.readRawBytes() val visitor = new JsonImportVisitor(fileScope, internedStrings, settings) - Some((ujson.ByteArrayParser.transform(content.readRawBytes(), visitor), fileScope)) + val expr = ujson.ByteArrayParser.transform(bytes, visitor) + rejectUnpairedSurrogateEscapes(bytes) + (expr, fileScope) } catch { + case _: ValVisitor.InvalidUnicodeString => + throw new InvalidJsonUnicode(fileScope) + case e: Exception if isUnpairedSurrogateParserError(e) => + throw new InvalidJsonUnicode(fileScope) case _: ujson.ParsingFailedException | _: DuplicateJsonKey | _: InvalidJsonNumber | _: JsonParseDepthExceeded | _: NumberFormatException => - None + null } } + private final val Backslash = '\\'.toByte + private final val Quote = '"'.toByte + private final val U = 'u'.toByte + private final val UpperD = 'D'.toByte + private final val LowerD = 'd'.toByte + private final val Zero = '0'.toByte + private final val Nine = '9'.toByte + private final val UpperA = 'A'.toByte + private final val UpperF = 'F'.toByte + private final val LowerA = 'a'.toByte + private final val LowerF = 'f'.toByte + + private def rejectUnpairedSurrogateEscapes(bytes: Array[Byte]): Unit = { + var i = 0 + var foundPotentialSurrogateEscape = false + while (i + 5 < bytes.length && !foundPotentialSurrogateEscape) { + foundPotentialSurrogateEscape = + bytes(i) == Backslash && bytes(i + 1) == U && isD(bytes(i + 2)) + i += 1 + } + if (!foundPotentialSurrogateEscape) return + + i = 0 + var inString = false + var escaped = false + while (i < bytes.length) { + val b = bytes(i) + if (!inString) { + if (b == Quote) inString = true + i += 1 + } else if (escaped) { + if (b == U && i + 4 < bytes.length && isHex4(bytes, i + 1)) { + val code = hex4(bytes, i + 1) + if (Character.isHighSurrogate(code.toChar)) { + val nextEscape = i + 5 + if ( + nextEscape + 6 > bytes.length || bytes(nextEscape) != Backslash || + bytes(nextEscape + 1) != U || !isHex4(bytes, nextEscape + 2) || + !Character.isLowSurrogate(hex4(bytes, nextEscape + 2).toChar) + ) { + throw new ValVisitor.InvalidUnicodeString + } + i = nextEscape + 6 + } else if (Character.isLowSurrogate(code.toChar)) { + throw new ValVisitor.InvalidUnicodeString + } else { + i += 5 + } + } else { + i += 1 + } + escaped = false + } else if (b == Backslash) { + escaped = true + i += 1 + } else { + if (b == Quote) inString = false + i += 1 + } + } + } + + private def isD(b: Byte): Boolean = b == UpperD || b == LowerD + + private def isHex4(bytes: Array[Byte], offset: Int): Boolean = + isHex(bytes(offset)) && isHex(bytes(offset + 1)) && isHex(bytes(offset + 2)) && + isHex(bytes(offset + 3)) + + private def isHex(b: Byte): Boolean = + (b >= Zero && b <= Nine) || (b >= UpperA && b <= UpperF) || + (b >= LowerA && b <= LowerF) + + private def hex4(bytes: Array[Byte], offset: Int): Int = + (hex(bytes(offset)) << 12) | (hex(bytes(offset + 1)) << 8) | + (hex(bytes(offset + 2)) << 4) | hex(bytes(offset + 3)) + + private def hex(b: Byte): Int = + if (b <= Nine) b - Zero + else if (b <= UpperF) b - UpperA + 10 + else b - LowerA + 10 + + private def isUnpairedSurrogateParserError(e: Exception): Boolean = + e.getClass == classOf[Exception] && + e.getMessage != null && + e.getMessage.startsWith("Un-paired ") && + e.getMessage.contains(" surrogate ") + private final class JsonImportVisitor( fileScope: FileScope, internedStrings: mutable.HashMap[String, String], @@ -420,7 +528,11 @@ object CachedResolver { private var key: String = _ def subVisitor: upickle.core.Visitor[?, ?] = self def visitKey(index: Int): upickle.core.StringVisitor.type = upickle.core.StringVisitor - def visitKeyValue(s: Any): Unit = key = intern(s.toString) + def visitKeyValue(s: Any): Unit = { + val str = s.toString + ValVisitor.rejectUnpairedSurrogates(str) + key = intern(str) + } def visitValue(v: Val, index: Int): Unit = { if (!seen.add(key)) throw new DuplicateJsonKey keys += key @@ -474,6 +586,7 @@ object CachedResolver { case str: String => str case _ => s.toString } + ValVisitor.rejectUnpairedSurrogates(str) val unique = intern(str) Val.Str(pos(index), unique) } diff --git a/sjsonnet/src/sjsonnet/Preloader.scala b/sjsonnet/src/sjsonnet/Preloader.scala index 2c5a483d1..5549fceba 100644 --- a/sjsonnet/src/sjsonnet/Preloader.scala +++ b/sjsonnet/src/sjsonnet/Preloader.scala @@ -130,16 +130,18 @@ class Preloader(parentImporter: Importer, settings: Settings = Settings.default) } private def discover(path: Path, content: ResolvedFile): Either[Error, Unit] = { - CachedResolver.parseJsonImport( - path, - content, - internedStrings, - settings - ) match { - case Some((expr, fs)) => + try { + val parsedJson = CachedResolver.parseJsonImportOrNull( + path, + content, + internedStrings, + settings + ) + if (parsedJson != null) { + val (expr, fs) = parsedJson cache.put((path, false), PreParsedResolvedFile(content, expr, fs)) Right(()) - case None => + } else { val parser = new Parser(path, internedStrings, internedFieldSets, settings) try { fastparse.parse(content.getParserInput(), parser.document(_)) match { @@ -164,6 +166,10 @@ class Preloader(parentImporter: Importer, settings: Settings = Settings.default) } catch { case e: ParseError => Left(e) } + } + } catch { + case _: CachedResolver.InvalidJsonUnicode => + Left(new ParseError(s"$path: ${CachedResolver.InvalidJsonUnicodeMessage}")) } } diff --git a/sjsonnet/src/sjsonnet/ValVisitor.scala b/sjsonnet/src/sjsonnet/ValVisitor.scala index 33eda06c6..f0d103bd6 100644 --- a/sjsonnet/src/sjsonnet/ValVisitor.scala +++ b/sjsonnet/src/sjsonnet/ValVisitor.scala @@ -7,8 +7,31 @@ import upickle.core.{ArrVisitor, ObjVisitor, Visitor} import scala.collection.mutable +object ValVisitor { + final class InvalidUnicodeString extends RuntimeException(null, null, false, false) + + def rejectUnpairedSurrogates(s: CharSequence): Unit = { + var i = 0 + val length = s.length + while (i < length) { + val c = s.charAt(i) + if (Character.isHighSurrogate(c)) { + if (i + 1 >= length || !Character.isLowSurrogate(s.charAt(i + 1))) { + throw new InvalidUnicodeString + } + i += 2 + } else if (Character.isLowSurrogate(c)) { + throw new InvalidUnicodeString + } else { + i += 1 + } + } + } +} + /** Parse JSON directly into a literal `Val` */ -class ValVisitor(pos: Position) extends JsVisitor[Val, Val] { self => +class ValVisitor(pos: Position, rejectUnpairedSurrogates: Boolean = false) + extends JsVisitor[Val, Val] { self => override def visitJsonableObject(length: Int, index: Int): ObjVisitor[Val, Val] = visitObject(length, index) @@ -27,7 +50,10 @@ class ValVisitor(pos: Position) extends JsVisitor[Val, Val] { self => var key: String = _ def subVisitor: Visitor[?, ?] = self def visitKey(index: Int): upickle.core.StringVisitor.type = upickle.core.StringVisitor - def visitKeyValue(s: Any): Unit = key = s.toString + def visitKeyValue(s: Any): Unit = { + key = s.toString + if (rejectUnpairedSurrogates) ValVisitor.rejectUnpairedSurrogates(key) + } def visitValue(v: Val, index: Int): Unit = { cache.put(key, v) allKeys.put(key, false) @@ -52,5 +78,8 @@ class ValVisitor(pos: Position) extends JsVisitor[Val, Val] { self => } ) - def visitString(s: CharSequence, index: Int): Val = Val.Str(pos, s.toString) + def visitString(s: CharSequence, index: Int): Val = { + if (rejectUnpairedSurrogates) ValVisitor.rejectUnpairedSurrogates(s) + Val.Str(pos, s.toString) + } } diff --git a/sjsonnet/src/sjsonnet/stdlib/ManifestModule.scala b/sjsonnet/src/sjsonnet/stdlib/ManifestModule.scala index b7591e8ac..bbb237acb 100644 --- a/sjsonnet/src/sjsonnet/stdlib/ManifestModule.scala +++ b/sjsonnet/src/sjsonnet/stdlib/ManifestModule.scala @@ -126,8 +126,13 @@ object ManifestModule extends AbstractFunctionModule { private object ParseJson extends Val.Builtin1("parseJson", "str") { def evalRhs(str: Eval, ev: EvalScope, pos: Position): Val = { try { - ujson.StringParser.transform(str.value.asString, new ValVisitor(pos)) + ujson.StringParser.transform( + str.value.asString, + new ValVisitor(pos, rejectUnpairedSurrogates = true) + ) } catch { + case _: ValVisitor.InvalidUnicodeString => + throw Error.fail("Invalid JSON: unpaired surrogate in string", pos)(ev) case e: ujson.ParseException => throw Error.fail("Invalid JSON: " + e.getMessage, pos)(ev) case _: ujson.IncompleteParseException => diff --git a/sjsonnet/test/resources/new_test_suite/error.parsejson_lone_high_surrogate.jsonnet b/sjsonnet/test/resources/new_test_suite/error.parsejson_lone_high_surrogate.jsonnet new file mode 100644 index 000000000..3917870bd --- /dev/null +++ b/sjsonnet/test/resources/new_test_suite/error.parsejson_lone_high_surrogate.jsonnet @@ -0,0 +1 @@ +std.parseJson('"\\uD800"') diff --git a/sjsonnet/test/resources/new_test_suite/error.parsejson_lone_high_surrogate.jsonnet.golden b/sjsonnet/test/resources/new_test_suite/error.parsejson_lone_high_surrogate.jsonnet.golden new file mode 100644 index 000000000..326f493b1 --- /dev/null +++ b/sjsonnet/test/resources/new_test_suite/error.parsejson_lone_high_surrogate.jsonnet.golden @@ -0,0 +1,2 @@ +sjsonnet.Error: [std.parseJson] Invalid JSON: unpaired surrogate in string + at [].(error.parsejson_lone_high_surrogate.jsonnet:1:14) diff --git a/sjsonnet/test/resources/new_test_suite/error.parsejson_lone_low_surrogate.jsonnet b/sjsonnet/test/resources/new_test_suite/error.parsejson_lone_low_surrogate.jsonnet new file mode 100644 index 000000000..6ebe068df --- /dev/null +++ b/sjsonnet/test/resources/new_test_suite/error.parsejson_lone_low_surrogate.jsonnet @@ -0,0 +1 @@ +std.parseJson('"\\uDC00"') diff --git a/sjsonnet/test/resources/new_test_suite/error.parsejson_lone_low_surrogate.jsonnet.golden b/sjsonnet/test/resources/new_test_suite/error.parsejson_lone_low_surrogate.jsonnet.golden new file mode 100644 index 000000000..524706423 --- /dev/null +++ b/sjsonnet/test/resources/new_test_suite/error.parsejson_lone_low_surrogate.jsonnet.golden @@ -0,0 +1,2 @@ +sjsonnet.Error: [std.parseJson] Invalid JSON: unpaired surrogate in string + at [].(error.parsejson_lone_low_surrogate.jsonnet:1:14) diff --git a/sjsonnet/test/resources/new_test_suite/error.parsejson_lone_surrogate_key.jsonnet b/sjsonnet/test/resources/new_test_suite/error.parsejson_lone_surrogate_key.jsonnet new file mode 100644 index 000000000..faefa7b85 --- /dev/null +++ b/sjsonnet/test/resources/new_test_suite/error.parsejson_lone_surrogate_key.jsonnet @@ -0,0 +1 @@ +std.parseJson('{"\\uD800": 1}') diff --git a/sjsonnet/test/resources/new_test_suite/error.parsejson_lone_surrogate_key.jsonnet.golden b/sjsonnet/test/resources/new_test_suite/error.parsejson_lone_surrogate_key.jsonnet.golden new file mode 100644 index 000000000..c4554cb12 --- /dev/null +++ b/sjsonnet/test/resources/new_test_suite/error.parsejson_lone_surrogate_key.jsonnet.golden @@ -0,0 +1,2 @@ +sjsonnet.Error: [std.parseJson] Invalid JSON: unpaired surrogate in string + at [].(error.parsejson_lone_surrogate_key.jsonnet:1:14) diff --git a/sjsonnet/test/resources/new_test_suite/parsejson_surrogate_pairs.jsonnet b/sjsonnet/test/resources/new_test_suite/parsejson_surrogate_pairs.jsonnet new file mode 100644 index 000000000..a590a98bf --- /dev/null +++ b/sjsonnet/test/resources/new_test_suite/parsejson_surrogate_pairs.jsonnet @@ -0,0 +1,6 @@ +local obj = std.parseJson('{"\\uD83D\\uDE00": "\\u0041"}'); + +std.assertEqual(std.codepoint(std.parseJson('"\\uD83D\\uDE00"')), 128512) && +std.assertEqual(std.parseJson('"\\u0041"'), "A") && +std.assertEqual(std.objectFields(obj), ["😀"]) && +std.assertEqual(obj["😀"], "A") diff --git a/sjsonnet/test/resources/new_test_suite/parsejson_surrogate_pairs.jsonnet.golden b/sjsonnet/test/resources/new_test_suite/parsejson_surrogate_pairs.jsonnet.golden new file mode 100644 index 000000000..27ba77dda --- /dev/null +++ b/sjsonnet/test/resources/new_test_suite/parsejson_surrogate_pairs.jsonnet.golden @@ -0,0 +1 @@ +true diff --git a/sjsonnet/test/src/sjsonnet/JsonImportFastPathTests.scala b/sjsonnet/test/src/sjsonnet/JsonImportFastPathTests.scala index 515a37cb5..a41331f8b 100644 --- a/sjsonnet/test/src/sjsonnet/JsonImportFastPathTests.scala +++ b/sjsonnet/test/src/sjsonnet/JsonImportFastPathTests.scala @@ -35,6 +35,19 @@ object JsonImportFastPathTests extends TestSuite { } } + private def unicodeEscape(hex: String): String = "\\" + "u" + hex + + private def assertInvalidUnicodeImport(json: String): Unit = { + val result = eval(Map("surrogate.json" -> json), """import "surrogate.json"""") + assert(result.isLeft) + result match { + case Left(error) => + assert(error.contains("Invalid JSON: unpaired surrogate in string")) + assert(!error.contains("Internal Error")) + case Right(_) => assert(false) + } + } + def tests: Tests = Tests { test("strict json imports produce normal Jsonnet values") { val files = Map( @@ -89,6 +102,19 @@ object JsonImportFastPathTests extends TestSuite { eval(files, """import "loose.json"""") ==> Right(ujson.Obj("a" -> 1)) } + test("surrogate escapes in Jsonnet fallback comments do not fail json import") { + val files = Map( + "loose-surrogate-comment.json" -> + s"""// "${unicodeEscape("D800")}" + |{ + | a: 1, + |} + |""".stripMargin + ) + + eval(files, """import "loose-surrogate-comment.json"""") ==> Right(ujson.Obj("a" -> 1)) + } + test("duplicate json object keys keep Jsonnet duplicate-field error semantics") { val files = Map("dupe.json" -> """{"a":1,"a":2}""") @@ -129,6 +155,34 @@ object JsonImportFastPathTests extends TestSuite { } } + test("unpaired surrogate json imports fail before producing Jsonnet strings") { + assertInvalidUnicodeImport(s"""{"s":"${unicodeEscape("D800")}"}""") + assertInvalidUnicodeImport(s"""{"s":"${unicodeEscape("DC00")}"}""") + assertInvalidUnicodeImport(s""""${unicodeEscape("D800")}"""") + assertInvalidUnicodeImport(s""""${unicodeEscape("DC00")}"""") + } + + test("valid surrogate pair json imports keep codepoint strings") { + val pairEscape = unicodeEscape("D83D") + unicodeEscape("DE00") + val files = Map("surrogate-pair.json" -> s"""{"s":"$pairEscape","$pairEscape":"key"}""") + + eval(files, """local obj = import "surrogate-pair.json"; [std.codepoint(obj.s), obj[std.char(128512)]]""") ==> + Right(ujson.Arr(128512, "key")) + } + + test("escaped surrogate text json imports stay ordinary strings") { + val escapedText = "\\" + unicodeEscape("D800") + val files = Map( + "escaped-surrogate-text.json" -> s"""{"s":"$escapedText"}""", + "escaped-surrogate-text-with-suffix.json" -> s"""{"s":"${escapedText}abc"}""" + ) + + eval(files, """(import "escaped-surrogate-text.json").s""") ==> + Right(ujson.Str(unicodeEscape("D800"))) + eval(files, """(import "escaped-surrogate-text-with-suffix.json").s""") ==> + Right(ujson.Str(unicodeEscape("D800") + "abc")) + } + test("deep json imports keep parser recursion guard") { val files = Map("deep.json" -> """[[[]]]""")