diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml index 8cec81b2c..cad9c5d4e 100644 --- a/src/documentation/content/xdocs/status.xml +++ b/src/documentation/content/xdocs/status.xml @@ -34,6 +34,7 @@ + 49931 - Avoid concurrency problems when re-ordering multiple HSSF header records for a PageSettingsBlock 49765 - Fix XWPFDocument.addPicture so that it correctly sets up relationships 48018 - Improve HWPF handling of lists in documents read and then saved, by preserving order better 49820 - Fix HWPF paragraph levels, so that outline levels can be properly fetched diff --git a/src/java/org/apache/poi/hssf/record/aggregates/PageSettingsBlock.java b/src/java/org/apache/poi/hssf/record/aggregates/PageSettingsBlock.java index 6dde94c3c..17cb84d55 100644 --- a/src/java/org/apache/poi/hssf/record/aggregates/PageSettingsBlock.java +++ b/src/java/org/apache/poi/hssf/record/aggregates/PageSettingsBlock.java @@ -18,6 +18,7 @@ package org.apache.poi.hssf.record.aggregates; import java.util.ArrayList; +import java.util.Collections; import java.util.Iterator; import java.util.List; import java.util.Arrays; @@ -639,7 +640,7 @@ public final class PageSettingsBlock extends RecordAggregate { /** * Some apps can define multiple HeaderFooterRecord records for a sheet. - * When saving such a file Excel 2007 re-positiones them according to the followig rules: + * When saving such a file Excel 2007 re-positions them according to the following rules: * - take a HeaderFooterRecord and read 16-byte GUID at offset 12. If it is zero, * it means the current sheet and the given HeaderFooterRecord belongs to this PageSettingsBlock * - If GUID is not zero then search in preceding CustomViewSettingsRecordAggregates. @@ -649,10 +650,13 @@ public final class PageSettingsBlock extends RecordAggregate { * @param sheetRecords the list of sheet records read so far */ public void positionRecords(List sheetRecords) { + // Take a copy to loop over, so we can update the real one + // without concurrency issues + List hfRecordsToIterate = new ArrayList(_sviewHeaderFooters); + // loop through HeaderFooterRecord records having not-empty GUID and match them with // CustomViewSettingsRecordAggregate blocks having UserSViewBegin with the same GUID - for (final Iterator it = _sviewHeaderFooters.iterator(); it.hasNext(); ) { - final HeaderFooterRecord hf = it.next(); + for(final HeaderFooterRecord hf : hfRecordsToIterate) { for (RecordBase rb : sheetRecords) { if (rb instanceof CustomViewSettingsRecordAggregate) { final CustomViewSettingsRecordAggregate cv = (CustomViewSettingsRecordAggregate) rb; @@ -663,7 +667,7 @@ public final class PageSettingsBlock extends RecordAggregate { byte[] guid2 = hf.getGuid(); if (Arrays.equals(guid1, guid2)) { cv.append(hf); - it.remove(); + _sviewHeaderFooters.remove(hf); } } } diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java b/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java index a7f52e9a6..b92a3d5f6 100644 --- a/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java +++ b/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java @@ -1857,6 +1857,15 @@ if(1==2) { } } assertTrue(namedStyles.containsAll(collecteddStyles)); - } + + /** + * Regression with the PageSettingsBlock + */ + public void test49931() throws Exception { + HSSFWorkbook wb = openSample("49931.xls"); + + assertEquals(1, wb.getNumberOfSheets()); + assertEquals("Foo", wb.getSheetAt(0).getRow(0).getCell(0).getRichStringCellValue().toString()); + } } diff --git a/test-data/spreadsheet/49931.xls b/test-data/spreadsheet/49931.xls new file mode 100644 index 000000000..0ec10b0ba Binary files /dev/null and b/test-data/spreadsheet/49931.xls differ