Skip to content

Commit 8974a1e

Browse files
committed
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 google/go-jsonnet#885 com-lihaoyi/upickle#722
1 parent e686d89 commit 8974a1e

13 files changed

Lines changed: 258 additions & 35 deletions

sjsonnet/src/sjsonnet/Importer.scala

Lines changed: 136 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -302,24 +302,29 @@ class CachedResolver(
302302

303303
def parse(path: Path, content: ResolvedFile)(implicit
304304
ev: EvalErrorScope): Either[Error, (Expr, FileScope)] = {
305-
parseCache.getOrElseUpdate(
306-
(path, content.contentHash()), {
307-
val parsed: Either[Error, (Expr, FileScope)] = content.preParsedAst match {
308-
case Some(pre) => Right(pre)
309-
case None =>
310-
CachedResolver.parseJsonImport(
311-
path,
312-
content,
313-
internedStrings,
314-
settings
315-
) match {
316-
case Some(parsedJson) => Right(parsedJson)
317-
case None => parseJsonnet(path, content)
318-
}
305+
try {
306+
parseCache.getOrElseUpdate(
307+
(path, content.contentHash()), {
308+
val parsed: Either[Error, (Expr, FileScope)] = content.preParsedAst match {
309+
case Some(pre) => Right(pre)
310+
case None =>
311+
CachedResolver.parseJsonImportOrNull(
312+
path,
313+
content,
314+
internedStrings,
315+
settings
316+
) match {
317+
case null => parseJsonnet(path, content)
318+
case parsedJson => Right(parsedJson)
319+
}
320+
}
321+
parsed.flatMap { case (e, fs) => process(e, fs) }
319322
}
320-
parsed.flatMap { case (e, fs) => process(e, fs) }
321-
}
322-
)
323+
)
324+
} catch {
325+
case e: CachedResolver.InvalidJsonUnicode =>
326+
Left(new Error(CachedResolver.InvalidJsonUnicodeMessage).addFrame(e.fileScope.noOffsetPos))
327+
}
323328
}
324329

325330
private def parseJsonnet(path: Path, content: ResolvedFile)(implicit
@@ -364,25 +369,128 @@ object CachedResolver {
364369
private final class DuplicateJsonKey extends RuntimeException(null, null, false, false)
365370
private final class InvalidJsonNumber extends RuntimeException(null, null, false, false)
366371
private final class JsonParseDepthExceeded extends RuntimeException(null, null, false, false)
372+
private[sjsonnet] final class InvalidJsonUnicode(val fileScope: FileScope)
373+
extends RuntimeException(null, null, false, false)
367374

368-
private[sjsonnet] def parseJsonImport(
375+
private[sjsonnet] val InvalidJsonUnicodeMessage = "Invalid JSON: unpaired surrogate in string"
376+
377+
/**
378+
* Parses strict `.json` imports through ujson's parser. Returns null when this fast path should
379+
* fall back to the Jsonnet parser; the nullable result avoids Some/None allocations in the import
380+
* hot path without adding a reusable OptionVal abstraction to this semantic fix.
381+
*/
382+
private[sjsonnet] def parseJsonImportOrNull(
369383
path: Path,
370384
content: ResolvedFile,
371385
internedStrings: mutable.HashMap[String, String],
372-
settings: Settings): Option[(Expr, FileScope)] = {
373-
if (!path.last.endsWith(".json")) return None
386+
settings: Settings): (Expr, FileScope) = {
387+
if (!path.last.endsWith(".json")) return null
374388
val fileScope = new FileScope(path)
375389
try {
390+
val bytes = content.readRawBytes()
376391
val visitor =
377392
new JsonImportVisitor(fileScope, internedStrings, settings)
378-
Some((ujson.ByteArrayParser.transform(content.readRawBytes(), visitor), fileScope))
393+
val expr = ujson.ByteArrayParser.transform(bytes, visitor)
394+
rejectUnpairedSurrogateEscapes(bytes)
395+
(expr, fileScope)
379396
} catch {
397+
case _: ValVisitor.InvalidUnicodeString =>
398+
throw new InvalidJsonUnicode(fileScope)
399+
case e: Exception if isUnpairedSurrogateParserError(e) =>
400+
throw new InvalidJsonUnicode(fileScope)
380401
case _: ujson.ParsingFailedException | _: DuplicateJsonKey | _: InvalidJsonNumber |
381402
_: JsonParseDepthExceeded | _: NumberFormatException =>
382-
None
403+
null
383404
}
384405
}
385406

407+
private final val Backslash = '\\'.toByte
408+
private final val Quote = '"'.toByte
409+
private final val U = 'u'.toByte
410+
private final val UpperD = 'D'.toByte
411+
private final val LowerD = 'd'.toByte
412+
private final val Zero = '0'.toByte
413+
private final val Nine = '9'.toByte
414+
private final val UpperA = 'A'.toByte
415+
private final val UpperF = 'F'.toByte
416+
private final val LowerA = 'a'.toByte
417+
private final val LowerF = 'f'.toByte
418+
419+
private def rejectUnpairedSurrogateEscapes(bytes: Array[Byte]): Unit = {
420+
var i = 0
421+
var foundPotentialSurrogateEscape = false
422+
while (i + 5 < bytes.length && !foundPotentialSurrogateEscape) {
423+
foundPotentialSurrogateEscape =
424+
bytes(i) == Backslash && bytes(i + 1) == U && isD(bytes(i + 2))
425+
i += 1
426+
}
427+
if (!foundPotentialSurrogateEscape) return
428+
429+
i = 0
430+
var inString = false
431+
var escaped = false
432+
while (i < bytes.length) {
433+
val b = bytes(i)
434+
if (!inString) {
435+
if (b == Quote) inString = true
436+
i += 1
437+
} else if (escaped) {
438+
if (b == U && i + 4 < bytes.length && isHex4(bytes, i + 1)) {
439+
val code = hex4(bytes, i + 1)
440+
if (Character.isHighSurrogate(code.toChar)) {
441+
val nextEscape = i + 5
442+
if (
443+
nextEscape + 6 > bytes.length || bytes(nextEscape) != Backslash ||
444+
bytes(nextEscape + 1) != U || !isHex4(bytes, nextEscape + 2) ||
445+
!Character.isLowSurrogate(hex4(bytes, nextEscape + 2).toChar)
446+
) {
447+
throw new ValVisitor.InvalidUnicodeString
448+
}
449+
i = nextEscape + 6
450+
} else if (Character.isLowSurrogate(code.toChar)) {
451+
throw new ValVisitor.InvalidUnicodeString
452+
} else {
453+
i += 5
454+
}
455+
} else {
456+
i += 1
457+
}
458+
escaped = false
459+
} else if (b == Backslash) {
460+
escaped = true
461+
i += 1
462+
} else {
463+
if (b == Quote) inString = false
464+
i += 1
465+
}
466+
}
467+
}
468+
469+
private def isD(b: Byte): Boolean = b == UpperD || b == LowerD
470+
471+
private def isHex4(bytes: Array[Byte], offset: Int): Boolean =
472+
isHex(bytes(offset)) && isHex(bytes(offset + 1)) && isHex(bytes(offset + 2)) &&
473+
isHex(bytes(offset + 3))
474+
475+
private def isHex(b: Byte): Boolean =
476+
(b >= Zero && b <= Nine) || (b >= UpperA && b <= UpperF) ||
477+
(b >= LowerA && b <= LowerF)
478+
479+
private def hex4(bytes: Array[Byte], offset: Int): Int =
480+
(hex(bytes(offset)) << 12) | (hex(bytes(offset + 1)) << 8) |
481+
(hex(bytes(offset + 2)) << 4) | hex(bytes(offset + 3))
482+
483+
private def hex(b: Byte): Int =
484+
if (b <= Nine) b - Zero
485+
else if (b <= UpperF) b - UpperA + 10
486+
else b - LowerA + 10
487+
488+
private def isUnpairedSurrogateParserError(e: Exception): Boolean =
489+
e.getClass == classOf[Exception] &&
490+
e.getMessage != null &&
491+
e.getMessage.startsWith("Un-paired ") &&
492+
e.getMessage.contains(" surrogate ")
493+
386494
private final class JsonImportVisitor(
387495
fileScope: FileScope,
388496
internedStrings: mutable.HashMap[String, String],
@@ -420,7 +528,11 @@ object CachedResolver {
420528
private var key: String = _
421529
def subVisitor: upickle.core.Visitor[?, ?] = self
422530
def visitKey(index: Int): upickle.core.StringVisitor.type = upickle.core.StringVisitor
423-
def visitKeyValue(s: Any): Unit = key = intern(s.toString)
531+
def visitKeyValue(s: Any): Unit = {
532+
val str = s.toString
533+
ValVisitor.rejectUnpairedSurrogates(str)
534+
key = intern(str)
535+
}
424536
def visitValue(v: Val, index: Int): Unit = {
425537
if (!seen.add(key)) throw new DuplicateJsonKey
426538
keys += key
@@ -474,6 +586,7 @@ object CachedResolver {
474586
case str: String => str
475587
case _ => s.toString
476588
}
589+
ValVisitor.rejectUnpairedSurrogates(str)
477590
val unique = intern(str)
478591
Val.Str(pos(index), unique)
479592
}

sjsonnet/src/sjsonnet/Preloader.scala

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -130,16 +130,18 @@ class Preloader(parentImporter: Importer, settings: Settings = Settings.default)
130130
}
131131

132132
private def discover(path: Path, content: ResolvedFile): Either[Error, Unit] = {
133-
CachedResolver.parseJsonImport(
134-
path,
135-
content,
136-
internedStrings,
137-
settings
138-
) match {
139-
case Some((expr, fs)) =>
133+
try {
134+
val parsedJson = CachedResolver.parseJsonImportOrNull(
135+
path,
136+
content,
137+
internedStrings,
138+
settings
139+
)
140+
if (parsedJson != null) {
141+
val (expr, fs) = parsedJson
140142
cache.put((path, false), PreParsedResolvedFile(content, expr, fs))
141143
Right(())
142-
case None =>
144+
} else {
143145
val parser = new Parser(path, internedStrings, internedFieldSets, settings)
144146
try {
145147
fastparse.parse(content.getParserInput(), parser.document(_)) match {
@@ -164,6 +166,10 @@ class Preloader(parentImporter: Importer, settings: Settings = Settings.default)
164166
} catch {
165167
case e: ParseError => Left(e)
166168
}
169+
}
170+
} catch {
171+
case _: CachedResolver.InvalidJsonUnicode =>
172+
Left(new ParseError(s"$path: ${CachedResolver.InvalidJsonUnicodeMessage}"))
167173
}
168174
}
169175

sjsonnet/src/sjsonnet/ValVisitor.scala

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,31 @@ import upickle.core.{ArrVisitor, ObjVisitor, Visitor}
77

88
import scala.collection.mutable
99

10+
object ValVisitor {
11+
final class InvalidUnicodeString extends RuntimeException(null, null, false, false)
12+
13+
def rejectUnpairedSurrogates(s: CharSequence): Unit = {
14+
var i = 0
15+
val length = s.length
16+
while (i < length) {
17+
val c = s.charAt(i)
18+
if (Character.isHighSurrogate(c)) {
19+
if (i + 1 >= length || !Character.isLowSurrogate(s.charAt(i + 1))) {
20+
throw new InvalidUnicodeString
21+
}
22+
i += 2
23+
} else if (Character.isLowSurrogate(c)) {
24+
throw new InvalidUnicodeString
25+
} else {
26+
i += 1
27+
}
28+
}
29+
}
30+
}
31+
1032
/** Parse JSON directly into a literal `Val` */
11-
class ValVisitor(pos: Position) extends JsVisitor[Val, Val] { self =>
33+
class ValVisitor(pos: Position, rejectUnpairedSurrogates: Boolean = false)
34+
extends JsVisitor[Val, Val] { self =>
1235

1336
override def visitJsonableObject(length: Int, index: Int): ObjVisitor[Val, Val] =
1437
visitObject(length, index)
@@ -27,7 +50,10 @@ class ValVisitor(pos: Position) extends JsVisitor[Val, Val] { self =>
2750
var key: String = _
2851
def subVisitor: Visitor[?, ?] = self
2952
def visitKey(index: Int): upickle.core.StringVisitor.type = upickle.core.StringVisitor
30-
def visitKeyValue(s: Any): Unit = key = s.toString
53+
def visitKeyValue(s: Any): Unit = {
54+
key = s.toString
55+
if (rejectUnpairedSurrogates) ValVisitor.rejectUnpairedSurrogates(key)
56+
}
3157
def visitValue(v: Val, index: Int): Unit = {
3258
cache.put(key, v)
3359
allKeys.put(key, false)
@@ -52,5 +78,8 @@ class ValVisitor(pos: Position) extends JsVisitor[Val, Val] { self =>
5278
}
5379
)
5480

55-
def visitString(s: CharSequence, index: Int): Val = Val.Str(pos, s.toString)
81+
def visitString(s: CharSequence, index: Int): Val = {
82+
if (rejectUnpairedSurrogates) ValVisitor.rejectUnpairedSurrogates(s)
83+
Val.Str(pos, s.toString)
84+
}
5685
}

sjsonnet/src/sjsonnet/stdlib/ManifestModule.scala

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,8 +126,13 @@ object ManifestModule extends AbstractFunctionModule {
126126
private object ParseJson extends Val.Builtin1("parseJson", "str") {
127127
def evalRhs(str: Eval, ev: EvalScope, pos: Position): Val = {
128128
try {
129-
ujson.StringParser.transform(str.value.asString, new ValVisitor(pos))
129+
ujson.StringParser.transform(
130+
str.value.asString,
131+
new ValVisitor(pos, rejectUnpairedSurrogates = true)
132+
)
130133
} catch {
134+
case _: ValVisitor.InvalidUnicodeString =>
135+
throw Error.fail("Invalid JSON: unpaired surrogate in string", pos)(ev)
131136
case e: ujson.ParseException =>
132137
throw Error.fail("Invalid JSON: " + e.getMessage, pos)(ev)
133138
case _: ujson.IncompleteParseException =>
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
std.parseJson('"\\uD800"')
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
sjsonnet.Error: [std.parseJson] Invalid JSON: unpaired surrogate in string
2+
at [<root>].(error.parsejson_lone_high_surrogate.jsonnet:1:14)
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
std.parseJson('"\\uDC00"')
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
sjsonnet.Error: [std.parseJson] Invalid JSON: unpaired surrogate in string
2+
at [<root>].(error.parsejson_lone_low_surrogate.jsonnet:1:14)
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
std.parseJson('{"\\uD800": 1}')
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
sjsonnet.Error: [std.parseJson] Invalid JSON: unpaired surrogate in string
2+
at [<root>].(error.parsejson_lone_surrogate_key.jsonnet:1:14)

0 commit comments

Comments
 (0)