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 f98e297e8..21eb39c6a 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); + } +}