fix: decode _xHHHH_ escapes when reading inline string cells

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);

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
-     * <p>
-     * 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.
-     * <p>
-     * Example: The Unicode character 0D is invalid in an XML 1.0 document,
-     * so it shall be escaped as <code>_x000D_</code>.
-     * </p>
-     * 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();
-    }
 }

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.
+ * <p>
+ * 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}.
+ * <p>
+ * 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}.
+ * <p>
+ * 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
+     * <p>
+     * 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.
+     * <p>
+     * Example: The Unicode character 0D is invalid in an XML 1.0 document,
+     * so it shall be escaped as <code>_x000D_</code>.
+     * </p>
+     * 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();
+    }
+}

fesod-sheet/src/main/java/org/apache/fesod/sheet/write/handler/EscapeHexCellWriteHandler.java

@@ -35,6 +35,17 @@
  * <p>
  * 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.
+ * <p>
+ * 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.
+ * <p>
+ * 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 {
 

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 <a href="https://github.com/apache/fesod/issues/696">issue #696</a>: 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<SimpleData> listener = new CollectingReadListener<>();
+        FesodSheet.read(file, SimpleData.class, listener).sheet().doRead();
+        List<SimpleData> rows = listener.getRows();
+
+        Assertions.assertEquals(1, rows.size());
+        Assertions.assertEquals("Product_x0002_Code", rows.get(0).getName());
+    }
+}

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<Arguments> 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<String> 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<String> matchesPoiForEveryInput() {
+        return Stream.concat(
+                decodesEscapes().map(arguments -> (String) arguments.get()[0]), leavesTextWithoutAnEscapeAlone());
+    }
+}