Fix bug #49931 - Avoid concurrency problems when re-ordering multiple HSSF header records for a PageSettingsBlock

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@998967 13f79535-47bb-0310-9956-ffa450edef68

4 files changed, 19 insertions, 5 deletions

diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml
index 8cec81b2c0..cad9c5d4e1 100644
--- a/src/documentation/content/xdocs/status.xml
+++ b/src/documentation/content/xdocs/status.xml
@@ -34,6 +34,7 @@
 
     <changes>
         <release version="3.7-beta3" date="2010-??-??">
+           <action dev="poi-developers" type="fix">49931 - Avoid concurrency problems when re-ordering multiple HSSF header records for a PageSettingsBlock</action>
            <action dev="poi-developers" type="fix">49765 - Fix XWPFDocument.addPicture so that it correctly sets up relationships</action>
            <action dev="poi-developers" type="fix">48018 - Improve HWPF handling of lists in documents read and then saved, by preserving order better</action>
            <action dev="poi-developers" type="fix">49820 - Fix HWPF paragraph levels, so that outline levels can be properly fetched</action>
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 6dde94c3c7..17cb84d55d 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<RecordBase> sheetRecords) {
+        // Take a copy to loop over, so we can update the real one
+        //  without concurrency issues
+        List<HeaderFooterRecord> hfRecordsToIterate = new ArrayList<HeaderFooterRecord>(_sviewHeaderFooters);
+       
         // loop through HeaderFooterRecord records having not-empty GUID and match them with
         // CustomViewSettingsRecordAggregate blocks having UserSViewBegin with the same GUID
-        for (final Iterator<HeaderFooterRecord> 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 a7f52e9a6d..b92a3d5f6d 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 0000000000..0ec10b0ba3
--- /dev/null
+++ b/test-data/spreadsheet/49931.xls