From 3b1d808c73db4a4c17c4e7d2fb2c62e676840999 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Wed, 2 Sep 2026 15:04:38 +0200 Subject: [PATCH] [Oracle] Fix nesting-depth cap, tolerate unescaped control chars, lazy-parse review fixes --- oracle/src/oracleimporter.cpp | 1 - oracle/src/raw_json_scanner.cpp | 16 +++++++-------- tests/oracle/oracle_importer_test.cpp | 29 +++++++++++++++++++++++++++ 3 files changed, 37 insertions(+), 9 deletions(-) diff --git a/oracle/src/oracleimporter.cpp b/oracle/src/oracleimporter.cpp index 6a5633215..d745b250c 100644 --- a/oracle/src/oracleimporter.cpp +++ b/oracle/src/oracleimporter.cpp @@ -551,7 +551,6 @@ int OracleImporter::startImport() { static ICardSetPriorityController *noOpController = new NoopCardSetPriorityController(); -<<<<<<< HEAD // Pre-allocate the cards hash to avoid rehashing during import. Keys are // distinct card names while raw ranges only count printings (AllPrintings // ~100k printings vs ~35k names), so this over-reserves somewhat; an exact diff --git a/oracle/src/raw_json_scanner.cpp b/oracle/src/raw_json_scanner.cpp index 01ea3fab5..db972491c 100644 --- a/oracle/src/raw_json_scanner.cpp +++ b/oracle/src/raw_json_scanner.cpp @@ -213,9 +213,10 @@ bool decodeString(const char *&p, const char *end, QString &out) flush(); return true; } - if (static_cast(c) < 0x20) { - return false; // unescaped control character is invalid JSON - } + // Deliberately accept unescaped control characters (e.g. a tab inside + // a set name): QJsonDocument and skipString accept them too, so + // rejecting them here would fail the whole document on a byte that + // Qt is fine with — the very total-failure mode this scanner avoids. utf8 += c; ++p; } @@ -399,20 +400,19 @@ bool skipArray(const char *&p, const char *end, int depth) bool skipValue(const char *&p, const char *end, int depth) { - if (depth <= 0) { - return false; // nest deeper than the cap - } p = skipWhitespace(p, end); if (p >= end) { return false; } const char c = *p; if (c == '{') { - return skipObject(p, end, depth - 1); + // pass depth through: skipObject consumes the single decrement for this level + return skipObject(p, end, depth); } if (c == '[') { - return skipArray(p, end, depth - 1); + return skipArray(p, end, depth); } + // a primitive is a leaf, so it never wastes a nesting level return skipPrimitive(p, end); } diff --git a/tests/oracle/oracle_importer_test.cpp b/tests/oracle/oracle_importer_test.cpp index c42840f7e..3834be9ed 100644 --- a/tests/oracle/oracle_importer_test.cpp +++ b/tests/oracle/oracle_importer_test.cpp @@ -634,6 +634,9 @@ TEST_F(OracleImporterTest, ScanSetRangesMatchesFullJsonParseVerdicts) "{\"data\":{\"A\":{\"code\":\"a\",\"name\":\"ok\",\"type\":null,\"releaseDate\":\"2024-01-01\",\"cards\":[]}}}", "{\"data\":{\"A\":{\"code\":\"a\",\"name\":\"ok\",\"type\":7,\"releaseDate\":\"2024-01-01\",\"cards\":null}}}", "{\"data\":{\"A\":{\"code\":\"a\",\"name\":\"ok\",\"releaseDate\":\"2024-01-01\",\"cards\":[1,2,3]}}}", + // unescaped control character inside a string: QJsonDocument and + // skipString both accept it, so the scanner must not reject the whole doc + "{\"data\":{\"A\":{\"code\":\"a\",\"name\":\"N\tX\",\"releaseDate\":\"2024-01-01\",\"cards\":[{\"n\":1}]}}}", // structurally invalid JSON (both parsers must reject) "not json", "{\"data\":{\"A\":{\"name\":\"unterminated}}", @@ -683,6 +686,32 @@ TEST_F(OracleImporterTest, ScanSetRangesRejectsDeepNesting) ASSERT_TRUE(scanError.isError()) << "scanner accepted a document Qt rejects as too deeply nested"; } +TEST_F(OracleImporterTest, ScanSetRangesAcceptsQtMaxNesting) +{ + // Pins the boundary rather than only the far-past case: a depth Qt still + // accepts must be accepted by the scanner too. Before the fix the scanner's + // cap was roughly half of Qt's (each level cost two decrements), so a + // depth of 1000 here was rejected even though QJsonDocument parses it. + constexpr int depth = 1000; + QString nesting; + nesting.reserve(2 * depth); + for (int i = 0; i < depth; ++i) { + nesting += '['; + } + for (int i = 0; i < depth; ++i) { + nesting += ']'; + } + const QByteArray json = ("{\"data\":{\"A\":{\"code\":\"a\",\"cards\":" + nesting + "}}}").toUtf8(); + + QJsonParseError qtError; + QJsonDocument::fromJson(json, &qtError); + ASSERT_EQ(qtError.error, QJsonParseError::NoError) << "expected Qt to accept depth " << depth; + + RawJson::ScanError scanError; + RawJson::scanSetRanges(json, &scanError); + ASSERT_FALSE(scanError.isError()) << "scanner rejected a document Qt accepts at depth " << depth; +} + // ============================================================================ // Lazy per-set parsing tests // ============================================================================