Bugzilla 47199 - Fixed PageSettingsBlock/Sheet to tolerate margin records after other non-PSB records
git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@780774 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
Josh Micich
parent
96e8585cff
commit
bffe0432f6
@ -35,6 +35,7 @@
|
||||
<release version="3.5-beta7" date="2009-??-??">
|
||||
</release>
|
||||
<release version="3.5-beta6" date="2009-06-11">
|
||||
<action dev="POI-DEVELOPERS" type="fix">47199 - Fixed PageSettingsBlock/Sheet to tolerate margin records after other non-PSB records</action>
|
||||
<action dev="POI-DEVELOPERS" type="fix">47069 - Fixed HSSFSheet#getFirstRowNum and HSSFSheet#getLastRowNum to return correct values after removal of all rows</action>
|
||||
<action dev="POI-DEVELOPERS" type="fix">47278 - Fixed XSSFCell to avoid generating xsi:nil entries in shared string table</action>
|
||||
<action dev="POI-DEVELOPERS" type="fix">47206 - Fixed XSSFCell to properly read inline strings</action>
|
||||
|
||||
@ -57,7 +57,6 @@ import org.apache.poi.hssf.record.SaveRecalcRecord;
|
||||
import org.apache.poi.hssf.record.ScenarioProtectRecord;
|
||||
import org.apache.poi.hssf.record.SelectionRecord;
|
||||
import org.apache.poi.hssf.record.UncalcedRecord;
|
||||
import org.apache.poi.hssf.record.UnknownRecord;
|
||||
import org.apache.poi.hssf.record.WSBoolRecord;
|
||||
import org.apache.poi.hssf.record.WindowTwoRecord;
|
||||
import org.apache.poi.hssf.record.aggregates.ChartSubstreamRecordAggregate;
|
||||
@ -207,7 +206,7 @@ public final class Sheet implements Model {
|
||||
records.add(rra); //only add the aggregate once
|
||||
continue;
|
||||
}
|
||||
|
||||
|
||||
if (CustomViewSettingsRecordAggregate.isBeginRecord(recSid)) {
|
||||
// This happens three times in test sample file "29982.xls"
|
||||
// Also several times in bugzilla samples 46840-23373 and 46840-23374
|
||||
@ -217,28 +216,13 @@ public final class Sheet implements Model {
|
||||
|
||||
if (PageSettingsBlock.isComponentRecord(recSid)) {
|
||||
if (_psBlock == null) {
|
||||
// typical case - just one PSB (so far)
|
||||
// first PSB record encountered - read all of them:
|
||||
_psBlock = new PageSettingsBlock(rs);
|
||||
records.add(_psBlock);
|
||||
continue;
|
||||
} else {
|
||||
// one or more PSB records found after some intervening non-PSB records
|
||||
_psBlock.addLateRecords(rs);
|
||||
}
|
||||
if (recSid == UnknownRecord.HEADER_FOOTER_089C) {
|
||||
// test samples: SharedFormulaTest.xls, ex44921-21902.xls, ex42570-20305.xls
|
||||
_psBlock.addLateHeaderFooter(rs.getNext());
|
||||
continue;
|
||||
}
|
||||
// Some apps write PLS, WSBOOL, <psb> but PLS is part of <psb>
|
||||
// This happens in the test sample file "NoGutsRecords.xls" and "WORKBOOK_in_capitals.xls"
|
||||
// In this case the first PSB is two records back
|
||||
int prevPsbIx = records.size()-2;
|
||||
if (_psBlock != records.get(prevPsbIx) || !(records.get(prevPsbIx+1) instanceof WSBoolRecord)) {
|
||||
// not quite the expected situation
|
||||
throw new RuntimeException("two Page Settings Blocks found in the same sheet");
|
||||
}
|
||||
records.remove(prevPsbIx); // WSBOOL will drop down one position.
|
||||
PageSettingsBlock latePsb = new PageSettingsBlock(rs);
|
||||
_psBlock = mergePSBs(_psBlock, latePsb);
|
||||
records.add(_psBlock);
|
||||
continue;
|
||||
}
|
||||
|
||||
@ -247,7 +231,7 @@ public final class Sheet implements Model {
|
||||
_mergedCellsTable.read(rs);
|
||||
continue;
|
||||
}
|
||||
|
||||
|
||||
if (recSid == BOFRecord.sid) {
|
||||
ChartSubstreamRecordAggregate chartAgg = new ChartSubstreamRecordAggregate(rs);
|
||||
if (false) {
|
||||
@ -372,34 +356,7 @@ public final class Sheet implements Model {
|
||||
recs.add(r);
|
||||
}});
|
||||
}
|
||||
/**
|
||||
* Hack to recover from the situation where the page settings block has been split by
|
||||
* an intervening {@link WSBoolRecord}
|
||||
*/
|
||||
private static PageSettingsBlock mergePSBs(PageSettingsBlock a, PageSettingsBlock b) {
|
||||
List<Record> temp = new ArrayList<Record>();
|
||||
RecordTransferrer rt = new RecordTransferrer(temp);
|
||||
a.visitContainedRecords(rt);
|
||||
b.visitContainedRecords(rt);
|
||||
RecordStream rs = new RecordStream(temp, 0);
|
||||
PageSettingsBlock result = new PageSettingsBlock(rs);
|
||||
if (rs.hasNext()) {
|
||||
throw new RuntimeException("PageSettingsBlocks did not merge properly");
|
||||
}
|
||||
return result;
|
||||
}
|
||||
|
||||
private static final class RecordTransferrer implements RecordVisitor {
|
||||
|
||||
private final List<Record> _destList;
|
||||
|
||||
public RecordTransferrer(List<Record> destList) {
|
||||
_destList = destList;
|
||||
}
|
||||
public void visitRecord(Record r) {
|
||||
_destList.add(r);
|
||||
}
|
||||
}
|
||||
private static final class RecordCloner implements RecordVisitor {
|
||||
|
||||
private final List<RecordBase> _destList;
|
||||
@ -1099,7 +1056,7 @@ public final class Sheet implements Model {
|
||||
*/
|
||||
public void setColumnWidth(int column, int width) {
|
||||
if(width > 255*256) throw new IllegalArgumentException("The maximum column width for an individual cell is 255 characters.");
|
||||
|
||||
|
||||
setColumn(column, null, new Integer(width), null, null, null);
|
||||
}
|
||||
|
||||
|
||||
@ -125,36 +125,47 @@ public final class PageSettingsBlock extends RecordAggregate {
|
||||
private boolean readARecord(RecordStream rs) {
|
||||
switch (rs.peekNextSid()) {
|
||||
case HorizontalPageBreakRecord.sid:
|
||||
checkNotPresent(_rowBreaksRecord);
|
||||
_rowBreaksRecord = (PageBreakRecord) rs.getNext();
|
||||
break;
|
||||
case VerticalPageBreakRecord.sid:
|
||||
checkNotPresent(_columnBreaksRecord);
|
||||
_columnBreaksRecord = (PageBreakRecord) rs.getNext();
|
||||
break;
|
||||
case HeaderRecord.sid:
|
||||
checkNotPresent(_header);
|
||||
_header = (HeaderRecord) rs.getNext();
|
||||
break;
|
||||
case FooterRecord.sid:
|
||||
checkNotPresent(_footer);
|
||||
_footer = (FooterRecord) rs.getNext();
|
||||
break;
|
||||
case HCenterRecord.sid:
|
||||
checkNotPresent(_hCenter);
|
||||
_hCenter = (HCenterRecord) rs.getNext();
|
||||
break;
|
||||
case VCenterRecord.sid:
|
||||
checkNotPresent(_vCenter);
|
||||
_vCenter = (VCenterRecord) rs.getNext();
|
||||
break;
|
||||
case LeftMarginRecord.sid:
|
||||
checkNotPresent(_leftMargin);
|
||||
_leftMargin = (LeftMarginRecord) rs.getNext();
|
||||
break;
|
||||
case RightMarginRecord.sid:
|
||||
checkNotPresent(_rightMargin);
|
||||
_rightMargin = (RightMarginRecord) rs.getNext();
|
||||
break;
|
||||
case TopMarginRecord.sid:
|
||||
checkNotPresent(_topMargin);
|
||||
_topMargin = (TopMarginRecord) rs.getNext();
|
||||
break;
|
||||
case BottomMarginRecord.sid:
|
||||
checkNotPresent(_bottomMargin);
|
||||
_bottomMargin = (BottomMarginRecord) rs.getNext();
|
||||
break;
|
||||
case UnknownRecord.PLS_004D:
|
||||
checkNotPresent(_pls);
|
||||
_pls = rs.getNext();
|
||||
while (rs.peekNextSid()==ContinueRecord.sid) {
|
||||
if (_plsContinues==null) {
|
||||
@ -164,15 +175,19 @@ public final class PageSettingsBlock extends RecordAggregate {
|
||||
}
|
||||
break;
|
||||
case PrintSetupRecord.sid:
|
||||
checkNotPresent(_printSetup);
|
||||
_printSetup = (PrintSetupRecord)rs.getNext();
|
||||
break;
|
||||
case UnknownRecord.BITMAP_00E9:
|
||||
checkNotPresent(_bitmap);
|
||||
_bitmap = rs.getNext();
|
||||
break;
|
||||
case UnknownRecord.PRINTSIZE_0033:
|
||||
checkNotPresent(_printSize);
|
||||
_printSize = rs.getNext();
|
||||
break;
|
||||
case UnknownRecord.HEADER_FOOTER_089C:
|
||||
checkNotPresent(_headerFooter);
|
||||
_headerFooter = rs.getNext();
|
||||
break;
|
||||
default:
|
||||
@ -182,6 +197,13 @@ public final class PageSettingsBlock extends RecordAggregate {
|
||||
return true;
|
||||
}
|
||||
|
||||
private void checkNotPresent(Record rec) {
|
||||
if (rec != null) {
|
||||
throw new RecordFormatException("Duplicate PageSettingsBlock record (sid=0x"
|
||||
+ Integer.toHexString(rec.getSid()) + ")");
|
||||
}
|
||||
}
|
||||
|
||||
private PageBreakRecord getRowBreaksRecord() {
|
||||
if (_rowBreaksRecord == null) {
|
||||
_rowBreaksRecord = new HorizontalPageBreakRecord();
|
||||
@ -551,4 +573,40 @@ public final class PageSettingsBlock extends RecordAggregate {
|
||||
}
|
||||
_headerFooter = rec;
|
||||
}
|
||||
|
||||
/**
|
||||
* This method reads PageSettingsBlock records from the supplied RecordStream until the first
|
||||
* non-PageSettingsBlock record is encountered. As each record is read, it is incorporated
|
||||
* into this PageSettingsBlock.
|
||||
* <p/>
|
||||
* The latest Excel version seems to write the PageSettingsBlock uninterrupted. However there
|
||||
* are several examples (that Excel reads OK) where these records are not written together:
|
||||
* <ul>
|
||||
* <li><b>HEADER_FOOTER(0x089C) after WINDOW2</b> - This record is new in 2007. Some apps
|
||||
* seem to have scattered this record long after the PageSettingsBlock where it belongs
|
||||
* test samples: SharedFormulaTest.xls, ex44921-21902.xls, ex42570-20305.xls</li>
|
||||
* <li><b>PLS, WSBOOL, PageSettingsBlock</b> - WSBOOL is not a PSB record.
|
||||
* This happens in the test sample file "NoGutsRecords.xls" and "WORKBOOK_in_capitals.xls"</li>
|
||||
* <li><b>Margins after DIMENSION</b> - All of PSB should be before DIMENSION. (Bug-47199)</li>
|
||||
* </ul>
|
||||
* These were probably written by other applications (or earlier versions of Excel). It was
|
||||
* decided to not write specific code for detecting each of these cases. POI now tolerates
|
||||
* PageSettingsBlock records scattered all over the sheet record stream, and in any order, but
|
||||
* does not allow duplicates of any of those records.
|
||||
*
|
||||
* <p/>
|
||||
* <b>Note</b> - when POI writes out this PageSettingsBlock, the records will always be written
|
||||
* in one consolidated block (in the standard ordering) regardless of how scattered the records
|
||||
* were when they were originally read.
|
||||
*
|
||||
* @throws RecordFormatException if any PSB record encountered has the same type (sid) as
|
||||
* a record that is already part of this PageSettingsBlock
|
||||
*/
|
||||
public void addLateRecords(RecordStream rs) {
|
||||
while(true) {
|
||||
if (!readARecord(rs)) {
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@ -26,6 +26,7 @@ import org.apache.poi.hssf.HSSFTestDataSamples;
|
||||
import org.apache.poi.hssf.model.RecordStream;
|
||||
import org.apache.poi.hssf.model.Sheet;
|
||||
import org.apache.poi.hssf.record.BOFRecord;
|
||||
import org.apache.poi.hssf.record.BottomMarginRecord;
|
||||
import org.apache.poi.hssf.record.DimensionsRecord;
|
||||
import org.apache.poi.hssf.record.EOFRecord;
|
||||
import org.apache.poi.hssf.record.FooterRecord;
|
||||
@ -33,6 +34,7 @@ import org.apache.poi.hssf.record.HeaderRecord;
|
||||
import org.apache.poi.hssf.record.IndexRecord;
|
||||
import org.apache.poi.hssf.record.NumberRecord;
|
||||
import org.apache.poi.hssf.record.Record;
|
||||
import org.apache.poi.hssf.record.RecordFormatException;
|
||||
import org.apache.poi.hssf.record.UnknownRecord;
|
||||
import org.apache.poi.hssf.record.WindowTwoRecord;
|
||||
import org.apache.poi.hssf.usermodel.HSSFPrintSetup;
|
||||
@ -135,7 +137,7 @@ public final class TestPageSettingsBlock extends TestCase {
|
||||
Sheet sheet = Sheet.createSheet(rs);
|
||||
|
||||
RecordCollector rv = new RecordCollector();
|
||||
sheet.visitContainedRecords(rv, rowIx);
|
||||
sheet.visitContainedRecords(rv, 0);
|
||||
Record[] outRecs = rv.getRecords();
|
||||
if (outRecs[4] == EOFRecord.instance) {
|
||||
throw new AssertionFailedError("Identified bug 46953 - EOF incorrectly appended to PSB");
|
||||
@ -151,6 +153,85 @@ public final class TestPageSettingsBlock extends TestCase {
|
||||
assertEquals(WindowTwoRecord.class, outRecs[6].getClass());
|
||||
assertEquals(EOFRecord.instance, outRecs[7]);
|
||||
}
|
||||
/**
|
||||
* Bug 47199 was due to the margin records being located well after the initial PSB records.
|
||||
* The example file supplied (attachment 23710) had three non-PSB record types
|
||||
* between the PRINTSETUP record and first MARGIN record:
|
||||
* <ul>
|
||||
* <li>PRINTSETUP(0x00A1)</li>
|
||||
* <li>DEFAULTCOLWIDTH(0x0055)</li>
|
||||
* <li>COLINFO(0x007D)</li>
|
||||
* <li>DIMENSIONS(0x0200)</li>
|
||||
* <li>BottomMargin(0x0029)</li>
|
||||
* </ul>
|
||||
*/
|
||||
public void testLateMargins_bug47199() {
|
||||
|
||||
Record[] recs = {
|
||||
BOFRecord.createSheetBOF(),
|
||||
new HeaderRecord("&LSales Figures"),
|
||||
new FooterRecord("&LJanuary"),
|
||||
new DimensionsRecord(),
|
||||
createBottomMargin(0.787F),
|
||||
new WindowTwoRecord(),
|
||||
EOFRecord.instance,
|
||||
};
|
||||
RecordStream rs = new RecordStream(Arrays.asList(recs), 0);
|
||||
|
||||
Sheet sheet;
|
||||
try {
|
||||
sheet = Sheet.createSheet(rs);
|
||||
} catch (RuntimeException e) {
|
||||
if (e.getMessage().equals("two Page Settings Blocks found in the same sheet")) {
|
||||
throw new AssertionFailedError("Identified bug 47199a - failed to process late margings records");
|
||||
}
|
||||
throw e;
|
||||
}
|
||||
|
||||
RecordCollector rv = new RecordCollector();
|
||||
sheet.visitContainedRecords(rv, 0);
|
||||
Record[] outRecs = rv.getRecords();
|
||||
assertEquals(recs.length+1, outRecs.length); // +1 for index record
|
||||
|
||||
assertEquals(BOFRecord.class, outRecs[0].getClass());
|
||||
assertEquals(IndexRecord.class, outRecs[1].getClass());
|
||||
assertEquals(HeaderRecord.class, outRecs[2].getClass());
|
||||
assertEquals(FooterRecord.class, outRecs[3].getClass());
|
||||
assertEquals(DimensionsRecord.class, outRecs[5].getClass());
|
||||
assertEquals(WindowTwoRecord.class, outRecs[6].getClass());
|
||||
assertEquals(EOFRecord.instance, outRecs[7]);
|
||||
}
|
||||
|
||||
private Record createBottomMargin(float value) {
|
||||
BottomMarginRecord result = new BottomMarginRecord();
|
||||
result.setMargin(value);
|
||||
return result;
|
||||
}
|
||||
|
||||
/**
|
||||
* The PageSettingsBlock should not allow multiple copies of the same record. This extra assertion
|
||||
* was added while fixing bug 47199. All existing POI test samples comply with this requirement.
|
||||
*/
|
||||
public void testDuplicatePSBRecord_bug47199() {
|
||||
|
||||
// Hypothetical setup of PSB records which should cause POI to crash
|
||||
Record[] recs = {
|
||||
new HeaderRecord("&LSales Figures"),
|
||||
new HeaderRecord("&LInventory"),
|
||||
};
|
||||
RecordStream rs = new RecordStream(Arrays.asList(recs), 0);
|
||||
|
||||
try {
|
||||
new PageSettingsBlock(rs);
|
||||
throw new AssertionFailedError("Identified bug 47199b - duplicate PSB records should not be allowed");
|
||||
} catch (RecordFormatException e) {
|
||||
if (e.getMessage().equals("Duplicate PageSettingsBlock record (sid=0x14)")) {
|
||||
// expected during successful test
|
||||
} else {
|
||||
throw new AssertionFailedError("Expected RecordFormatException due to duplicate PSB record");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private static UnknownRecord ur(int sid, String hexData) {
|
||||
return new UnknownRecord(sid, HexRead.readFromString(hexData));
|
||||
|
||||
Loading…
Reference in New Issue
Block a user