From 13a58139d418089cd3cf8f3b7503d1227f2dff69 Mon Sep 17 00:00:00 2001 From: DemchaAV Date: Wed, 23 Sep 2026 01:34:50 +0100 Subject: [PATCH] chore(docx): stop parsing values the export wrote itself MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeQL flagged each of these as an uncaught NumberFormatException or a deprecated call as they landed, and none had been cleared. The font obfuscation key is taken from the UUID's bits rather than from its printed form — the GUID's digits read backwards by pair are its sixteen bytes read backwards — and the published example's key bytes still match. A shading colour is read from the three bytes XmlBeans hands a written RGB back as, rather than formatted to text and parsed. The hyperlink run goes through POI's IRunBody constructor and a run's paragraph through getParent(), both in place of deprecated overloads. The tests read twips back through one helper, DocxTwips.of, which asserts the value is the number the export writes instead of each helper parsing its own copy of the text, and the probe fixtures use RowBuilder.spacing where they used the deprecated gap it delegates to. --- .../semantic/docx/DocxFontEmbedding.java | 12 ++++--- .../semantic/docx/DocxSemanticBackend.java | 19 ++++++------ .../semantic/docx/DocxCellPaddingTest.java | 2 +- .../semantic/docx/DocxComposedCellTest.java | 8 ++--- .../docx/DocxHangingIndentIsIgnoredTest.java | 2 +- .../semantic/docx/DocxLineHeightTest.java | 2 +- .../docx/DocxListGeometryFreezeTest.java | 2 +- .../docx/DocxPageZonePositionTest.java | 6 ++-- .../semantic/docx/DocxRowLayoutTest.java | 6 ++-- .../semantic/docx/DocxTableWidthTest.java | 2 +- .../backend/semantic/docx/DocxTwips.java | 31 +++++++++++++++++++ .../docx/DocxVerticalSpacingTest.java | 4 +-- .../docx/probe/EditableExportFixtures.java | 4 +-- 13 files changed, 67 insertions(+), 33 deletions(-) create mode 100644 render-docx/src/test/java/com/demcha/compose/document/backend/semantic/docx/DocxTwips.java diff --git a/render-docx/src/main/java/com/demcha/compose/document/backend/semantic/docx/DocxFontEmbedding.java b/render-docx/src/main/java/com/demcha/compose/document/backend/semantic/docx/DocxFontEmbedding.java index 108fe2bdf..00bffb656 100644 --- a/render-docx/src/main/java/com/demcha/compose/document/backend/semantic/docx/DocxFontEmbedding.java +++ b/render-docx/src/main/java/com/demcha/compose/document/backend/semantic/docx/DocxFontEmbedding.java @@ -152,11 +152,15 @@ static Obfuscated obfuscate(byte[] font, UUID uuid) { * @return the key the first bytes of the font are exclusive-ored against */ static byte[] keyOf(UUID uuid) { - String hex = uuid.toString().replace("-", ""); + // The GUID's digits read backwards by pair are its sixteen bytes read backwards: the + // low half's least significant byte first. Taken from the bits rather than from the + // printed form, so there is no text to parse. + long most = uuid.getMostSignificantBits(); + long least = uuid.getLeastSignificantBits(); byte[] key = new byte[16]; - for (int index = 0; index < key.length; index++) { - int at = hex.length() - 2 - index * 2; - key[index] = (byte) Integer.parseInt(hex.substring(at, at + 2), 16); + for (int index = 0; index < 8; index++) { + key[index] = (byte) (least >>> (8 * index)); + key[8 + index] = (byte) (most >>> (8 * index)); } return key; } diff --git a/render-docx/src/main/java/com/demcha/compose/document/backend/semantic/docx/DocxSemanticBackend.java b/render-docx/src/main/java/com/demcha/compose/document/backend/semantic/docx/DocxSemanticBackend.java index 99844e146..35f4e3e71 100644 --- a/render-docx/src/main/java/com/demcha/compose/document/backend/semantic/docx/DocxSemanticBackend.java +++ b/render-docx/src/main/java/com/demcha/compose/document/backend/semantic/docx/DocxSemanticBackend.java @@ -51,6 +51,7 @@ import com.demcha.compose.font.FontName; import org.apache.poi.util.Units; import org.apache.poi.xwpf.usermodel.BreakType; +import org.apache.poi.xwpf.usermodel.IRunBody; import org.apache.poi.xwpf.usermodel.ParagraphAlignment; import com.demcha.compose.document.node.PageFieldKind; import com.demcha.compose.document.node.PageFieldNode; @@ -1787,7 +1788,7 @@ private static String chipLost(InlineBackground background) { * unstyled cell holding a chip would come away carrying an empty one.

*/ private java.awt.Color colourUnder(XWPFRun run) { - XWPFParagraph para = run.getParagraph(); + XWPFParagraph para = run.getParent() instanceof XWPFParagraph parent ? parent : null; CTPPr paragraphProperties = para == null || !para.getCTP().isSetPPr() ? null : para.getCTP().getPPr(); @@ -1813,17 +1814,13 @@ private java.awt.Color colourUnder(XWPFRun run) { * identity, which parses as no colour at all and silently flattens against white.

*/ private static java.awt.Color shadingFillOf(CTShd shading) { + // A written RGB comes back as its three bytes. Anything else — Word's "auto", or a + // value this export did not write — is no colour it can composite against. Object fill = shading == null ? null : shading.getFill(); - if (fill == null) { + if (!(fill instanceof byte[] rgb) || rgb.length != 3) { return null; } - String hex = fill instanceof byte[] bytes - ? java.util.HexFormat.of().formatHex(bytes) - : String.valueOf(fill).trim(); - if (!hex.matches("(?i)[0-9a-f]{6}")) { - return null; - } - return new java.awt.Color(Integer.parseInt(hex, 16)); + return new java.awt.Color(rgb[0] & 0xFF, rgb[1] & 0xFF, rgb[2] & 0xFF); } /** @@ -1876,7 +1873,9 @@ private XWPFRun newRun(XWPFParagraph para, DocumentLinkTarget target) { if (name != null) { CTHyperlink link = para.getCTP().addNewHyperlink(); link.setAnchor(name); - return new XWPFRun(link.addNewR(), para); + // Through the IRunBody constructor: the XWPFParagraph overload is deprecated, + // and an unqualified paragraph argument would pick it. + return new XWPFRun(link.addNewR(), (IRunBody) para); } } return para.createRun(); diff --git a/render-docx/src/test/java/com/demcha/compose/document/backend/semantic/docx/DocxCellPaddingTest.java b/render-docx/src/test/java/com/demcha/compose/document/backend/semantic/docx/DocxCellPaddingTest.java index f90a8174d..78217aac1 100644 --- a/render-docx/src/test/java/com/demcha/compose/document/backend/semantic/docx/DocxCellPaddingTest.java +++ b/render-docx/src/test/java/com/demcha/compose/document/backend/semantic/docx/DocxCellPaddingTest.java @@ -110,7 +110,7 @@ private static long[] margins(XWPFTableCell cell) { private static long width(CTTblWidth margin) { return margin == null || margin.getW() == null ? -1 - : Long.parseLong(String.valueOf(margin.getW())); + : DocxTwips.of(margin.getW()); } private static XWPFTableCell firstCell(Consumer content) throws Exception { diff --git a/render-docx/src/test/java/com/demcha/compose/document/backend/semantic/docx/DocxComposedCellTest.java b/render-docx/src/test/java/com/demcha/compose/document/backend/semantic/docx/DocxComposedCellTest.java index f344bfdec..cbdd8ac44 100644 --- a/render-docx/src/test/java/com/demcha/compose/document/backend/semantic/docx/DocxComposedCellTest.java +++ b/render-docx/src/test/java/com/demcha/compose/document/backend/semantic/docx/DocxComposedCellTest.java @@ -101,10 +101,10 @@ void aNestedTableIsGivenTheWidthOfTheColumnItSitsIn() throws Exception { .rowCells(DocumentTableCell.text("Breakdown"), DocumentTableCell.node(innerTable())))); - long column = Long.parseLong(String.valueOf(cell.getTableRow().getTable() - .getCTTbl().getTblGrid().getGridColArray(1).getW())); - long nested = Long.parseLong(String.valueOf(cell.getTables().get(0) - .getCTTbl().getTblPr().getTblW().getW())); + long column = DocxTwips.of(cell.getTableRow().getTable() + .getCTTbl().getTblGrid().getGridColArray(1).getW()); + long nested = DocxTwips.of(cell.getTables().get(0) + .getCTTbl().getTblPr().getTblW().getW()); assertThat(cell.getTables().get(0).getCTTbl().getTblPr().getTblW().getType().toString()) .as("stated, not left to Word") diff --git a/render-docx/src/test/java/com/demcha/compose/document/backend/semantic/docx/DocxHangingIndentIsIgnoredTest.java b/render-docx/src/test/java/com/demcha/compose/document/backend/semantic/docx/DocxHangingIndentIsIgnoredTest.java index 2f606716b..e854d3187 100644 --- a/render-docx/src/test/java/com/demcha/compose/document/backend/semantic/docx/DocxHangingIndentIsIgnoredTest.java +++ b/render-docx/src/test/java/com/demcha/compose/document/backend/semantic/docx/DocxHangingIndentIsIgnoredTest.java @@ -234,7 +234,7 @@ private static List indentsOf(XWPFDocument document) { * {@code Object} and hands back whichever member matched.

*/ private static int twips(Object measure) { - return Integer.parseInt(String.valueOf(measure)); + return (int) DocxTwips.of(measure); } private static XWPFDocument export(Consumer author) throws Exception { diff --git a/render-docx/src/test/java/com/demcha/compose/document/backend/semantic/docx/DocxLineHeightTest.java b/render-docx/src/test/java/com/demcha/compose/document/backend/semantic/docx/DocxLineHeightTest.java index f6b5f2676..004bdeaa2 100644 --- a/render-docx/src/test/java/com/demcha/compose/document/backend/semantic/docx/DocxLineHeightTest.java +++ b/render-docx/src/test/java/com/demcha/compose/document/backend/semantic/docx/DocxLineHeightTest.java @@ -128,7 +128,7 @@ private static XWPFDocument withLayout(Consumer content) throws } private static long lineTwips(XWPFParagraph paragraph) { - return Long.parseLong(String.valueOf(paragraph.getCTP().getPPr().getSpacing().getLine())); + return DocxTwips.of(paragraph.getCTP().getPPr().getSpacing().getLine()); } private static String lineRule(XWPFParagraph paragraph) { diff --git a/render-docx/src/test/java/com/demcha/compose/document/backend/semantic/docx/DocxListGeometryFreezeTest.java b/render-docx/src/test/java/com/demcha/compose/document/backend/semantic/docx/DocxListGeometryFreezeTest.java index 7a9da0b03..eec4a8fb9 100644 --- a/render-docx/src/test/java/com/demcha/compose/document/backend/semantic/docx/DocxListGeometryFreezeTest.java +++ b/render-docx/src/test/java/com/demcha/compose/document/backend/semantic/docx/DocxListGeometryFreezeTest.java @@ -127,7 +127,7 @@ void oneParagraphPerItemRegardlessOfLengthBecauseDocxDoesNotWrap() throws Except * {@code Object} and hands back whichever member matched.

*/ private static int twips(Object measure) { - return Integer.parseInt(String.valueOf(measure)); + return (int) DocxTwips.of(measure); } /** True when a line begins with an ASCII space or carries a non-breaking one. */ diff --git a/render-docx/src/test/java/com/demcha/compose/document/backend/semantic/docx/DocxPageZonePositionTest.java b/render-docx/src/test/java/com/demcha/compose/document/backend/semantic/docx/DocxPageZonePositionTest.java index f8f4c75d1..84510a668 100644 --- a/render-docx/src/test/java/com/demcha/compose/document/backend/semantic/docx/DocxPageZonePositionTest.java +++ b/render-docx/src/test/java/com/demcha/compose/document/backend/semantic/docx/DocxPageZonePositionTest.java @@ -78,17 +78,17 @@ private static DocumentPageZone zone(DocumentHeaderFooterZone kind, double heigh private static long footerDistance(DocumentPageZone zone) throws Exception { CTPageMar margin = marginOf(exportWithLayout(zone)); - return margin.getFooter() == null ? -1 : Long.parseLong(String.valueOf(margin.getFooter())); + return margin.getFooter() == null ? -1 : DocxTwips.of(margin.getFooter()); } private static long headerDistance(DocumentPageZone zone) throws Exception { CTPageMar margin = marginOf(exportWithLayout(zone)); - return margin.getHeader() == null ? -1 : Long.parseLong(String.valueOf(margin.getHeader())); + return margin.getHeader() == null ? -1 : DocxTwips.of(margin.getHeader()); } private static long footerDistanceWithoutLayout(DocumentPageZone zone) throws Exception { CTPageMar margin = marginOf(exportWithoutLayout(zone)); - return margin.getFooter() == null ? -1 : Long.parseLong(String.valueOf(margin.getFooter())); + return margin.getFooter() == null ? -1 : DocxTwips.of(margin.getFooter()); } private static CTPageMar marginOf(byte[] docx) throws Exception { diff --git a/render-docx/src/test/java/com/demcha/compose/document/backend/semantic/docx/DocxRowLayoutTest.java b/render-docx/src/test/java/com/demcha/compose/document/backend/semantic/docx/DocxRowLayoutTest.java index 0b7da5ec3..49f850c55 100644 --- a/render-docx/src/test/java/com/demcha/compose/document/backend/semantic/docx/DocxRowLayoutTest.java +++ b/render-docx/src/test/java/com/demcha/compose/document/backend/semantic/docx/DocxRowLayoutTest.java @@ -83,7 +83,7 @@ void weightsAndTheGapDecideTheColumnsAndTheCellMargins() throws Exception { // column's width and comes back out as that cell's right margin: the text box is // the slot, and the second column starts exactly where the second slot does. XWPFTable table = onlyTable(page -> page.addRow(r -> r - .gap(20) + .spacing(20) .weights(3, 2) .addParagraph(p -> p.text("Scope")) .addParagraph(p -> p.text("Period")))); @@ -154,7 +154,7 @@ void theMeasuredSplitAgreesWithTheStatedOneWhereBothCanAnswer() throws Exception // Weights are arithmetic either way, so the two paths have to arrive at the same // columns — if they ever part, one of them is reading the row wrong. Consumer weighted = page -> page.addRow(r -> r - .gap(20) + .spacing(20) .weights(3, 2) .addParagraph(p -> p.text("Scope")) .addParagraph(p -> p.text("Period"))); @@ -210,7 +210,7 @@ private static long sum(List values) { /** {@code ST_TwipsMeasure} is an xmlbeans union, so the accessor is typed Object. */ private static long twips(Object measure) { - return Long.parseLong(String.valueOf(measure)); + return DocxTwips.of(measure); } private static int occurrences(String haystack, String needle) { diff --git a/render-docx/src/test/java/com/demcha/compose/document/backend/semantic/docx/DocxTableWidthTest.java b/render-docx/src/test/java/com/demcha/compose/document/backend/semantic/docx/DocxTableWidthTest.java index 8bd8a5d8f..b77fe57f3 100644 --- a/render-docx/src/test/java/com/demcha/compose/document/backend/semantic/docx/DocxTableWidthTest.java +++ b/render-docx/src/test/java/com/demcha/compose/document/backend/semantic/docx/DocxTableWidthTest.java @@ -185,7 +185,7 @@ private static String widthType(XWPFTable table) { /** {@code ST_TwipsMeasure} is an xmlbeans union, so the accessor is typed Object. */ private static long twips(Object measure) { - return Long.parseLong(String.valueOf(measure)); + return DocxTwips.of(measure); } /** The table as written with nothing measured behind it. */ diff --git a/render-docx/src/test/java/com/demcha/compose/document/backend/semantic/docx/DocxTwips.java b/render-docx/src/test/java/com/demcha/compose/document/backend/semantic/docx/DocxTwips.java new file mode 100644 index 000000000..f6a134e14 --- /dev/null +++ b/render-docx/src/test/java/com/demcha/compose/document/backend/semantic/docx/DocxTwips.java @@ -0,0 +1,31 @@ +package com.demcha.compose.document.backend.semantic.docx; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Reads a twip value back out of an exported file. + * + *

XmlBeans hands a measure back as the schema's union, and the export only ever writes + * plain twips, which come back as a {@link Number}. Reading one as text and parsing it — + * what these tests used to do, each helper its own copy — works until the value is not a + * number, and then fails as a parse error far from what was wrong. Asserting the type says + * what was wrong.

+ * + * @author Artem Demchyshyn + */ +final class DocxTwips { + + private DocxTwips() { + } + + /** + * @param measure a measure read from the file + * @return its value in twips + */ + static long of(Object measure) { + assertThat(measure) + .as("a twip value this export writes as a plain number") + .isInstanceOf(Number.class); + return ((Number) measure).longValue(); + } +} diff --git a/render-docx/src/test/java/com/demcha/compose/document/backend/semantic/docx/DocxVerticalSpacingTest.java b/render-docx/src/test/java/com/demcha/compose/document/backend/semantic/docx/DocxVerticalSpacingTest.java index b5bdd957a..e3955ba67 100644 --- a/render-docx/src/test/java/com/demcha/compose/document/backend/semantic/docx/DocxVerticalSpacingTest.java +++ b/render-docx/src/test/java/com/demcha/compose/document/backend/semantic/docx/DocxVerticalSpacingTest.java @@ -318,7 +318,7 @@ private static long before(XWPFParagraph paragraph) { if (properties == null || !properties.isSetSpacing() || !properties.getSpacing().isSetBefore()) { return 0; } - return Long.parseLong(String.valueOf(properties.getSpacing().getBefore())); + return DocxTwips.of(properties.getSpacing().getBefore()); } private static long after(XWPFParagraph paragraph) { @@ -326,7 +326,7 @@ private static long after(XWPFParagraph paragraph) { if (properties == null || !properties.isSetSpacing() || !properties.getSpacing().isSetAfter()) { return 0; } - return Long.parseLong(String.valueOf(properties.getSpacing().getAfter())); + return DocxTwips.of(properties.getSpacing().getAfter()); } private static List bodyOf(Consumer content) throws Exception { diff --git a/render-docx/src/test/java/com/demcha/compose/document/backend/semantic/docx/probe/EditableExportFixtures.java b/render-docx/src/test/java/com/demcha/compose/document/backend/semantic/docx/probe/EditableExportFixtures.java index bf766442f..e72b041ee 100644 --- a/render-docx/src/test/java/com/demcha/compose/document/backend/semantic/docx/probe/EditableExportFixtures.java +++ b/render-docx/src/test/java/com/demcha/compose/document/backend/semantic/docx/probe/EditableExportFixtures.java @@ -94,7 +94,7 @@ public static DocumentSession mixedTwoPager(Path pdfFile) { session.chrome().zone(DocumentPageZone.footer(30, page -> new RowBuilder() .name("FooterZone") - .gap(8) + .spacing(8) .addParagraph(p -> p.text("Editable export probe").textStyle(CHROME)) .flexSpacer() .add(page.pageNumber(CHROME)) @@ -123,7 +123,7 @@ public static DocumentSession mixedTwoPager(Path pdfFile) { // Two columns of unequal length: the row must keep them side by side and // must not lose the longer column's tail at a page boundary. - page.addRow("TwoColumns", r -> r.gap(18).columns(weight(3), weight(2)) + page.addRow("TwoColumns", r -> r.spacing(18).columns(weight(3), weight(2)) .padding(DocumentInsets.symmetric(14, 0)) .addSection(left -> left .addParagraph(p -> p.text("Scope").textStyle(HEADING))