From 07ed891d69800b5120f5e08db61e3f4e8245c6ff Mon Sep 17 00:00:00 2001 From: He-Pin Date: Sat, 4 Jul 2026 18:46:49 +0800 Subject: [PATCH 1/3] 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" -> """[[[]]]""") From 7da3db0d0342fb44ee59adca3787d5b2dd9465c2 Mon Sep 17 00:00:00 2001 From: He-Pin Date: Wed, 12 Aug 2026 14:57:52 +0800 Subject: [PATCH 2/3] docs: clarify layered defense roles in surrogate rejection Motivation: The three defense layers (visitor check, byte scanner, ujson message match) have non-obvious roles and dependencies that future maintainers need to understand. Modification: Add comments explaining: (1) byte scanner catches surrogates even if ujson normalizes to U+FFFD before calling visitString; (2) the message heuristic is fragile and depends on upickle#722 behavior. Result: Defense hierarchy is self-documenting; fragility is explicitly noted. --- sjsonnet/src/sjsonnet/Importer.scala | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/sjsonnet/src/sjsonnet/Importer.scala b/sjsonnet/src/sjsonnet/Importer.scala index 147588da8..8dd2094b4 100644 --- a/sjsonnet/src/sjsonnet/Importer.scala +++ b/sjsonnet/src/sjsonnet/Importer.scala @@ -416,6 +416,8 @@ object CachedResolver { private final val LowerA = 'a'.toByte private final val LowerF = 'f'.toByte + // Scans raw bytes for \uD800-\uDFFF escape sequences, catching unpaired surrogates even if + // ujson normalizes them to U+FFFD before calling visitString (where the visitor check would miss). private def rejectUnpairedSurrogateEscapes(bytes: Array[Byte]): Unit = { var i = 0 var foundPotentialSurrogateEscape = false @@ -485,6 +487,9 @@ object CachedResolver { else if (b <= UpperF) b - UpperA + 10 else b - LowerA + 10 + // Last-resort catch for ujson's own surrogate validation (upickle#722). + // Fragile: depends on ujson throwing plain Exception with "Un-paired ... surrogate ..." message. + // Primary defenses are ValVisitor.rejectUnpairedSurrogates + rejectUnpairedSurrogateEscapes above. private def isUnpairedSurrogateParserError(e: Exception): Boolean = e.getClass == classOf[Exception] && e.getMessage != null && From 8849768fcda10bd740a5f7d65034df19115e08d7 Mon Sep 17 00:00:00 2001 From: He-Pin Date: Thu, 13 Aug 2026 12:12:38 +0800 Subject: [PATCH 3/3] fix: report line:col position for unpaired surrogate errors Motivation: The byte scanner knows the exact offset of the unpaired surrogate escape, but the error was reported with noOffsetPos, making it hard to locate the problem in large JSON files. Modification: Add byte offset to InvalidJsonUnicode exception. The scanner passes the offset of the backslash that starts the escape sequence. CachedResolver and Preloader use the offset to create positioned errors. Result: Import errors now report the line:col of the unpaired surrogate escape, improving debuggability for large JSON files. --- sjsonnet/src/sjsonnet/Importer.scala | 15 +++++++++------ sjsonnet/src/sjsonnet/Preloader.scala | 4 ++-- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/sjsonnet/src/sjsonnet/Importer.scala b/sjsonnet/src/sjsonnet/Importer.scala index 8dd2094b4..d16320188 100644 --- a/sjsonnet/src/sjsonnet/Importer.scala +++ b/sjsonnet/src/sjsonnet/Importer.scala @@ -323,7 +323,10 @@ class CachedResolver( ) } catch { case e: CachedResolver.InvalidJsonUnicode => - Left(new Error(CachedResolver.InvalidJsonUnicodeMessage).addFrame(e.fileScope.noOffsetPos)) + val pos = + if (e.offset >= 0) new Position(e.fileScope, e.offset) + else e.fileScope.noOffsetPos + Left(new Error(CachedResolver.InvalidJsonUnicodeMessage).addFrame(pos)) } } @@ -369,7 +372,7 @@ 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) + private[sjsonnet] final class InvalidJsonUnicode(val fileScope: FileScope, val offset: Int = -1) extends RuntimeException(null, null, false, false) private[sjsonnet] val InvalidJsonUnicodeMessage = "Invalid JSON: unpaired surrogate in string" @@ -391,7 +394,7 @@ object CachedResolver { val visitor = new JsonImportVisitor(fileScope, internedStrings, settings) val expr = ujson.ByteArrayParser.transform(bytes, visitor) - rejectUnpairedSurrogateEscapes(bytes) + rejectUnpairedSurrogateEscapes(bytes, fileScope) (expr, fileScope) } catch { case _: ValVisitor.InvalidUnicodeString => @@ -418,7 +421,7 @@ object CachedResolver { // Scans raw bytes for \uD800-\uDFFF escape sequences, catching unpaired surrogates even if // ujson normalizes them to U+FFFD before calling visitString (where the visitor check would miss). - private def rejectUnpairedSurrogateEscapes(bytes: Array[Byte]): Unit = { + private def rejectUnpairedSurrogateEscapes(bytes: Array[Byte], fileScope: FileScope): Unit = { var i = 0 var foundPotentialSurrogateEscape = false while (i + 5 < bytes.length && !foundPotentialSurrogateEscape) { @@ -446,11 +449,11 @@ object CachedResolver { bytes(nextEscape + 1) != U || !isHex4(bytes, nextEscape + 2) || !Character.isLowSurrogate(hex4(bytes, nextEscape + 2).toChar) ) { - throw new ValVisitor.InvalidUnicodeString + throw new InvalidJsonUnicode(fileScope, i - 1) } i = nextEscape + 6 } else if (Character.isLowSurrogate(code.toChar)) { - throw new ValVisitor.InvalidUnicodeString + throw new InvalidJsonUnicode(fileScope, i - 1) } else { i += 5 } diff --git a/sjsonnet/src/sjsonnet/Preloader.scala b/sjsonnet/src/sjsonnet/Preloader.scala index 5549fceba..4bfc23b14 100644 --- a/sjsonnet/src/sjsonnet/Preloader.scala +++ b/sjsonnet/src/sjsonnet/Preloader.scala @@ -168,8 +168,8 @@ class Preloader(parentImporter: Importer, settings: Settings = Settings.default) } } } catch { - case _: CachedResolver.InvalidJsonUnicode => - Left(new ParseError(s"$path: ${CachedResolver.InvalidJsonUnicodeMessage}")) + case e: CachedResolver.InvalidJsonUnicode => + Left(new ParseError(s"$path: ${CachedResolver.InvalidJsonUnicodeMessage}", offset = e.offset)) } }