Skip to content

fix: read XLS literal error cells as errors instead of booleans - #1011

Open
codeAnqiang-ma wants to merge 1 commit into
apache:mainfrom
codeAnqiang-ma:fix/xls-literal-error-cells
Open

fix: read XLS literal error cells as errors instead of booleans#1011
codeAnqiang-ma wants to merge 1 commit into
apache:mainfrom
codeAnqiang-ma:fix/xls-literal-error-cells

Conversation

@codeAnqiang-ma

Copy link
Copy Markdown

Purpose of the pull request

Closed: #1010

What's changed?

A .xls cell holding a literal error value — an error stored as the cell value rather than as a formula result, e.g. after "Paste Special → Values" — was read as a boolean, while the same content in .xlsx read correctly:

cell POI usermodel Fesod .xls before Fesod .xlsx
#DIV/0! ERROR true #DIV/0!
#N/A ERROR true #N/A
#NULL! ERROR false #NULL!

A BIFF BOOLERR record holds either a boolean or an error code (fError flag, exposed by POI as isBoolean()/isError()), but BoolErrRecordHandler called getBooleanValue() unconditionally — POI returns errorCode != 0 for error records, so non-zero codes became true and #NULL! (code 0) became false.

The handler now branches on isError() and emits a CellDataTypeEnum.ERROR cell carrying the error text, which the existing StringErrorConverter renders as the value XLSX already produces. That matches FormulaRecordHandler (XLS formula errors are already ERROR) and CellTagHandler (t="e" → error text); the boolean branch is unchanged. The text comes from ErrorEval.getText(...), as in POI's own DataFormatter.

BoolErrRecordHandlerTest writes that row to both formats and asserts they read back identically, in STRING and ACTUAL_DATA mode.

Test evidence: fails before the fix, passes after, no regressions

Before the fix (test present, BoolErrRecordHandler at main) — both cases fail:

[ERROR] BoolErrRecordHandlerTest.read_literalErrorCells_returnErrorText_inStringMode:68
        expected: <{0=true, 1=#DIV/0!, 2=#N/A, 3=#NULL!, 4=marker}>
         but was: <{0=true, 1=true,    2=true, 3=false,  4=marker}>
[ERROR] BoolErrRecordHandlerTest.read_literalErrorCells_returnErrorText_inActualDataMode:79
        expected: <#DIV/0!> but was: <true>
[ERROR] Tests run: 2, Failures: 2, Errors: 0, Skipped: 0

After the fix:

$ ./mvnw test -pl fesod-sheet -Dmaven.test.skip=false -Dtest=BoolErrRecordHandlerTest
[INFO] Tests run: 2, Failures: 0, Errors: 0, Skipped: 0

Full module suite and format check:

$ ./mvnw test -pl fesod-sheet -Dmaven.test.skip=false
[INFO] Tests run: 681, Failures: 0, Errors: 0, Skipped: 0     # 679 before, +2 new
[INFO] BUILD SUCCESS

$ ./mvnw spotless:check
[INFO] BUILD SUCCESS

Run on JDK 21 (Temurin 21.0.5). I did not run the other JDKs of the CI matrix (8/11/17/25) locally; the change uses no version-specific API.

Checklist

  • I have read the Contributor Guide.
  • I have written the necessary doc or comment.
  • I have added the necessary unit tests and all cases have passed.

Assisted-by: Cursor (Fable 5). I reproduced the behaviour locally and reviewed every line of this change and its test myself.

A BIFF BOOLERR record stores either a boolean or an error code,
distinguished by its fError flag, but BoolErrRecordHandler called
getBooleanValue() unconditionally. POI returns `errorCode != 0` for
error records, so a literal #DIV/0! or #N/A read back as true and
#NULL! (code 0) read back as false, while the same content stored in
.xlsx read back as the error text.

Branch on isError() and emit a CellDataTypeEnum.ERROR cell holding the
error text, so the existing StringErrorConverter yields the same
user-visible value as the xlsx path. This also lines the handler up
with FormulaRecordHandler, which already maps XLS formula errors to
ERROR.

Assisted-by: Cursor (Fable 5)
Co-authored-by: Cursor <cursoragent@cursor.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] XLS literal error cells (#DIV/0!, #N/A, #NULL!) are read as Boolean true/false

1 participant