diff --git a/sjsonnet/src/sjsonnet/Importer.scala b/sjsonnet/src/sjsonnet/Importer.scala index f5a2257d..147588da 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 2c5a483d..5549fceb 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 33eda06c..f0d103bd 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 b7591e8a..bbb237ac 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 00000000..3917870b --- /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 00000000..326f493b --- /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 00000000..6ebe068d --- /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 00000000..52470642 --- /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 00000000..faefa7b8 --- /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 00000000..c4554cb1 --- /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 00000000..a590a98b --- /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 00000000..27ba77dd --- /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 515a37cb..a41331f8 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" -> """[[[]]]""")