diff --git a/Sources/OOXMLSwift/Models/WordDocument+UpdateAllFields.swift b/Sources/OOXMLSwift/Models/WordDocument+UpdateAllFields.swift index 5ac041c..81ed769 100644 --- a/Sources/OOXMLSwift/Models/WordDocument+UpdateAllFields.swift +++ b/Sources/OOXMLSwift/Models/WordDocument+UpdateAllFields.swift @@ -298,7 +298,8 @@ extension WordDocument { // the SAME run that holds all 5 `` elements in its // `rawXML`. Use the regex-based `rewriteCachedResult` to splice // the new value into the embedded `...` between - // `separate` and `end`. + // `separate` and `end`. See `isBakedFormCachedRun(_:)` for the + // discriminator invariant pinned by tests (#33). // // 2. Canonical 5-run form (post-roundtrip / native Word): // `cachedResultRunIdx` points to a DEDICATED run whose only @@ -310,7 +311,7 @@ extension WordDocument { // update `Run.text` directly — no regex needed. if let idx = field.cachedResultRunIdx, idx < para.runs.count { let cachedRun = para.runs[idx] - let isBakedForm = (cachedRun.rawXML?.contains("fldChar") ?? false) + let isBakedForm = isBakedFormCachedRun(cachedRun) if isBakedForm { let oldXML = cachedRun.rawXML ?? "" @@ -370,6 +371,19 @@ extension WordDocument { return rewroteSomething } + /// Returns true for the v2.0.0 baked SEQ emission form where the parsed + /// field's cached-result run is the same `Run.rawXML` that embeds the full + /// begin/instrText/separate/cached/end field block. + /// + /// In canonical 5-run form, `cachedResultRunIdx` points to a dedicated + /// cached-value run. That run may have `...` rawXML, but it must + /// not contain `fldChar`. This deliberately narrow discriminator prevents + /// canonical cached runs from being routed through the baked-form regex + /// rewrite path (#33). + private func isBakedFormCachedRun(_ run: Run) -> Bool { + run.rawXML?.contains("fldChar") ?? false + } + /// Returns heading level (1-9) if paragraph has `pStyle == "Heading N"`, else nil. private func headingLevel(of para: Paragraph) -> Int? { guard let style = para.properties.style else { return nil } diff --git a/Sources/OOXMLSwift/Parsing/FieldParser.swift b/Sources/OOXMLSwift/Parsing/FieldParser.swift index 5e26b03..d396319 100644 --- a/Sources/OOXMLSwift/Parsing/FieldParser.swift +++ b/Sources/OOXMLSwift/Parsing/FieldParser.swift @@ -153,55 +153,88 @@ public enum FieldParser { options: [.dotMatchesLineSeparators] ) - func extractInstrText(_ rawXML: String) -> String? { + func extractInstrText(in fragment: String) -> String? { guard let regex = instrTextRegex else { return nil } - let nsRange = NSRange(rawXML.startIndex..., in: rawXML) - guard let match = regex.firstMatch(in: rawXML, options: [], range: nsRange), + let nsRange = NSRange(fragment.startIndex..., in: fragment) + guard let match = regex.firstMatch(in: fragment, options: [], range: nsRange), match.numberOfRanges >= 2, - let innerRange = Range(match.range(at: 1), in: rawXML) else { return nil } - return String(rawXML[innerRange]) + let innerRange = Range(match.range(at: 1), in: fragment) else { return nil } + return String(fragment[innerRange]) + } + + struct RunSignals { + var hasFldCharBegin = false + var hasFldCharSeparate = false + var hasFldCharEnd = false + var hasInstrText = false + var hasText = false + var instrText: String? } // Probe a single run for fldChar/instrText fragments. DocxReader stores // unrecognized run children (including `` and // ``) in `Run.rawElements` (NOT `Run.rawXML`). Native-Word // 5-run paragraphs constructed by hand may instead embed the fragment - // directly in `Run.rawXML`. Check both surfaces. - func runFragments(_ run: Run) -> String { - var pieces: [String] = [] - if let rawXML = run.rawXML { pieces.append(rawXML) } + // directly in `Run.rawXML`. Scan both surfaces once without joining + // them into a transient per-run string (#32). + func scanFragment(_ fragment: String, into signals: inout RunSignals) { + if !signals.hasFldCharBegin { + signals.hasFldCharBegin = fragment.contains("fldCharType=\"begin\"") + } + if !signals.hasFldCharSeparate { + signals.hasFldCharSeparate = fragment.contains("fldCharType=\"separate\"") + } + if !signals.hasFldCharEnd { + signals.hasFldCharEnd = fragment.contains("fldCharType=\"end\"") + } + if !signals.hasInstrText { + signals.hasInstrText = fragment.contains(" RunSignals { + var signals = RunSignals() + if let rawXML = run.rawXML { + scanFragment(rawXML, into: &signals) + } if let elems = run.rawElements { for elem in elems { - pieces.append(elem.xml) + scanFragment(elem.xml, into: &signals) } } - return pieces.joined() + if !signals.hasText, + run.rawXML == nil, + (run.rawElements?.isEmpty ?? true), + !run.text.isEmpty { + signals.hasText = true + } + return signals } for (idx, run) in paragraph.runs.enumerated() { - let fragments = runFragments(run) + let signals = scanRun(run) // Caption text runs (only ``) carry no fldChar/instrText // signal. Treat them as neutral — they neither advance nor reset // the in-progress span (caption text interleaved with fldChar runs // is normal). But a run with `` text right after `separate` // IS the cached value, so check that case below. - let hasFldCharBegin = fragments.contains("fldCharType=\"begin\"") - let hasFldCharSeparate = fragments.contains("fldCharType=\"separate\"") - let hasFldCharEnd = fragments.contains("fldCharType=\"end\"") - let hasInstrText = fragments.contains("` is in `recognizedRunChildren`). if let span = current, span.separateRunIdx != nil, span.cachedRunIdx == nil, - hasText { + signals.hasText { var updated = span updated.cachedRunIdx = idx current = updated @@ -229,7 +262,7 @@ public enum FieldParser { } // fldChar end: emit span and reset - if hasFldCharEnd, let span = current { + if signals.hasFldCharEnd, let span = current { if let instrText = span.instrText { let parsedValue = dispatchParse(instrText: instrText) result.append(ParsedField( diff --git a/Tests/OOXMLSwiftTests/Issue104FieldParserCanonicalFormTests.swift b/Tests/OOXMLSwiftTests/Issue104FieldParserCanonicalFormTests.swift index 0dc6ac9..0114ab7 100644 --- a/Tests/OOXMLSwiftTests/Issue104FieldParserCanonicalFormTests.swift +++ b/Tests/OOXMLSwiftTests/Issue104FieldParserCanonicalFormTests.swift @@ -67,6 +67,95 @@ final class Issue104FieldParserCanonicalFormTests: XCTestCase { "ParsedField should expose identifier 'Figure' from instrText 'SEQ Figure \\* ARABIC'") } + // MARK: - 104.1.1 Baked-vs-canonical discriminator invariant (#33) + + /// `wrapCaptionSequenceFields` emits the baked form before save: the parsed + /// cached-result run is the same run whose rawXML embeds the full field + /// block, including fldChar markers. `updateAllFields` depends on this + /// invariant to choose the baked rewrite path (#33). + func testBakedFormCachedRunRawXMLAlwaysContainsFldCharBeforeSave() throws { + var doc = WordDocument() + doc.body.children = [ + .paragraph(Paragraph(text: "Figure 7. Distribution")) + ] + let pattern = try NSRegularExpression(pattern: "Figure (\\d+)\\.") + _ = try doc.wrapCaptionSequenceFields(pattern: pattern, sequenceName: "Figure") + + guard case .paragraph(let paragraph) = doc.body.children[0] else { + return XCTFail("expected paragraph") + } + + let fields = FieldParser.parse(paragraph: paragraph) + XCTAssertEqual(fields.count, 1) + guard let field = fields.first, + let cachedRunIdx = field.cachedResultRunIdx else { + return XCTFail("expected parsed field with cached result run") + } + + XCTAssertEqual(field.startRunIdx, cachedRunIdx, + "baked form should report the same run for field start and cached result") + let cachedRawXML = paragraph.runs[cachedRunIdx].rawXML + XCTAssertNotNil(cachedRawXML) + XCTAssertTrue(cachedRawXML?.contains("fldChar") ?? false, + "baked-form cached run must carry fldChar markers for the discriminator") + } + + /// After Writer -> Reader roundtrip, the same field is canonical 5-run + /// form: cached-result run is a dedicated value run and must not contain + /// fldChar. This prevents the baked-form discriminator from accidentally + /// routing canonical fields through the rawXML field-block regex (#33). + func testRoundTripCanonicalFormCachedRunRawXMLDoesNotContainFldChar() throws { + let tempDir = FileManager.default.temporaryDirectory + .appendingPathComponent("Issue33RoundTripCanonical-\(UUID().uuidString)") + try FileManager.default.createDirectory(at: tempDir, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: tempDir) } + + var doc = WordDocument() + doc.body.children = [ + .paragraph(Paragraph(text: "Figure 7. Distribution")) + ] + let pattern = try NSRegularExpression(pattern: "Figure (\\d+)\\.") + _ = try doc.wrapCaptionSequenceFields(pattern: pattern, sequenceName: "Figure") + + let docxURL = tempDir.appendingPathComponent("test.docx") + try DocxWriter.write(doc, to: docxURL) + let reloaded = try DocxReader.read(from: docxURL) + + guard case .paragraph(let paragraph) = reloaded.body.children[0] else { + return XCTFail("expected paragraph after roundtrip") + } + + let fields = FieldParser.parse(paragraph: paragraph) + XCTAssertEqual(fields.count, 1) + guard let field = fields.first, + let cachedRunIdx = field.cachedResultRunIdx else { + return XCTFail("expected parsed field with cached result run") + } + + XCTAssertNotEqual(field.startRunIdx, cachedRunIdx, + "canonical form should use a dedicated cached-result run") + XCTAssertFalse(paragraph.runs[cachedRunIdx].rawXML?.contains("fldChar") ?? false, + "canonical cached run must not look like a baked field block") + } + + // MARK: - 104.1.2 Defensive large-paragraph scan (#32) + + /// DoS regression guard for #32: large paragraphs with many runs should not + /// allocate a joined fragment string per run. The parser should complete a + /// 100k-run empty paragraph within a conservative local-test budget. + func testFieldParserHandlesHundredThousandEmptyRunsWithinBudget() { + var paragraph = Paragraph() + paragraph.runs = Array(repeating: Run(text: ""), count: 100_000) + + let started = Date() + let fields = FieldParser.parse(paragraph: paragraph) + let elapsed = Date().timeIntervalSince(started) + + XCTAssertTrue(fields.isEmpty) + XCTAssertLessThan(elapsed, 5.0, + "FieldParser.parse should not do expensive per-run fragment joining for empty runs") + } + // MARK: - 104.2 End-to-end: updateAllFields finds and updates SEQ after roundtrip /// Drives the actual MCP user scenario: roundtrip → `updateAllFields()`