Skip to content

Round trip corrupts keys containing a line terminator: unflatten(flatten(x)) != x #105

Description

@Namron2000

Version: 0.18.2 (current release, reproduced against the published jar)
Class: com.github.wnameless.json.unflattener.JsonUnflattener

A key that contains a line terminator and array-index syntax survives flatten()
but is torn apart by unflatten(), turning one key into a nested object inside an array.

{"\r[7]": false}
String json = "{\"\\r[7]\": false}";
String flat = new JsonFlattener(json).withFlattenMode(FlattenMode.KEEP_ARRAYS).flatten();
// {"[\"\r[7]\"]":false}                     <- correct, the key is bracket-quoted
String back = new JsonUnflattener(flat).withFlattenMode(FlattenMode.KEEP_ARRAYS).unflatten();
// {"\r":[null,null,null,null,null,null,null,false]}   <- wrong

The flattener does its part correctly: it wraps the complex key as ["…"]. The
unflattener fails to recognise that wrapper and instead splits the key at [7], treating
it as an array index.

Found by the project's own RoundTripFuzzer (src/test/java/com/example/) via OSS-Fuzz —
Round-trip key count mismatch (mode=KEEP_ARRAYS sep='?'): 1 vs 2.

The README's own example breaks with one added character

This is not an exotic corner: the README documents key-with-reserved-characters support,
and that example fails as soon as the key also contains a newline. Plain static API, all
defaults:

// verbatim from the README
JsonFlattener.flatten("[{\"a.a.[\":1},2,{\"c\":[3,4]}]");
//   flattened: {"[0][\"a.a.[\"]":1,"[1]":2,"[2].c[0]":3,"[2].c[1]":4}
//   restored : [{"a.a.[":1},2,{"c":[3,4]}]          <- round trip preserved

// the same, with \r added to the key
JsonFlattener.flatten("[{\"a.a.[\\r\":1},2,{\"c\":[3,4]}]");
//   flattened: {"[0][\"a.a.[\r\"]":1,...}
//   restored : [{"\"a":{"a":{"\r\"":1}}},2,{"c":[3,4]}]   <- one key became three levels

Note the key must use the escaped \r form — a literal CR in JSON text is illegal and
the parser rejects it before the flattener sees it.

Cause

JsonUnflattener.objectComplexKeyPattern() (line 209):

String regex = Pattern.quote(leftBracket.toString()) + "\\s*\".*?\"\\s*"
    + Pattern.quote(rightBracket.toString());

In Java, . does not match line terminators unless Pattern.DOTALL is set. So
["…"] fails to match whenever the key contains one, keyPartPattern() falls through to
arrayIndexPattern() + objectKeyPattern(), and the key is split as if [7] were
structure.

This predicts that exactly Java's five line terminators break it, and that is what
happens:

character round trip
U+000A LF broken
U+000D CR broken
U+0085 NEL broken
U+2028 LS broken
U+2029 PS broken
U+0009 TAB, U+000B, U+000C, U+0020, U+001C, U+00A0, U+2007 ok

Affects NORMAL, KEEP_ARRAYS and KEEP_PRIMITIVE_ARRAYS (and MONGODB, which has a
separate problem — see below). The separator character is irrelevant.

Fix

--- a/src/main/java/com/github/wnameless/json/unflattener/JsonUnflattener.java
+++ b/src/main/java/com/github/wnameless/json/unflattener/JsonUnflattener.java
@@ -207,7 +207,12 @@
   private Pattern objectComplexKeyPattern() {
-    String regex = Pattern.quote(leftBracket.toString()) + "\\s*\".*?\"\\s*"
+    // [\s\S] rather than "." : "." does not match line terminators, so a complex key
+    // containing CR, LF, NEL, LS or PS failed to match here and was split as if it were a
+    // plain key. A character class is used rather than Pattern.DOTALL because
+    // keyPartPattern() concatenates these pattern *strings* and recompiles them, which
+    // would discard a compile flag.
+    String regex = Pattern.quote(leftBracket.toString()) + "\\s*\"[\\s\\S]*?\"\\s*"
         + Pattern.quote(rightBracket.toString());

Note on why not Pattern.DOTALL. The obvious fix — passing the flag to
Pattern.compile here — does not work, and it is worth knowing why:
keyPartPattern() builds its combined pattern from the strings of the three
sub-patterns and recompiles them:

String regex = arrayIndexPattern().pattern() + "|" + objectComplexKeyPattern().pattern()
    + "|" + objectKeyPattern().pattern();
patternCache.put(regex, Pattern.compile(regex));

Pattern.pattern() returns only the regex text, so any compile flag set on the
sub-pattern is silently dropped. Embedding the behaviour in the regex itself — via
[\s\S] (or an inline (?s:…) group) — is what survives that concatenation. I tried
DOTALL first and the tests still failed, which is how this surfaced.

Verified against 0.18.2: all five line terminators now round-trip correctly in NORMAL,
KEEP_ARRAYS and KEEP_PRIMITIVE_ARRAYS, as does the original fuzzer input. Unaffected
by the change: keys a[7], [7], \t[7], [7], a.b, a"b, plain, \r,
\rplain, a\rb, and documents {"a":{"b":1}}, {"a":[1,2,3]}, [1,2,3],
{"a.b":{"c":1}}, {}.

Separately: MONGODB mode corrupts keys much more broadly

MONGODB mode takes a different branch in keyPartPattern() and is unaffected by the
fix above. It silently damages keys that have nothing to do with line terminators:

key unflatten(flatten(key))
a[7] {"a[7": false} — closing bracket dropped
\rx {"x": false} — the CR is dropped from the key
\r[7] [null, …, false] — the object became an array

Its pattern is \b[^<sep>\s]+\b|…, which excludes whitespace and relies on word
boundaries, so whitespace inside keys is lost and bracket characters are mishandled. That
looks like a distinct defect worth its own issue; I have not attempted a fix for it.

Impact

Silent data corruption rather than an exception: unflatten(flatten(x)) returns a
different document, with one key becoming a nested structure and the value relocated. Any
code using this pair as a lossless transform — the documented purpose — can lose or
mangle data when a key contains a newline, which is legal JSON and not exotic in
practice (keys copied from user input, CSV headers, log fields).


Best regards,
The Fandango Cispa Team

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions