diff --git a/fesod-sheet/src/main/java/org/apache/fesod/sheet/analysis/v07/handlers/CellTagHandler.java b/fesod-sheet/src/main/java/org/apache/fesod/sheet/analysis/v07/handlers/CellTagHandler.java index 21eb39c6a..3953db5a6 100644 --- a/fesod-sheet/src/main/java/org/apache/fesod/sheet/analysis/v07/handlers/CellTagHandler.java +++ b/fesod-sheet/src/main/java/org/apache/fesod/sheet/analysis/v07/handlers/CellTagHandler.java @@ -37,6 +37,7 @@ import org.apache.fesod.sheet.metadata.GlobalConfiguration; import org.apache.fesod.sheet.metadata.data.ReadCellData; import org.apache.fesod.sheet.read.metadata.holder.xlsx.XlsxReadSheetHolder; +import org.apache.fesod.sheet.util.XlsxEscapeUtils; import org.xml.sax.Attributes; /** @@ -116,6 +117,10 @@ public void endElement(XlsxReadContext xlsxReadContext, String name) { tempCellData.setStringValue(stringValue); break; case DIRECT_STRING: + // Undo the '_xHHHH_' escapes of characters XML forbids + tempCellData.setStringValue(XlsxEscapeUtils.utfDecode(tempDataString)); + tempCellData.setType(CellDataTypeEnum.STRING); + break; case ERROR: tempCellData.setStringValue(tempDataString); tempCellData.setType(CellDataTypeEnum.STRING); diff --git a/fesod-sheet/src/main/java/org/apache/fesod/sheet/analysis/v07/handlers/sax/SharedStringsTableHandler.java b/fesod-sheet/src/main/java/org/apache/fesod/sheet/analysis/v07/handlers/sax/SharedStringsTableHandler.java index 8a660b3b1..7c4be66b7 100644 --- a/fesod-sheet/src/main/java/org/apache/fesod/sheet/analysis/v07/handlers/sax/SharedStringsTableHandler.java +++ b/fesod-sheet/src/main/java/org/apache/fesod/sheet/analysis/v07/handlers/sax/SharedStringsTableHandler.java @@ -25,10 +25,9 @@ package org.apache.fesod.sheet.analysis.v07.handlers.sax; -import java.util.regex.Matcher; -import java.util.regex.Pattern; import org.apache.fesod.sheet.cache.ReadCache; import org.apache.fesod.sheet.constant.ExcelXmlConstants; +import org.apache.fesod.sheet.util.XlsxEscapeUtils; import org.xml.sax.Attributes; import org.xml.sax.helpers.DefaultHandler; @@ -39,8 +38,6 @@ */ public class SharedStringsTableHandler extends DefaultHandler { - private static final Pattern UTF_PATTERN = Pattern.compile("_x([0-9A-Fa-f]{4})_"); - /** * The final piece of data */ @@ -114,7 +111,7 @@ public void endElement(String uri, String localName, String name) { if (currentData == null) { readCache.put(null); } else { - readCache.put(utfDecode(currentData.toString())); + readCache.put(XlsxEscapeUtils.utfDecode(currentData.toString())); } break; case ExcelXmlConstants.SHAREDSTRINGS_RPH_TAG: @@ -137,51 +134,4 @@ public void characters(char[] ch, int start, int length) { } currentElementData.append(ch, start, length); } - - /** - * from poi XSSFRichTextString - * - * @param value the string to decode - * @return the decoded string or null if the input string is null - *

- * For all characters which cannot be represented in XML as defined by the XML 1.0 specification, - * the characters are escaped using the Unicode numerical character representation escape character - * format _xHHHH_, where H represents a hexadecimal character in the character's value. - *

- * Example: The Unicode character 0D is invalid in an XML 1.0 document, - * so it shall be escaped as _x000D_. - *

- * See section 3.18.9 in the OOXML spec. - * @see org.apache.poi.xssf.usermodel.XSSFRichTextString#utfDecode(String) - */ - static String utfDecode(String value) { - if (value == null || !value.contains("_x")) { - return value; - } - - StringBuilder buf = new StringBuilder(); - Matcher m = UTF_PATTERN.matcher(value); - int idx = 0; - while (m.find()) { - int pos = m.start(); - if (pos > idx) { - buf.append(value, idx, pos); - } - - String code = m.group(1); - int icode = Integer.decode("0x" + code); - buf.append((char) icode); - - idx = m.end(); - } - - // small optimization: don't go via StringBuilder if not necessary, - // the encodings are very rare, so we should almost always go via this shortcut. - if (idx == 0) { - return value; - } - - buf.append(value.substring(idx)); - return buf.toString(); - } } diff --git a/fesod-sheet/src/main/java/org/apache/fesod/sheet/util/XlsxEscapeUtils.java b/fesod-sheet/src/main/java/org/apache/fesod/sheet/util/XlsxEscapeUtils.java new file mode 100644 index 000000000..64f42d4ec --- /dev/null +++ b/fesod-sheet/src/main/java/org/apache/fesod/sheet/util/XlsxEscapeUtils.java @@ -0,0 +1,98 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.fesod.sheet.util; + +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +/** + * The {@code _xHHHH_} escapes that xlsx uses for characters XML 1.0 forbids, defined by section 3.18.9 of the Office + * Open XML spec. + *

+ * Every xlsx read path decodes them, whichever part carries the text: + * {@link org.apache.fesod.sheet.analysis.v07.handlers.sax.SharedStringsTableHandler SharedStringsTableHandler} for + * {@code sharedStrings.xml}, and + * {@link org.apache.fesod.sheet.analysis.v07.handlers.CellTagHandler CellTagHandler} for an inline or direct string + * held by the cell itself. POI decodes on both of its own read paths, the DOM {@code XSSFCell} and the streaming + * {@code XSSFSheetXMLHandler}. + *

+ * The convention escapes itself: text that is literally {@code _x0041_} is stored as {@code _x005F_x0041_}, since + * {@code _x005F_} is the escape for the underscore. Decoding it yields the literal back, not {@code A}. + *

+ * The write half of the same convention lives in + * {@link org.apache.fesod.sheet.write.handler.EscapeHexCellWriteHandler EscapeHexCellWriteHandler}, which produces + * that {@code _x005F_x} form. Both sides read {@code _xHHHH_} the same way, so a change to what counts as an escape + * belongs in both. + * + * @see org.apache.fesod.sheet.write.handler.EscapeHexCellWriteHandler + */ +public class XlsxEscapeUtils { + + private static final Pattern UTF_PATTERN = Pattern.compile("_x([0-9A-Fa-f]{4})_"); + + private XlsxEscapeUtils() {} + + /** + * from poi XSSFRichTextString + * + * @param value the string to decode + * @return the decoded string or null if the input string is null + *

+ * For all characters which cannot be represented in XML as defined by the XML 1.0 specification, + * the characters are escaped using the Unicode numerical character representation escape character + * format _xHHHH_, where H represents a hexadecimal character in the character's value. + *

+ * Example: The Unicode character 0D is invalid in an XML 1.0 document, + * so it shall be escaped as _x000D_. + *

+ * See section 3.18.9 in the OOXML spec. + * @see org.apache.poi.xssf.usermodel.XSSFRichTextString#utfDecode(String) + */ + public static String utfDecode(String value) { + if (value == null || !value.contains("_x")) { + return value; + } + + StringBuilder buf = new StringBuilder(); + Matcher m = UTF_PATTERN.matcher(value); + int idx = 0; + while (m.find()) { + int pos = m.start(); + if (pos > idx) { + buf.append(value, idx, pos); + } + + String code = m.group(1); + int icode = Integer.decode("0x" + code); + buf.append((char) icode); + + idx = m.end(); + } + + // small optimization: don't go via StringBuilder if not necessary, + // the encodings are very rare, so we should almost always go via this shortcut. + if (idx == 0) { + return value; + } + + buf.append(value.substring(idx)); + return buf.toString(); + } +} diff --git a/fesod-sheet/src/main/java/org/apache/fesod/sheet/write/handler/EscapeHexCellWriteHandler.java b/fesod-sheet/src/main/java/org/apache/fesod/sheet/write/handler/EscapeHexCellWriteHandler.java index dc172e2b2..b2ec70d50 100644 --- a/fesod-sheet/src/main/java/org/apache/fesod/sheet/write/handler/EscapeHexCellWriteHandler.java +++ b/fesod-sheet/src/main/java/org/apache/fesod/sheet/write/handler/EscapeHexCellWriteHandler.java @@ -35,6 +35,17 @@ *

* To store the literal _xHHHH_ sequence without it being decoded by POI, we need to escape the initial underscore by * replacing _x with _x005F_x. + *

+ * This handler is not registered by default. Without it the writer stores {@code _xHHHH_}-shaped text exactly as + * typed, and any reader that follows the convention - Fesod, POI or Excel - decodes it back to the character it + * names, so the literal does not survive a round trip. Register it on the write to keep such text intact. + *

+ * The read half of the same convention lives in + * {@link org.apache.fesod.sheet.util.XlsxEscapeUtils#utfDecode(String) XlsxEscapeUtils.utfDecode}, which undoes what + * this handler writes. Both sides read {@code _xHHHH_} the same way, so a change to what counts as an escape belongs + * in both. + * + * @see org.apache.fesod.sheet.util.XlsxEscapeUtils#utfDecode(String) */ public class EscapeHexCellWriteHandler implements CellWriteHandler { diff --git a/fesod-sheet/src/test/java/org/apache/fesod/sheet/readwrite/HexEscapeRoundTripTest.java b/fesod-sheet/src/test/java/org/apache/fesod/sheet/readwrite/HexEscapeRoundTripTest.java new file mode 100644 index 000000000..fba19522e --- /dev/null +++ b/fesod-sheet/src/test/java/org/apache/fesod/sheet/readwrite/HexEscapeRoundTripTest.java @@ -0,0 +1,67 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.fesod.sheet.readwrite; + +import java.io.File; +import java.io.IOException; +import java.util.Collections; +import java.util.List; +import org.apache.fesod.sheet.FesodSheet; +import org.apache.fesod.sheet.testkit.Tags; +import org.apache.fesod.sheet.testkit.base.AbstractExcelTest; +import org.apache.fesod.sheet.testkit.enums.ExcelFormat; +import org.apache.fesod.sheet.testkit.listeners.CollectingReadListener; +import org.apache.fesod.sheet.testkit.models.SimpleData; +import org.apache.fesod.sheet.write.handler.EscapeHexCellWriteHandler; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; + +/** + * Regression test for issue #696: the {@code _xHHHH_} + * escapes were undone only for cells backed by {@code sharedStrings.xml}, so an inline + * string - what the default writer emits - reached the caller with the raw escape. + */ +@Tag(Tags.ROUND_TRIP) +class HexEscapeRoundTripTest extends AbstractExcelTest { + + /** + * The handler is what puts a real escape in the cell, storing the literal as {@code Product_x005F_x0002_Code}. + * Taking it from the writer's own output instead would tie the expectation to a writer default, not to the + * reader under test. + */ + @Test + void escapedOnWrite_readsBackAsTheLiteral() throws IOException { + SimpleData data = new SimpleData(); + data.setName("Product_x0002_Code"); + File file = createTempFile("hex-escape", ExcelFormat.XLSX); + FesodSheet.write(file, SimpleData.class) + .registerWriteHandler(new EscapeHexCellWriteHandler()) + .sheet() + .doWrite(Collections.singletonList(data)); + + CollectingReadListener listener = new CollectingReadListener<>(); + FesodSheet.read(file, SimpleData.class, listener).sheet().doRead(); + List rows = listener.getRows(); + + Assertions.assertEquals(1, rows.size()); + Assertions.assertEquals("Product_x0002_Code", rows.get(0).getName()); + } +} diff --git a/fesod-sheet/src/test/java/org/apache/fesod/sheet/util/XlsxEscapeUtilsTest.java b/fesod-sheet/src/test/java/org/apache/fesod/sheet/util/XlsxEscapeUtilsTest.java new file mode 100644 index 000000000..c25914c74 --- /dev/null +++ b/fesod-sheet/src/test/java/org/apache/fesod/sheet/util/XlsxEscapeUtilsTest.java @@ -0,0 +1,106 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.fesod.sheet.util; + +import java.util.stream.Stream; +import org.apache.fesod.sheet.testkit.Tags; +import org.apache.poi.xssf.usermodel.XSSFRichTextString; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +/** + * {@link XlsxEscapeUtils#utfDecode(String)} is a copy of POI's {@code XSSFRichTextString#utfDecode}, so every + * expectation below is what POI produces - see {@link #matchesPoiForEveryInput(String)}, which pins that rather than + * leaving it to the copy staying in step by luck. + */ +@Tag(Tags.UNIT) +class XlsxEscapeUtilsTest { + + static Stream decodesEscapes() { + return Stream.of( + Arguments.of("_x0041_", "A"), + Arguments.of("_x0041_tail", "Atail"), + Arguments.of("head_x0041_", "headA"), + Arguments.of("head_x0041_tail", "headAtail"), + Arguments.of("_x0041__x0042_", "AB"), + Arguments.of("__x0041_", "_A"), + Arguments.of("_x000D_", "\r"), + // the hex digits are case insensitive, the leading x is not - see leavesTextWithoutAnEscapeAlone + Arguments.of("_x00e9_", "é"), + Arguments.of("_x00E9_", "é"), + // an escape whose own underscore is escaped decodes to the literal text, not to the character + Arguments.of("_x005F_x0041_", "_x0041_"), + // as written by Excel - see the sharedStrings.xml of compatibility/t09.xlsx + Arguments.of("SH_x005f_x000D_Z002", "SH_x000D_Z002"), + // a valid escape is decoded even when a malformed one sits next to it + Arguments.of("_x0041_ _xGHIJ_", "A _xGHIJ_"), + // uppercase X is not an escape even once a lowercase one has taken the input past the _x fast path + Arguments.of("_x0041_ _X0042_", "A _X0042_")); + } + + @ParameterizedTest(name = "[{index}] {0} -> {1}") + @MethodSource + void decodesEscapes(String input, String expected) { + Assertions.assertEquals(expected, XlsxEscapeUtils.utfDecode(input)); + } + + static Stream leavesTextWithoutAnEscapeAlone() { + return Stream.of( + "", + "plain text", + "_X0041_", // uppercase X + "_x041_", // three hex digits + "_x00041_", // five hex digits + "_x0041", // no closing underscore + "_x00G1_", // a non-hex digit + "x0041_", // no leading underscore + "_x"); // the marker alone + } + + @ParameterizedTest(name = "[{index}] {0} is left alone") + @MethodSource + void leavesTextWithoutAnEscapeAlone(String input) { + Assertions.assertEquals(input, XlsxEscapeUtils.utfDecode(input)); + } + + @Test + void returnsNullForNull() { + Assertions.assertNull(XlsxEscapeUtils.utfDecode(null)); + } + + /** + * POI is the reference for this decoding, so it is also the oracle: any input where the two disagree is a defect + * here, whatever the table above says. + */ + @ParameterizedTest(name = "[{index}] {0} decodes as POI does") + @MethodSource + void matchesPoiForEveryInput(String input) { + Assertions.assertEquals(new XSSFRichTextString(input).getString(), XlsxEscapeUtils.utfDecode(input)); + } + + static Stream matchesPoiForEveryInput() { + return Stream.concat( + decodesEscapes().map(arguments -> (String) arguments.get()[0]), leavesTextWithoutAnEscapeAlone()); + } +}