From d3404e158387ddfa5a16ace1f0eda31f889092dc Mon Sep 17 00:00:00 2001 From: leehaut Date: Mon, 12 Oct 2026 13:35:31 +0800 Subject: [PATCH 1/2] fix decode XLS RKRecord numeric cells instead of treating them as empty Signed-off-by: leehaut --- .../v03/handlers/RkRecordHandler.java | 36 ++++- .../v03/handlers/RkRecordHandlerTest.java | 131 ++++++++++++++++++ 2 files changed, 160 insertions(+), 7 deletions(-) create mode 100644 fesod-sheet/src/test/java/org/apache/fesod/sheet/analysis/v03/handlers/RkRecordHandlerTest.java diff --git a/fesod-sheet/src/main/java/org/apache/fesod/sheet/analysis/v03/handlers/RkRecordHandler.java b/fesod-sheet/src/main/java/org/apache/fesod/sheet/analysis/v03/handlers/RkRecordHandler.java index 1d117e64e..3c5c5370c 100644 --- a/fesod-sheet/src/main/java/org/apache/fesod/sheet/analysis/v03/handlers/RkRecordHandler.java +++ b/fesod-sheet/src/main/java/org/apache/fesod/sheet/analysis/v03/handlers/RkRecordHandler.java @@ -25,29 +25,51 @@ package org.apache.fesod.sheet.analysis.v03.handlers; +import java.math.BigDecimal; import org.apache.fesod.sheet.analysis.v03.IgnorableXlsRecordHandler; +import org.apache.fesod.sheet.constant.BuiltinFormats; import org.apache.fesod.sheet.context.xls.XlsReadContext; +import org.apache.fesod.sheet.enums.RowTypeEnum; +import org.apache.fesod.sheet.metadata.data.DataFormatData; import org.apache.fesod.sheet.metadata.data.ReadCellData; import org.apache.poi.hssf.record.RKRecord; import org.apache.poi.hssf.record.Record; /** - * Record handler + * Record handler for BIFF {@link RKRecord}. + * + *

RK is a compact numeric cell used by older Excel files. The event-API path still delivers these + * records; they must be decoded like {@link NumberRecordHandler}, not treated as empty cells. */ public class RkRecordHandler extends AbstractXlsRecordHandler implements IgnorableXlsRecordHandler { @Override - public void processRecord(XlsReadContext xlsReadContext, Record record) { - RKRecord re = (RKRecord) record; + public void processRecord(XlsReadContext xlsReadContext, Record rec) { + RKRecord re = (RKRecord) rec; Integer targetColumnIndex = xlsReadContext.readSheetHolder().determineTargetColumnIndex(re.getColumn()); if (targetColumnIndex == null) { return; } - xlsReadContext - .xlsReadSheetHolder() - .getCellMap() - .put(targetColumnIndex, ReadCellData.newEmptyInstance(re.getRow(), targetColumnIndex)); + short dataFormat = (short) xlsReadContext + .xlsReadWorkbookHolder() + .getFormatTrackingHSSFListener() + .getFormatIndex(re); + DataFormatData dataFormatData = new DataFormatData(); + dataFormatData.setIndex(dataFormat); + dataFormatData.setFormat(BuiltinFormats.getBuiltinFormat( + dataFormat, + xlsReadContext + .xlsReadWorkbookHolder() + .getFormatTrackingHSSFListener() + .getFormatString(re), + xlsReadContext.readSheetHolder().getGlobalConfiguration().getLocale())); + + ReadCellData cellData = + ReadCellData.newInstanceOriginal(BigDecimal.valueOf(re.getRKNumber()), re.getRow(), targetColumnIndex); + cellData.setDataFormatData(dataFormatData); + xlsReadContext.xlsReadSheetHolder().getCellMap().put(targetColumnIndex, cellData); + xlsReadContext.xlsReadSheetHolder().setTempRowType(RowTypeEnum.DATA); } } diff --git a/fesod-sheet/src/test/java/org/apache/fesod/sheet/analysis/v03/handlers/RkRecordHandlerTest.java b/fesod-sheet/src/test/java/org/apache/fesod/sheet/analysis/v03/handlers/RkRecordHandlerTest.java new file mode 100644 index 000000000..523a7854f --- /dev/null +++ b/fesod-sheet/src/test/java/org/apache/fesod/sheet/analysis/v03/handlers/RkRecordHandlerTest.java @@ -0,0 +1,131 @@ +/* + * 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.v03.handlers; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.math.BigDecimal; +import java.util.LinkedHashMap; +import java.util.Locale; +import java.util.Map; +import org.apache.fesod.sheet.context.xls.XlsReadContext; +import org.apache.fesod.sheet.enums.CellDataTypeEnum; +import org.apache.fesod.sheet.enums.RowTypeEnum; +import org.apache.fesod.sheet.metadata.Cell; +import org.apache.fesod.sheet.metadata.GlobalConfiguration; +import org.apache.fesod.sheet.metadata.data.ReadCellData; +import org.apache.fesod.sheet.read.metadata.holder.ReadSheetHolder; +import org.apache.fesod.sheet.read.metadata.holder.xls.XlsReadSheetHolder; +import org.apache.fesod.sheet.read.metadata.holder.xls.XlsReadWorkbookHolder; +import org.apache.fesod.sheet.testkit.Tags; +import org.apache.poi.hssf.eventusermodel.FormatTrackingHSSFListener; +import org.apache.poi.hssf.record.RKRecord; +import org.apache.poi.hssf.record.RecordInputStream; +import org.apache.poi.util.LittleEndianOutputStream; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; + +/** + * Regression test: BIFF {@link RKRecord} numeric cells must be decoded, not stored as EMPTY. + * + *

Modern Excel / POI usermodel writes {@code NUMBER} records, not {@code RK}. The event-API path + * still sees {@code RK} in older BIFF files, so this test builds a real {@link RKRecord} from bytes + * (integer 42 encoded as RK) and feeds it to the handler. + */ +@Tag(Tags.UNIT) +class RkRecordHandlerTest { + + private static final int ROW = 3; + private static final int COLUMN = 1; + private static final int EXPECTED_VALUE = 42; + + @Test + void processRecord_decodesIntegerRkAsNumber() throws IOException { + Map cellMap = new LinkedHashMap<>(); + XlsReadContext context = createContext(cellMap); + + new RkRecordHandler().processRecord(context, createIntegerRkRecord(ROW, COLUMN, EXPECTED_VALUE)); + + Cell cell = cellMap.get(COLUMN); + Assertions.assertNotNull(cell, "RK cell must be stored in the cell map"); + Assertions.assertInstanceOf(ReadCellData.class, cell); + ReadCellData cellData = (ReadCellData) cell; + + Assertions.assertEquals( + CellDataTypeEnum.NUMBER, + cellData.getType(), + "RK record must be read as NUMBER, not EMPTY (silent data loss)"); + Assertions.assertEquals( + 0, + BigDecimal.valueOf(EXPECTED_VALUE).compareTo(cellData.getNumberValue()), + "RK integer value must be preserved"); + Assertions.assertEquals(ROW, cellData.getRowIndex()); + Assertions.assertEquals(COLUMN, cellData.getColumnIndex()); + Assertions.assertNotNull(cellData.getDataFormatData()); + Mockito.verify(context.xlsReadSheetHolder()).setTempRowType(RowTypeEnum.DATA); + } + + private static XlsReadContext createContext(Map cellMap) { + XlsReadContext context = Mockito.mock(XlsReadContext.class); + ReadSheetHolder readSheetHolder = Mockito.mock(ReadSheetHolder.class); + XlsReadSheetHolder xlsReadSheetHolder = Mockito.mock(XlsReadSheetHolder.class); + XlsReadWorkbookHolder xlsReadWorkbookHolder = Mockito.mock(XlsReadWorkbookHolder.class); + FormatTrackingHSSFListener formatListener = Mockito.mock(FormatTrackingHSSFListener.class); + GlobalConfiguration globalConfiguration = new GlobalConfiguration(); + globalConfiguration.setLocale(Locale.US); + + Mockito.when(context.readSheetHolder()).thenReturn(readSheetHolder); + Mockito.when(context.xlsReadSheetHolder()).thenReturn(xlsReadSheetHolder); + Mockito.when(context.xlsReadWorkbookHolder()).thenReturn(xlsReadWorkbookHolder); + Mockito.when(readSheetHolder.determineTargetColumnIndex(COLUMN)).thenReturn(COLUMN); + Mockito.when(readSheetHolder.getGlobalConfiguration()).thenReturn(globalConfiguration); + Mockito.when(xlsReadSheetHolder.getCellMap()).thenReturn(cellMap); + Mockito.when(xlsReadWorkbookHolder.getFormatTrackingHSSFListener()).thenReturn(formatListener); + Mockito.when(formatListener.getFormatIndex(Mockito.any())).thenReturn(0); + Mockito.when(formatListener.getFormatString(Mockito.any(RKRecord.class))) + .thenReturn("General"); + return context; + } + + /** + * Builds a BIFF {@code RK} record whose payload encodes {@code value} as an integer RK + * ({@code (value << 2) | 0x02}). + */ + private static RKRecord createIntegerRkRecord(int row, int column, int value) throws IOException { + int rkNumber = (value << 2) | 0x02; + ByteArrayOutputStream bos = new ByteArrayOutputStream(); + try (LittleEndianOutputStream out = new LittleEndianOutputStream(bos)) { + out.writeShort(RKRecord.sid); + out.writeShort(10); // row(2) + col(2) + xf(2) + rk(4) + out.writeShort(row); + out.writeShort(column); + out.writeShort(0); // xf index + out.writeInt(rkNumber); + } + RecordInputStream ris = new RecordInputStream(new ByteArrayInputStream(bos.toByteArray())); + ris.nextRecord(); + RKRecord rec = new RKRecord(ris); + Assertions.assertEquals(value, rec.getRKNumber(), 0.0, "fixture RK encoding must decode to " + value); + return rec; + } +} From e1d6978db4fc8f9a41145a6447af496ab122c508 Mon Sep 17 00:00:00 2001 From: leehaut Date: Tue, 13 Oct 2026 16:04:32 +0800 Subject: [PATCH 2/2] fix decode XLS RKRecord numeric cells instead of treating them as empty Signed-off-by: leehaut --- .../v03/handlers/RkRecordHandlerTest.java | 85 ++++++++++++++----- 1 file changed, 66 insertions(+), 19 deletions(-) diff --git a/fesod-sheet/src/test/java/org/apache/fesod/sheet/analysis/v03/handlers/RkRecordHandlerTest.java b/fesod-sheet/src/test/java/org/apache/fesod/sheet/analysis/v03/handlers/RkRecordHandlerTest.java index 523a7854f..3f8a07144 100644 --- a/fesod-sheet/src/test/java/org/apache/fesod/sheet/analysis/v03/handlers/RkRecordHandlerTest.java +++ b/fesod-sheet/src/test/java/org/apache/fesod/sheet/analysis/v03/handlers/RkRecordHandlerTest.java @@ -26,6 +26,7 @@ import java.util.LinkedHashMap; import java.util.Locale; import java.util.Map; +import java.util.stream.Stream; import org.apache.fesod.sheet.context.xls.XlsReadContext; import org.apache.fesod.sheet.enums.CellDataTypeEnum; import org.apache.fesod.sheet.enums.RowTypeEnum; @@ -43,46 +44,75 @@ 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; import org.mockito.Mockito; /** * Regression test: BIFF {@link RKRecord} numeric cells must be decoded, not stored as EMPTY. * *

Modern Excel / POI usermodel writes {@code NUMBER} records, not {@code RK}. The event-API path - * still sees {@code RK} in older BIFF files, so this test builds a real {@link RKRecord} from bytes - * (integer 42 encoded as RK) and feeds it to the handler. + * still sees {@code RK} in older BIFF files, so this test builds real {@link RKRecord}s from bytes + * covering the RK encodings supported by Apache POI ({@code RKUtil.decodeNumber}). */ @Tag(Tags.UNIT) class RkRecordHandlerTest { private static final int ROW = 3; private static final int COLUMN = 1; - private static final int EXPECTED_VALUE = 42; + private static final String FORMAT_STRING = "0.00"; @Test - void processRecord_decodesIntegerRkAsNumber() throws IOException { + void processRecord_preservesDataFormatAndMarksRowAsData() throws IOException { Map cellMap = new LinkedHashMap<>(); XlsReadContext context = createContext(cellMap); - new RkRecordHandler().processRecord(context, createIntegerRkRecord(ROW, COLUMN, EXPECTED_VALUE)); + new RkRecordHandler().processRecord(context, createRkRecord(ROW, COLUMN, encodeIntegerRk(42, false))); + + ReadCellData cellData = (ReadCellData) cellMap.get(COLUMN); + Assertions.assertNotNull(cellData.getDataFormatData()); + Assertions.assertEquals(FORMAT_STRING, cellData.getDataFormatData().getFormat()); + Mockito.verify(context.xlsReadSheetHolder()).setTempRowType(RowTypeEnum.DATA); + } + + @ParameterizedTest(name = "{0}") + @MethodSource("rkEncodings") + void processRecord_decodesRkEncodings(String caseName, int rkNumber, double expectedValue) throws IOException { + Map cellMap = new LinkedHashMap<>(); + XlsReadContext context = createContext(cellMap); + + RKRecord rec = createRkRecord(ROW, COLUMN, rkNumber); + Assertions.assertEquals( + expectedValue, rec.getRKNumber(), 0.0, "fixture RK encoding must decode to " + expectedValue); + + new RkRecordHandler().processRecord(context, rec); Cell cell = cellMap.get(COLUMN); - Assertions.assertNotNull(cell, "RK cell must be stored in the cell map"); + Assertions.assertNotNull(cell, caseName + ": RK cell must be stored in the cell map"); Assertions.assertInstanceOf(ReadCellData.class, cell); ReadCellData cellData = (ReadCellData) cell; Assertions.assertEquals( CellDataTypeEnum.NUMBER, cellData.getType(), - "RK record must be read as NUMBER, not EMPTY (silent data loss)"); + caseName + ": RK record must be read as NUMBER, not EMPTY"); Assertions.assertEquals( 0, - BigDecimal.valueOf(EXPECTED_VALUE).compareTo(cellData.getNumberValue()), - "RK integer value must be preserved"); + BigDecimal.valueOf(expectedValue).compareTo(cellData.getNumberValue()), + caseName + ": RK numeric value must be preserved"); Assertions.assertEquals(ROW, cellData.getRowIndex()); Assertions.assertEquals(COLUMN, cellData.getColumnIndex()); - Assertions.assertNotNull(cellData.getDataFormatData()); - Mockito.verify(context.xlsReadSheetHolder()).setTempRowType(RowTypeEnum.DATA); + } + + static Stream rkEncodings() { + return Stream.of( + Arguments.of("positive integer", encodeIntegerRk(42, false), 42.0), + Arguments.of("negative integer", encodeIntegerRk(-7, false), -7.0), + Arguments.of("integer with /100 flag", encodeIntegerRk(1234, true), 12.34), + Arguments.of("negative integer with /100 flag", encodeIntegerRk(-50, true), -0.5), + Arguments.of("IEEE floating point", encodeIeeeRk(1.5, false), 1.5), + Arguments.of("IEEE floating point with /100 flag", encodeIeeeRk(5.5, true), 0.055)); } private static XlsReadContext createContext(Map cellMap) { @@ -101,18 +131,37 @@ private static XlsReadContext createContext(Map cellMap) { Mockito.when(readSheetHolder.getGlobalConfiguration()).thenReturn(globalConfiguration); Mockito.when(xlsReadSheetHolder.getCellMap()).thenReturn(cellMap); Mockito.when(xlsReadWorkbookHolder.getFormatTrackingHSSFListener()).thenReturn(formatListener); - Mockito.when(formatListener.getFormatIndex(Mockito.any())).thenReturn(0); + Mockito.when(formatListener.getFormatIndex(Mockito.any())).thenReturn(2); Mockito.when(formatListener.getFormatString(Mockito.any(RKRecord.class))) - .thenReturn("General"); + .thenReturn(FORMAT_STRING); return context; } /** - * Builds a BIFF {@code RK} record whose payload encodes {@code value} as an integer RK - * ({@code (value << 2) | 0x02}). + * Integer RK: {@code (value << 2) | 0x02}, optionally {@code | 0x01} for divide-by-100. */ - private static RKRecord createIntegerRkRecord(int row, int column, int value) throws IOException { + private static int encodeIntegerRk(int value, boolean divideBy100) { int rkNumber = (value << 2) | 0x02; + if (divideBy100) { + rkNumber |= 0x01; + } + return rkNumber; + } + + /** + * IEEE RK: top 30 bits of the double bit pattern in bits 2–31, optionally {@code | 0x01} for + * divide-by-100. Only values whose lower 34 mantissa bits are zero encode losslessly. + */ + private static int encodeIeeeRk(double value, boolean divideBy100) { + long bits = Double.doubleToLongBits(value); + int rkNumber = (int) ((bits >>> 34) << 2); + if (divideBy100) { + rkNumber |= 0x01; + } + return rkNumber; + } + + private static RKRecord createRkRecord(int row, int column, int rkNumber) throws IOException { ByteArrayOutputStream bos = new ByteArrayOutputStream(); try (LittleEndianOutputStream out = new LittleEndianOutputStream(bos)) { out.writeShort(RKRecord.sid); @@ -124,8 +173,6 @@ private static RKRecord createIntegerRkRecord(int row, int column, int value) th } RecordInputStream ris = new RecordInputStream(new ByteArrayInputStream(bos.toByteArray())); ris.nextRecord(); - RKRecord rec = new RKRecord(ris); - Assertions.assertEquals(value, rec.getRKNumber(), 0.0, "fixture RK encoding must decode to " + value); - return rec; + return new RKRecord(ris); } }