Bug 59665 - [Patch] Using HSSFWorkbook#setSheetOrder to move sheets to the end corrupts bspos value in WorkbookRecordList
Summary: [Patch] Using HSSFWorkbook#setSheetOrder to move sheets to the end corrupts b...
Alias: None
Product: POI
Classification: Unclassified
Component: HSSF (show other bugs)
Version: unspecified
Hardware: All All
: P2 regression (vote)
Target Milestone: ---
Assignee: POI Developers List
Keywords: PatchAvailable
Depends on:
Reported: 2016-06-06 12:12 UTC by Matthew Wightman
Modified: 2016-07-17 09:28 UTC (History)
0 users

Patch which sets the value of bspos (1.84 KB, patch)
2016-06-06 12:12 UTC, Matthew Wightman
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Matthew Wightman 2016-06-06 12:12:32 UTC
Created attachment 33914 [details]
Patch which sets the value of bspos

Since POI 3.10, the setSheetOrder method on HSSFWorkbook corrupts internal state when used to move a sheet to the end of the workbook. Specifically, such a move results in the value of bspos in the WorkbookRecordList being reduced by one from the correct value, with the result that all future uses of that field to locate records get the wrong record.

For example, the following code

import org.apache.poi.hssf.usermodel.HSSFSheet;
import org.apache.poi.hssf.usermodel.HSSFWorkbook;

public class Demo {

  public static void main(String ... args) throws Exception {
    final HSSFWorkbook test = new HSSFWorkbook();
    for (int i=0; i<70; i++) {
      test.setSheetOrder("A", 0);

will throw an exception after 62 iterations:

Exception in thread "main" java.lang.ArrayIndexOutOfBoundsException: -1
    at java.util.ArrayList.elementData(ArrayList.java:418)
    at java.util.ArrayList.get(ArrayList.java:431)
    at org.apache.poi.hssf.model.WorkbookRecordList.get(WorkbookRecordList.java:50)
    at org.apache.poi.hssf.model.InternalWorkbook.setSheetOrder(InternalWorkbook.java:649)
    at org.apache.poi.hssf.usermodel.HSSFWorkbook.setSheetOrder(HSSFWorkbook.java:489)
    at Demo.main(Demo.java:10)

This happens because the removal of the bound sheet record in InternalWorkbook#setSheetOrder reduces the value of bspos by one, but when the record is added back, the position it is placed in is after what is now the last bound sheet record, so bspos is not incremented. Each iteration then reduces bspos by one, until it results in POI trying to fetch records from outside the bounds of the list.

This behaviour was introduced by the fix for Bug 50298 in commit https://svn.apache.org/repos/asf/poi/trunk@1516313. Before that, POI did not reorder the records in this scenario.

Please find attached a patch which sets the value of bspos following the sheet move, which fixes the observed issue.
Comment 1 David North 2016-06-06 12:20:58 UTC
Patch looks believable to me, but it would be good to have review from someone more familiar with HSSF.
Comment 2 Dominik Stadler 2016-07-17 09:28:48 UTC
Looks reasonable, applied via r1753038, thanks for the patch!