This is an automated email from the ASF dual-hosted git repository.

bengbengbalabalabeng pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/fesod.git


The following commit(s) were added to refs/heads/main by this push:
     new b1d35a67 fix: descriptive error for unrecognized cell type attribute 
(#965)
b1d35a67 is described below

commit b1d35a67b8a8e9325246de7cf224c0410116040f
Author: Nikita Kuprins <[email protected]>
AuthorDate: Mon Aug 3 11:43:09 2026 +0300

    fix: descriptive error for unrecognized cell type attribute (#965)
    
    An xlsx cell whose t attribute is not one of the recognized types
    (s, str, inlineStr, e, b, n) caused CellDataTypeEnum.buildFromCellType
    to return null. The very next line then tripped the ReadCellData
    constructor with a confusing 'IllegalArgumentException: Type can not be
    null' that named neither the cell nor the offending attribute.
    
    Detect the null in CellTagHandler and throw an ExcelAnalysisException
    that names the invalid type and the offending cell (by its Excel
    reference, e.g. B4), and document the nullable return of
    buildFromCellType so callers know to handle it. The read still aborts at
    the same point; only the exception type and message change.
    
    Closes #955
    
    Co-authored-by: DeleiGuo <[email protected]>
---
 .../analysis/v07/handlers/CellTagHandler.java      | 11 ++--
 .../apache/fesod/sheet/enums/CellDataTypeEnum.java |  5 +-
 .../analysis/v07/handlers/CellTagHandlerTest.java  | 61 ++++++++++++++++++++++
 3 files changed, 72 insertions(+), 5 deletions(-)

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 f98e297e..21eb39c6 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
@@ -33,6 +33,7 @@ import org.apache.fesod.sheet.constant.ExcelXmlConstants;
 import org.apache.fesod.sheet.constant.FesodSheetConstants;
 import org.apache.fesod.sheet.context.xlsx.XlsxReadContext;
 import org.apache.fesod.sheet.enums.CellDataTypeEnum;
+import org.apache.fesod.sheet.exception.ExcelAnalysisException;
 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;
@@ -49,8 +50,8 @@ public class CellTagHandler extends AbstractXlsxTagHandler {
     @Override
     public void startElement(XlsxReadContext xlsxReadContext, String name, 
Attributes attributes) {
         XlsxReadSheetHolder xlsxReadSheetHolder = 
xlsxReadContext.xlsxReadSheetHolder();
-        xlsxReadSheetHolder.setColumnIndex(PositionUtils.getCol(
-                attributes.getValue(ExcelXmlConstants.ATTRIBUTE_R), 
xlsxReadSheetHolder.getColumnIndex()));
+        String cellReference = 
attributes.getValue(ExcelXmlConstants.ATTRIBUTE_R);
+        xlsxReadSheetHolder.setColumnIndex(PositionUtils.getCol(cellReference, 
xlsxReadSheetHolder.getColumnIndex()));
 
         // t="s" ,it means String
         // t="str" ,it means String,but does not need to be read in the 
'sharedStrings.xml'
@@ -59,7 +60,11 @@ public class CellTagHandler extends AbstractXlsxTagHandler {
         // t="e" ,it means Error
         // t="n" ,it means Number
         // t is null ,it means Empty or Number
-        CellDataTypeEnum type = 
CellDataTypeEnum.buildFromCellType(attributes.getValue(ExcelXmlConstants.ATTRIBUTE_T));
+        String cellType = attributes.getValue(ExcelXmlConstants.ATTRIBUTE_T);
+        CellDataTypeEnum type = CellDataTypeEnum.buildFromCellType(cellType);
+        if (type == null) {
+            throw new ExcelAnalysisException("Invalid cell data type: '" + 
cellType + "' in cell " + cellReference);
+        }
         xlsxReadSheetHolder.setTempCellData(new ReadCellData<>(type));
         xlsxReadSheetHolder.setTempData(new StringBuilder());
 
diff --git 
a/fesod-sheet/src/main/java/org/apache/fesod/sheet/enums/CellDataTypeEnum.java 
b/fesod-sheet/src/main/java/org/apache/fesod/sheet/enums/CellDataTypeEnum.java
index c0ff6785..53a6c6e9 100644
--- 
a/fesod-sheet/src/main/java/org/apache/fesod/sheet/enums/CellDataTypeEnum.java
+++ 
b/fesod-sheet/src/main/java/org/apache/fesod/sheet/enums/CellDataTypeEnum.java
@@ -83,8 +83,9 @@ public enum CellDataTypeEnum {
     /**
      * Build data types
      *
-     * @param cellType
-     * @return
+     * @param cellType the raw {@code t} attribute value of a cell
+     * @return the matching type, {@link #EMPTY} when {@code cellType} is 
empty, or {@code null} when
+     *         {@code cellType} is not a recognized type; callers are expected 
to handle the {@code null} case.
      */
     public static CellDataTypeEnum buildFromCellType(String cellType) {
         if (StringUtils.isEmpty(cellType)) {
diff --git 
a/fesod-sheet/src/test/java/org/apache/fesod/sheet/analysis/v07/handlers/CellTagHandlerTest.java
 
b/fesod-sheet/src/test/java/org/apache/fesod/sheet/analysis/v07/handlers/CellTagHandlerTest.java
new file mode 100644
index 00000000..e6776d13
--- /dev/null
+++ 
b/fesod-sheet/src/test/java/org/apache/fesod/sheet/analysis/v07/handlers/CellTagHandlerTest.java
@@ -0,0 +1,61 @@
+/*
+ * 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.analysis.v07.handlers;
+
+import org.apache.fesod.sheet.context.xlsx.XlsxReadContext;
+import org.apache.fesod.sheet.exception.ExcelAnalysisException;
+import org.apache.fesod.sheet.read.metadata.holder.xlsx.XlsxReadSheetHolder;
+import org.apache.fesod.sheet.testkit.Tags;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+import org.mockito.Mockito;
+import org.xml.sax.helpers.AttributesImpl;
+
+/**
+ * Regression test for <a 
href="https://github.com/apache/fesod/issues/955";>issue #955</a>.
+ *
+ * <p>A cell whose {@code t} attribute is not a recognized type made {@code 
buildFromCellType} return
+ * {@code null}, which then tripped the {@code ReadCellData} constructor with 
a confusing
+ * {@code IllegalArgumentException: Type can not be null} that named neither 
the cell nor the attribute.
+ * The handler must instead throw an {@link ExcelAnalysisException} naming the 
invalid type and its location.
+ */
+@Tag(Tags.UNIT)
+class CellTagHandlerTest {
+
+    @Test
+    void startElement_throwsDescriptiveError_forUnknownCellType() {
+        XlsxReadContext context = Mockito.mock(XlsxReadContext.class);
+        XlsxReadSheetHolder sheetHolder = 
Mockito.mock(XlsxReadSheetHolder.class);
+        Mockito.when(context.xlsxReadSheetHolder()).thenReturn(sheetHolder);
+
+        AttributesImpl attributes = new AttributesImpl();
+        attributes.addAttribute("", "r", "r", "CDATA", "B4");
+        attributes.addAttribute("", "t", "t", "CDATA", "unknown");
+
+        ExcelAnalysisException exception = Assertions.assertThrows(
+                ExcelAnalysisException.class, () -> new 
CellTagHandler().startElement(context, "c", attributes));
+
+        // The message must name the unrecognized type and the exact cell 
(Excel reference) for diagnostics.
+        String message = exception.getMessage();
+        Assertions.assertTrue(message.contains("'unknown'"), "should name the 
unrecognized type: " + message);
+        Assertions.assertTrue(message.contains("B4"), "should name the cell 
reference: " + message);
+    }
+}


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to