commit 23499bca33023cbee34d7e0dae0ae335fe6a8026
parent 27b2a410ead5da69415c847eb0d362bc9aa08c4e
Author: Michael Camilleri <[email protected]>
Date: Wed, 1 Jul 2026 19:46:52 +0900
Fix Across Lite conversion crash, corruption and off-by-one error
Three related fixes to Across Lite (.puz) rebus handling:
- PUZToXDConverter keyed its grid placeholders by cell index rather than
by distinct rebus value, so the 13th key was `=` (breaking the Rebus:
grammar), the 16th `@` (reparsed as a circled cell), and the 207th
rebus-covered cell trapped on UInt8 overflow. Dedupe by value into
[String: Character], mirroring NYTToXDConverter, and draw from a
shared, reserved-char-avoiding placeholder set hoisted onto
XD.rebusPlaceholders (the writer-side counterpart to
XD.parseRebusHeader), with a bounds check that fails closed instead of
doing UInt8 arithmetic.
- parseRebus read the GRBS grid byte directly as the RTBL key, but .puz
stores GRBS = RTBL_key + 1 (0 = no rebus). On a real rebus file a
single-value rebus silently dropped every rebus cell and a multi-value
rebus mis-mapped each cell to the next value and dropped the last -
silent corruption, no crash, which is why it shipped. Confirmed
against the puz FileFormat wiki and puzpy. Fix: table[key - 1].
- adds GRBS/RTBL round-trip coverage plus a single-value decode
regression test.
Co-Authored-By: Claude Opus 4.8 <[email protected]>
Diffstat:
4 files changed, 194 insertions(+), 39 deletions(-)
diff --git a/Crossmate/Models/XD.swift b/Crossmate/Models/XD.swift
@@ -20,6 +20,28 @@ struct XD: Sendable {
/// regex, so a single oversized clue under the source cap is still costly.
static let maxClueTextLength = 1_024
+ /// Single-character grid placeholders for multi-letter (rebus) fills, used
+ /// by every converter that *writes* `.xd`. The grid is one character per
+ /// cell, so a cell whose fill is longer than one letter shows one of these
+ /// and is expanded via the `Rebus:` header (the `1` in `1=LW`). The spec
+ /// allows "digits, most symbols, and printable unicode characters" here; we
+ /// take digits first (the conventional, readable encoding) then the ASCII
+ /// symbols, where "most" excludes the characters the grid/header parser
+ /// already reserves — letters (grid fill), `#`/`_`/`.` (block/empty),
+ /// `@`/`*` (`Specials:` markers), and `=`/space (`Rebus:` `key=value`
+ /// syntax, split on whitespace by `parseRebusHeader`). We stop at ASCII
+ /// rather than walking into unicode; real puzzles use a handful of distinct
+ /// fills, nowhere near the ceiling. (Walking ASCII naïvely from `'1'`
+ /// overflows into `=` by the 13th key, producing an unparseable `==VALUE`
+ /// entry and a rejected grid char.)
+ static let rebusPlaceholders: [Character] = {
+ let reserved: Set<Character> = ["#", "_", ".", "@", "*", "=", " "]
+ let symbols = (UInt8(33)...UInt8(126))
+ .map { Character(UnicodeScalar($0)) }
+ .filter { !$0.isLetter && !$0.isNumber && !reserved.contains($0) }
+ return Array("123456789") + symbols
+ }()
+
let title: String?
let publisher: String?
let author: String?
diff --git a/Crossmate/Services/NYTToXDConverter.swift b/Crossmate/Services/NYTToXDConverter.swift
@@ -7,28 +7,6 @@ enum NYTToXDConverter {
var errorDescription: String? { message }
}
- /// Single-character grid placeholders for multi-letter (rebus) fills. The
- /// `.xd` grid is one character per cell, so a cell whose fill is longer than
- /// one letter shows one of these in the grid and is expanded via the
- /// `Rebus:` header (the `1` in `1=LW`). These are *not* NYT data — they're
- /// our `.xd` serialization. The `.xd` spec allows "digits, most symbols,
- /// and printable unicode characters (if needed)" here; we take digits first
- /// (the conventional, readable encoding) then the symbols, where "most"
- /// means every printable ASCII symbol *except* the ones the grid/header
- /// parser already reserves — letters (grid fill), `#`/`_`/`.` (block/empty),
- /// `@`/`*` (special markers), and `=`/space (`Rebus:` `key=value` syntax).
- /// We stop at ASCII rather than walking into unicode and throw past the
- /// ceiling; real puzzles use a handful of distinct fills, nowhere near it.
- /// (Walking ASCII naïvely from `'1'` overflows into `=` by the 13th key,
- /// which produced an unparseable `==VALUE` entry and a rejected grid char.)
- private static let rebusPlaceholders: [Character] = {
- let reserved: Set<Character> = ["#", "_", ".", "@", "*", "=", " "]
- let symbols = (UInt8(33)...UInt8(126))
- .map { Character(UnicodeScalar($0)) }
- .filter { !$0.isLetter && !$0.isNumber && !reserved.contains($0) }
- return Array("123456789") + symbols
- }()
-
/// Whether a cell fill must ride into the grid via a `Rebus:` placeholder
/// rather than appear literally. The `.xd` grid is one character per cell and
/// the grid parser only takes letters (plus block/special markers and
@@ -152,12 +130,12 @@ enum NYTToXDConverter {
for answer in answers {
guard let answer, needsRebusEncoding(answer) else { continue }
if rebusLookup[answer] == nil {
- guard rebusLookup.count < rebusPlaceholders.count else {
+ guard rebusLookup.count < XD.rebusPlaceholders.count else {
throw ConversionError(
message: "Too many distinct rebus fills (\(rebusLookup.count + 1)); ran out of grid placeholders."
)
}
- let key = rebusPlaceholders[rebusLookup.count]
+ let key = XD.rebusPlaceholders[rebusLookup.count]
rebusLookup[answer] = key
rebusEntries.append((key: key, value: answer))
}
diff --git a/Crossmate/Services/PUZToXDConverter.swift b/Crossmate/Services/PUZToXDConverter.swift
@@ -74,16 +74,32 @@ enum PUZToXDConverter {
clueTexts: clueTexts
)
+ // Each distinct rebus fill claims one grid placeholder from
+ // `XD.rebusPlaceholders`; cells sharing a fill reuse the same key
+ // (mirrors `NYTToXDConverter`). Keying by cell index instead would burn
+ // a fresh character per cell and walk straight into the grid/header
+ // reserved set (`=`, `@`, ...) and, past ~200 cells, off the end of the
+ // alphabet — hence the bounds check rather than raw arithmetic.
var rebusKeys: [Int: Character] = [:]
var rebusEntries: [(Character, String)] = []
- var nextRebusKey: UInt8 = Character("1").asciiValue!
+ var rebusLookup: [String: Character] = [:]
for index in 0..<cellCount where isOpen(solutionBytes[index]) {
guard let value = rebus[index], !value.isEmpty else { continue }
- guard rebusKeys[index] == nil else { continue }
- let key = Character(UnicodeScalar(nextRebusKey))
+ let normalized = value.uppercased()
+ let key: Character
+ if let existing = rebusLookup[normalized] {
+ key = existing
+ } else {
+ guard rebusLookup.count < XD.rebusPlaceholders.count else {
+ throw ConversionError(
+ message: "Across Lite puzzle has too many distinct rebus fills (\(rebusLookup.count + 1)); ran out of grid placeholders."
+ )
+ }
+ key = XD.rebusPlaceholders[rebusLookup.count]
+ rebusLookup[normalized] = key
+ rebusEntries.append((key, normalized))
+ }
rebusKeys[index] = key
- rebusEntries.append((key, value.uppercased()))
- nextRebusKey += 1
}
var metadata: [String] = []
@@ -321,8 +337,11 @@ enum PUZToXDConverter {
var rebus: [Int: String] = [:]
for offset in 0..<cellCount {
let gridIndex = grid.index(grid.startIndex, offsetBy: offset)
- let key = Int(grid[gridIndex])
- guard key > 0, let value = table[key] else { continue }
+ // The grid byte is 1-indexed: 0 means "no rebus" and `1 + n` refers
+ // to `RTBL` key `n` (see the .puz FileFormat reference). Subtract one
+ // to recover the table key.
+ let gridByte = Int(grid[gridIndex])
+ guard gridByte > 0, let value = table[gridByte - 1] else { continue }
rebus[offset] = value
}
return rebus
diff --git a/Tests/Unit/PUZToXDConverterTests.swift b/Tests/Unit/PUZToXDConverterTests.swift
@@ -140,6 +140,113 @@ struct PUZToXDConverterTests {
#expect(puzzle.cells[1][1].solution == "E")
}
+ @Test("Rebus fills convert, dedupe by value, and round-trip")
+ func rebusFillsConvertAndDedupe() throws {
+ // Cells 0 and 8 share the fill CAT; cell 4 is DOG. The two CAT cells
+ // must collapse to a single Rebus header entry and reuse one grid
+ // placeholder — keying by cell index (the old bug) would mint three.
+ let data = try puzData(
+ width: 3,
+ height: 3,
+ solution: "ABCDEFGHI",
+ title: "Rebus",
+ author: "",
+ copyright: "",
+ clues: [
+ "Across 1",
+ "Down 1",
+ "Down 2",
+ "Down 3",
+ "Across 4",
+ "Across 5"
+ ],
+ rebusGrid: [0: 0, 4: 1, 8: 0],
+ rebusTable: [0: "CAT", 1: "DOG"]
+ )
+
+ let source = try PUZToXDConverter.convert(puzData: data)
+ #expect(header("Rebus", in: source) == "1=CAT 2=DOG")
+ #expect(source.contains("\n1BC\nD2F\nGH1\n"))
+
+ let puzzle = Puzzle(xd: try XD.parse(source))
+ #expect(puzzle.cells[0][0].solution == "CAT")
+ #expect(puzzle.cells[1][1].solution == "DOG")
+ #expect(puzzle.cells[2][2].solution == "CAT")
+ }
+
+ @Test("A single rebus value at RTBL key 0 decodes rather than vanishing")
+ func singleRebusValueDecodes() throws {
+ // The common case: one rebus fill, `RTBL` key 0, so the `GRBS` byte is 1.
+ // Reading the grid byte directly as the table key (the off-by-one bug)
+ // looked up the absent key 1, silently dropping the rebus and leaving the
+ // cell's raw solution letter behind.
+ let data = try puzData(
+ width: 3,
+ height: 3,
+ solution: "ABCDEFGHI",
+ title: "Rebus",
+ author: "",
+ copyright: "",
+ clues: [
+ "Across 1",
+ "Down 1",
+ "Down 2",
+ "Down 3",
+ "Across 4",
+ "Across 5"
+ ],
+ rebusGrid: [4: 0],
+ rebusTable: [0: "HEART"]
+ )
+
+ let source = try PUZToXDConverter.convert(puzData: data)
+ #expect(header("Rebus", in: source) == "1=HEART")
+
+ let puzzle = Puzzle(xd: try XD.parse(source))
+ #expect(puzzle.cells[1][1].solution == "HEART")
+ }
+
+ @Test("Many distinct rebus fills never collide with reserved grid syntax")
+ func manyRebusFillsAvoidReservedKeys() throws {
+ // Keying placeholders by cell index used to reach '=' (the Rebus header
+ // delimiter) on the 13th rebus cell and '@' (the circle marker) on the
+ // 16th, and to trap on UInt8 overflow past ~207. Twenty distinct fills
+ // must all draw from the curated placeholder set and steer clear of
+ // every reserved grid/header character.
+ var grid: [Int: Int] = [:]
+ var table: [Int: String] = [:]
+ let values = (0..<20).map { "R\($0)X" }
+ for (offset, value) in values.enumerated() {
+ grid[offset] = offset
+ table[offset] = value
+ }
+
+ let data = try puzData(
+ width: 5,
+ height: 5,
+ solution: String(repeating: "A", count: 25),
+ title: "Rebus",
+ author: "",
+ copyright: "",
+ clues: Array(repeating: "Clue", count: 10),
+ rebusGrid: grid,
+ rebusTable: table
+ )
+
+ let source = try PUZToXDConverter.convert(puzData: data)
+ let rebus = try #require(header("Rebus", in: source))
+ for entry in rebus.split(separator: " ") {
+ let key = try #require(entry.first)
+ #expect(!["#", "_", ".", "@", "*", "="].contains(key))
+ }
+
+ // The whole puzzle now parses rather than throwing, with each distinct
+ // fill landing as its own multi-letter rebus solution.
+ let puzzle = Puzzle(xd: try XD.parse(source))
+ #expect(puzzle.cells[0][0].solution == "R0X")
+ #expect(puzzle.cells[3][4].solution == "R19X")
+ }
+
private func puzData(
width: UInt8,
height: UInt8,
@@ -149,7 +256,9 @@ struct PUZToXDConverterTests {
copyright: String,
clues: [String],
notes: String = "",
- circledCells: Set<Int> = []
+ circledCells: Set<Int> = [],
+ rebusGrid: [Int: Int] = [:],
+ rebusTable: [Int: String] = [:]
) throws -> Data {
let cellCount = Int(width) * Int(height)
let solutionBytes = Array(solution.utf8)
@@ -179,18 +288,45 @@ struct PUZToXDConverterTests {
for index in circledCells where index >= 0 && index < cellCount {
payload[index] = 0x80
}
- data.append(contentsOf: "GEXT".utf8)
- data.append(UInt8(cellCount & 0xFF))
- data.append(UInt8((cellCount >> 8) & 0xFF))
- data.append(0)
- data.append(0)
- data.append(contentsOf: payload)
- data.append(0)
+ appendExtension("GEXT", payload: payload, into: &data)
+ }
+ // Rebus grid (`GRBS`) + table (`RTBL`) follow the real .puz convention:
+ // `rebusGrid` maps a cell to its 0-based `RTBL` key, and the grid byte is
+ // stored 1-indexed (`key + 1`), reserving 0 for "no rebus".
+ if !rebusGrid.isEmpty {
+ var payload = Data(repeating: 0, count: cellCount)
+ for (index, key) in rebusGrid where index >= 0 && index < cellCount {
+ payload[index] = UInt8(key + 1)
+ }
+ appendExtension("GRBS", payload: payload, into: &data)
+
+ let table = rebusTable
+ .sorted { $0.key < $1.key }
+ .map { " \($0.key):\($0.value);" }
+ .joined()
+ appendExtension("RTBL", payload: try cp1252Data(table), into: &data)
}
return data
}
+ private func appendExtension(_ code: String, payload: Data, into data: inout Data) {
+ data.append(contentsOf: code.utf8)
+ data.append(UInt8(payload.count & 0xFF))
+ data.append(UInt8((payload.count >> 8) & 0xFF))
+ data.append(0)
+ data.append(0)
+ data.append(payload)
+ data.append(0)
+ }
+
private func cp1252Data(_ string: String) throws -> Data {
try #require(string.data(using: .windowsCP1252))
}
+
+ private func header(_ name: String, in xd: String) -> String? {
+ let prefix = "\(name): "
+ return xd.split(separator: "\n")
+ .first { $0.hasPrefix(prefix) }
+ .map { String($0.dropFirst(prefix.count)) }
+ }
}
