From 900894e4baedcef34b28cb2575a712a1069c8d51 Mon Sep 17 00:00:00 2001 From: Nikita Kuprins Date: Tue, 21 Jul 2026 14:46:48 +0300 Subject: [PATCH] fix: descriptive error for unrecognized cell type attribute 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 --- .../analysis/v07/handlers/CellTagHandler.java | 11 +++- .../fesod/sheet/enums/CellDataTypeEnum.java | 5 +- .../v07/handlers/CellTagHandlerTest.java | 61 +++++++++++++++++++ 3 files changed, 72 insertions(+), 5 deletions(-) create mode 100644 fesod-sheet/src/test/java/org/apache/fesod/sheet/analysis/v07/handlers/CellTagHandlerTest.java 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 3978beff9..2d2e6d6e0 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.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 void startElement(XlsxReadContext xlsxReadContext, String name, Attribute // 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 c0ff67854..53a6c6e9a 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 000000000..e6776d136 --- /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 issue #955. + * + *

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