From: Andreas Beeker Date: Tue, 3 Jan 2017 01:02:33 +0000 (+0000) Subject: SonarQube fixes X-Git-Tag: REL_3_16_BETA2~67 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=d5eed5ef080f88e03deae2f5d34164248770f926;p=poi.git SonarQube fixes add Iterable interface to EscherContainerRecord / deprecate getChildIterator() git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1777046 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/src/java/org/apache/poi/ddf/EscherContainerRecord.java b/src/java/org/apache/poi/ddf/EscherContainerRecord.java index 3b26d4211a..e570084b24 100644 --- a/src/java/org/apache/poi/ddf/EscherContainerRecord.java +++ b/src/java/org/apache/poi/ddf/EscherContainerRecord.java @@ -18,12 +18,16 @@ package org.apache.poi.ddf; import java.io.PrintWriter; -import java.util.*; +import java.util.ArrayList; +import java.util.Collections; +import java.util.Iterator; +import java.util.List; import org.apache.poi.util.HexDump; import org.apache.poi.util.LittleEndian; import org.apache.poi.util.POILogFactory; import org.apache.poi.util.POILogger; +import org.apache.poi.util.Removal; /** * Escher container records store other escher records as children. @@ -31,7 +35,7 @@ import org.apache.poi.util.POILogger; * the standard header used by all escher records. This one record is * used to represent many different types of records. */ -public final class EscherContainerRecord extends EscherRecord { +public final class EscherContainerRecord extends EscherRecord implements Iterable { public static final short DGG_CONTAINER = (short)0xF000; public static final short BSTORE_CONTAINER = (short)0xF001; public static final short DG_CONTAINER = (short)0xF002; @@ -97,17 +101,13 @@ public final class EscherContainerRecord extends EscherRecord { LittleEndian.putShort(data, offset, getOptions()); LittleEndian.putShort(data, offset+2, getRecordId()); int remainingBytes = 0; - Iterator iterator = _childRecords.iterator(); - while (iterator.hasNext()) { - EscherRecord r = iterator.next(); + for (EscherRecord r : this) { remainingBytes += r.getRecordSize(); } remainingBytes += _remainingLength; LittleEndian.putInt(data, offset+4, remainingBytes); int pos = offset+8; - iterator = _childRecords.iterator(); - while (iterator.hasNext()) { - EscherRecord r = iterator.next(); + for (EscherRecord r : this) { pos += r.serialize(pos, data, listener ); } @@ -118,9 +118,7 @@ public final class EscherContainerRecord extends EscherRecord { @Override public int getRecordSize() { int childRecordsSize = 0; - Iterator iterator = _childRecords.iterator(); - while (iterator.hasNext()) { - EscherRecord r = iterator.next(); + for (EscherRecord r : this) { childRecordsSize += r.getRecordSize(); } return 8 + childRecordsSize; @@ -134,9 +132,7 @@ public final class EscherContainerRecord extends EscherRecord { * @return true, if any child has the given recordId */ public boolean hasChildOfType(short recordId) { - Iterator iterator = _childRecords.iterator(); - while (iterator.hasNext()) { - EscherRecord r = iterator.next(); + for (EscherRecord r : this) { if(r.getRecordId() == recordId) { return true; } @@ -158,8 +154,20 @@ public final class EscherContainerRecord extends EscherRecord { /** * @return an iterator over the child records + * @deprecated POI 3.16 beta 1. use iterator() or loop over the container record instead, + * e.g. "for (EscherRecord r : container) ..." */ + @Removal(version="3.18") + @Deprecated public Iterator getChildIterator() { + return iterator(); + } + + /** + * @return an iterator over the child records + */ + @Override + public Iterator iterator() { return Collections.unmodifiableList(_childRecords).iterator(); } @@ -195,9 +203,7 @@ public final class EscherContainerRecord extends EscherRecord { */ public List getChildContainers() { List containers = new ArrayList(); - Iterator iterator = _childRecords.iterator(); - while (iterator.hasNext()) { - EscherRecord r = iterator.next(); + for (EscherRecord r : this) { if(r instanceof EscherContainerRecord) { containers.add((EscherContainerRecord) r); } @@ -228,9 +234,7 @@ public final class EscherContainerRecord extends EscherRecord { @Override public void display(PrintWriter w, int indent) { super.display(w, indent); - for (Iterator iterator = _childRecords.iterator(); iterator.hasNext();) - { - EscherRecord escherRecord = iterator.next(); + for (EscherRecord escherRecord : this) { escherRecord.display(w, indent + 1); } } @@ -252,8 +256,10 @@ public final class EscherContainerRecord extends EscherRecord { */ public void addChildBefore(EscherRecord record, int insertBeforeRecordId) { int idx = 0; - for (EscherRecord rec : _childRecords) { - if(rec.getRecordId() == (short)insertBeforeRecordId) break; + for (EscherRecord rec : this) { + if(rec.getRecordId() == (short)insertBeforeRecordId) { + break; + } // TODO - keep looping? Do we expect multiple matches? idx++; } @@ -271,10 +277,7 @@ public final class EscherContainerRecord extends EscherRecord { children.append( " children: " + nl ); int count = 0; - for ( Iterator iterator = _childRecords.iterator(); iterator - .hasNext(); ) - { - EscherRecord record = iterator.next(); + for ( EscherRecord record : this ) { children.append( " Child " + count + ":" + nl ); String childResult = String.valueOf( record ); childResult = childResult.replaceAll( "\n", "\n " ); @@ -298,22 +301,16 @@ public final class EscherContainerRecord extends EscherRecord { public String toXml(String tab) { StringBuilder builder = new StringBuilder(); builder.append(tab).append(formatXmlRecordHeader(getRecordName(), HexDump.toHex(getRecordId()), HexDump.toHex(getVersion()), HexDump.toHex(getInstance()))); - for ( Iterator iterator = _childRecords.iterator(); iterator - .hasNext(); ) - { - EscherRecord record = iterator.next(); + for ( EscherRecord record : this ) { builder.append(record.toXml(tab+"\t")); } builder.append(tab).append("\n"); return builder.toString(); } - public T getChildById( short recordId ) - { - for ( EscherRecord childRecord : _childRecords ) - { - if ( childRecord.getRecordId() == recordId ) - { + public T getChildById( short recordId ) { + for ( EscherRecord childRecord : this ) { + if ( childRecord.getRecordId() == recordId ) { @SuppressWarnings( "unchecked" ) final T result = (T) childRecord; return result; @@ -329,9 +326,7 @@ public final class EscherContainerRecord extends EscherRecord { * @param out - list to store found records */ public void getRecordsById(short recordId, List out){ - Iterator iterator = _childRecords.iterator(); - while (iterator.hasNext()) { - EscherRecord r = iterator.next(); + for (EscherRecord r : this) { if(r instanceof EscherContainerRecord) { EscherContainerRecord c = (EscherContainerRecord)r; c.getRecordsById(recordId, out ); diff --git a/src/java/org/apache/poi/hssf/dev/BiffViewer.java b/src/java/org/apache/poi/hssf/dev/BiffViewer.java index 5465f0ff10..04e965d441 100644 --- a/src/java/org/apache/poi/hssf/dev/BiffViewer.java +++ b/src/java/org/apache/poi/hssf/dev/BiffViewer.java @@ -194,6 +194,7 @@ import org.apache.poi.hssf.record.pivottable.ViewSourceRecord; import org.apache.poi.hssf.usermodel.HSSFWorkbook; import org.apache.poi.poifs.filesystem.NPOIFSFileSystem; import org.apache.poi.util.HexDump; +import org.apache.poi.util.IOUtils; import org.apache.poi.util.LittleEndian; import org.apache.poi.util.POILogFactory; import org.apache.poi.util.POILogger; @@ -251,12 +252,10 @@ public final class BiffViewer { } temp.add(record); - if (dumpInterpretedRecords) { - for (String header : recListener.getRecentHeaders()) { - ps.println(header); - } - ps.print(record.toString()); + for (String header : recListener.getRecentHeaders()) { + ps.println(header); } + ps.print(record.toString()); } else { recStream.readRemainder(); } @@ -560,33 +559,26 @@ public final class BiffViewer { pw = new PrintWriter(new OutputStreamWriter(System.out, Charset.defaultCharset())); } + NPOIFSFileSystem fs = null; + InputStream is = null; try { - NPOIFSFileSystem fs = new NPOIFSFileSystem(cmdArgs.getFile(), true); - try { - InputStream is = getPOIFSInputStream(fs); + fs = new NPOIFSFileSystem(cmdArgs.getFile(), true); + is = getPOIFSInputStream(fs); - try { - if (cmdArgs.shouldOutputRawHexOnly()) { - int size = is.available(); - byte[] data = new byte[size]; - - is.read(data); - HexDump.dump(data, 0, System.out, 0); - } else { - boolean dumpInterpretedRecords = cmdArgs.shouldDumpRecordInterpretations(); - boolean dumpHex = cmdArgs.shouldDumpBiffHex(); - boolean zeroAlignHexDump = dumpInterpretedRecords; // TODO - fix non-zeroAlign - runBiffViewer(pw, is, dumpInterpretedRecords, dumpHex, zeroAlignHexDump, - cmdArgs.suppressHeader()); - } - } finally { - is.close(); - } - } finally { - fs.close(); + if (cmdArgs.shouldOutputRawHexOnly()) { + byte[] data = IOUtils.toByteArray(is); + HexDump.dump(data, 0, System.out, 0); + } else { + boolean dumpInterpretedRecords = cmdArgs.shouldDumpRecordInterpretations(); + boolean dumpHex = cmdArgs.shouldDumpBiffHex(); + boolean zeroAlignHexDump = dumpInterpretedRecords; // TODO - fix non-zeroAlign + runBiffViewer(pw, is, dumpInterpretedRecords, dumpHex, zeroAlignHexDump, + cmdArgs.suppressHeader()); } } finally { - pw.close(); + IOUtils.closeQuietly(is); + IOUtils.closeQuietly(fs); + IOUtils.closeQuietly(pw); } } diff --git a/src/java/org/apache/poi/hssf/extractor/OldExcelExtractor.java b/src/java/org/apache/poi/hssf/extractor/OldExcelExtractor.java index 8cc1d96bd8..3d6bc3515c 100644 --- a/src/java/org/apache/poi/hssf/extractor/OldExcelExtractor.java +++ b/src/java/org/apache/poi/hssf/extractor/OldExcelExtractor.java @@ -82,28 +82,18 @@ public class OldExcelExtractor implements Closeable { return; } catch (OldExcelFormatException e) { // will be handled by workaround below - if (poifs != null) { - poifs.close(); - } } catch (NotOLE2FileException e) { // will be handled by workaround below - if (poifs != null) { - poifs.close(); - } } catch (IOException e) { // ensure streams are closed correctly - if (poifs != null) { - poifs.close(); - } - throw e; } catch (RuntimeException e) { // ensure streams are closed correctly - if (poifs != null) { - poifs.close(); - } - throw e; + } finally { + if (toClose == null) { + IOUtils.closeQuietly(poifs); + } } @SuppressWarnings("resource") diff --git a/src/java/org/apache/poi/hssf/model/InternalWorkbook.java b/src/java/org/apache/poi/hssf/model/InternalWorkbook.java index 3a5fc1ef55..843f142289 100644 --- a/src/java/org/apache/poi/hssf/model/InternalWorkbook.java +++ b/src/java/org/apache/poi/hssf/model/InternalWorkbook.java @@ -110,6 +110,7 @@ import org.apache.poi.util.Internal; import org.apache.poi.util.LocaleUtil; import org.apache.poi.util.POILogFactory; import org.apache.poi.util.POILogger; +import org.apache.poi.util.RecordFormatException; /** * Low level model implementation of a Workbook. Provides creational methods @@ -2292,8 +2293,7 @@ public final class InternalWorkbook { EscherDggRecord dgg = null; EscherContainerRecord bStore = null; - for(Iterator it = cr.getChildIterator(); it.hasNext();) { - EscherRecord er = it.next(); + for(EscherRecord er : cr) { if(er instanceof EscherDggRecord) { dgg = (EscherDggRecord)er; } else if (er.getRecordId() == EscherContainerRecord.BSTORE_CONTAINER) { @@ -2590,8 +2590,7 @@ public final class InternalWorkbook { dgg.setDrawingsSaved(dgg.getDrawingsSaved() + 1); EscherDgRecord dg = null; - for(Iterator it = escherContainer.getChildIterator(); it.hasNext();) { - EscherRecord er = it.next(); + for(EscherRecord er : escherContainer) { if(er instanceof EscherDgRecord) { dg = (EscherDgRecord)er; //update id of the drawing in the cloned sheet @@ -2604,6 +2603,9 @@ public final class InternalWorkbook { for(EscherRecord shapeChildRecord : shapeContainer.getChildRecords()) { int recordId = shapeChildRecord.getRecordId(); if (recordId == EscherSpRecord.RECORD_ID){ + if (dg == null) { + throw new RecordFormatException("EscherDgRecord wasn't set/processed before."); + } EscherSpRecord sp = (EscherSpRecord)shapeChildRecord; int shapeId = drawingManager.allocateShapeId((short)dgId, dg); //allocateShapeId increments the number of shapes. roll back to the previous value diff --git a/src/java/org/apache/poi/hssf/record/common/UnicodeString.java b/src/java/org/apache/poi/hssf/record/common/UnicodeString.java index 4cefdee102..9e758782d2 100644 --- a/src/java/org/apache/poi/hssf/record/common/UnicodeString.java +++ b/src/java/org/apache/poi/hssf/record/common/UnicodeString.java @@ -748,7 +748,7 @@ public class UnicodeString implements Comparable { } } - if (extendedDataSize > 0) { + if (extendedDataSize > 0 && field_5_ext_rst != null) { field_5_ext_rst.serialize(out); } } diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFShapeFactory.java b/src/java/org/apache/poi/hssf/usermodel/HSSFShapeFactory.java index 772ee5821d..5135078c00 100644 --- a/src/java/org/apache/poi/hssf/usermodel/HSSFShapeFactory.java +++ b/src/java/org/apache/poi/hssf/usermodel/HSSFShapeFactory.java @@ -17,14 +17,26 @@ package org.apache.poi.hssf.usermodel; -import org.apache.poi.ddf.*; -import org.apache.poi.hssf.record.*; -import org.apache.poi.poifs.filesystem.DirectoryNode; - -import java.util.Iterator; import java.util.List; import java.util.Map; +import org.apache.poi.ddf.EscherClientDataRecord; +import org.apache.poi.ddf.EscherContainerRecord; +import org.apache.poi.ddf.EscherOptRecord; +import org.apache.poi.ddf.EscherProperties; +import org.apache.poi.ddf.EscherProperty; +import org.apache.poi.ddf.EscherRecord; +import org.apache.poi.ddf.EscherTextboxRecord; +import org.apache.poi.hssf.record.CommonObjectDataSubRecord; +import org.apache.poi.hssf.record.EmbeddedObjectRefSubRecord; +import org.apache.poi.hssf.record.EscherAggregate; +import org.apache.poi.hssf.record.ObjRecord; +import org.apache.poi.hssf.record.Record; +import org.apache.poi.hssf.record.SubRecord; +import org.apache.poi.hssf.record.TextObjectRecord; +import org.apache.poi.poifs.filesystem.DirectoryNode; +import org.apache.poi.util.RecordFormatException; + /** * Factory class for producing Excel Shapes from Escher records */ @@ -58,7 +70,7 @@ public class HSSFShapeFactory { ObjRecord objRecord = null; TextObjectRecord txtRecord = null; - for (EscherRecord record : container.getChildRecords()) { + for (EscherRecord record : container) { switch (record.getRecordId()) { case EscherClientDataRecord.RECORD_ID: objRecord = (ObjRecord) shapeToObj.get(record); @@ -70,6 +82,9 @@ public class HSSFShapeFactory { break; } } + if (objRecord == null) { + throw new RecordFormatException("EscherClientDataRecord can't be found."); + } if (isEmbeddedObject(objRecord)) { HSSFObjectData objectData = new HSSFObjectData(container, objRecord, root); out.addShape(objectData); @@ -117,9 +132,7 @@ public class HSSFShapeFactory { } private static boolean isEmbeddedObject(ObjRecord obj) { - Iterator subRecordIter = obj.getSubRecords().iterator(); - while (subRecordIter.hasNext()) { - SubRecord sub = subRecordIter.next(); + for (SubRecord sub : obj.getSubRecords()) { if (sub instanceof EmbeddedObjectRefSubRecord) { return true; } diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFSheet.java b/src/java/org/apache/poi/hssf/usermodel/HSSFSheet.java index 471856a9e5..4e5f280951 100644 --- a/src/java/org/apache/poi/hssf/usermodel/HSSFSheet.java +++ b/src/java/org/apache/poi/hssf/usermodel/HSSFSheet.java @@ -1688,7 +1688,7 @@ public final class HSSFSheet implements org.apache.poi.ss.usermodel.Sheet { // Need to walk backward to find the last non-blank row // NOTE: n is always negative here _lastrow = Math.min(endRow + n, SpreadsheetVersion.EXCEL97.getLastRowIndex()); - for (int i = endRow - 1; i > endRow + n; i++) { + for (int i = endRow - 1; i > endRow + n; i--) { if (getRow(i) != null) { _lastrow = i; break; diff --git a/src/scratchpad/src/org/apache/poi/hslf/record/PPDrawingGroup.java b/src/scratchpad/src/org/apache/poi/hslf/record/PPDrawingGroup.java index 0cdb1f6273..d655e5a4fe 100644 --- a/src/scratchpad/src/org/apache/poi/hslf/record/PPDrawingGroup.java +++ b/src/scratchpad/src/org/apache/poi/hslf/record/PPDrawingGroup.java @@ -71,17 +71,14 @@ public final class PPDrawingGroup extends RecordAtom { public void writeOut(OutputStream out) throws IOException { ByteArrayOutputStream bout = new ByteArrayOutputStream(); - Iterator iter = dggContainer.getChildIterator(); - while (iter.hasNext()) { - EscherRecord r = iter.next(); + for (EscherRecord r : dggContainer) { if (r.getRecordId() == EscherContainerRecord.BSTORE_CONTAINER){ EscherContainerRecord bstore = (EscherContainerRecord)r; ByteArrayOutputStream b2 = new ByteArrayOutputStream(); - for (Iterator it= bstore.getChildIterator(); it.hasNext();) { - EscherBSERecord bse = (EscherBSERecord)it.next(); + for (EscherRecord br : bstore) { byte[] b = new byte[36+8]; - bse.serialize(0, b); + br.serialize(0, b); b2.write(b); } byte[] bstorehead = new byte[8]; @@ -120,8 +117,7 @@ public final class PPDrawingGroup extends RecordAtom { public EscherDggRecord getEscherDggRecord(){ if(dgg == null){ - for(Iterator it = dggContainer.getChildIterator(); it.hasNext();){ - EscherRecord r = it.next(); + for(EscherRecord r : dggContainer){ if(r instanceof EscherDggRecord){ dgg = (EscherDggRecord)r; break; diff --git a/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFGroupShape.java b/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFGroupShape.java index bd90edfec4..3d4d57fbdb 100644 --- a/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFGroupShape.java +++ b/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFGroupShape.java @@ -272,17 +272,17 @@ implements HSLFShapeContainer, GroupShape { @Override public List getShapes() { - // Out escher container record should contain several - // SpContainers, the first of which is the group shape itself - Iterator iter = getSpContainer().getChildIterator(); - - // Don't include the first SpContainer, it is always NotPrimitive - if (iter.hasNext()) { - iter.next(); - } + // Our escher container record should contain several + // SpContainers, the first of which is the group shape itself List shapeList = new ArrayList(); - while (iter.hasNext()) { - EscherRecord r = iter.next(); + boolean isFirst = true; + for (EscherRecord r : getSpContainer()) { + if (isFirst) { + // Don't include the first SpContainer, it is always NotPrimitive + isFirst = false; + continue; + } + if(r instanceof EscherContainerRecord) { // Create the Shape for it EscherContainerRecord container = (EscherContainerRecord)r; diff --git a/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFSheet.java b/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFSheet.java index 72042769d1..5bfd3bbc9c 100644 --- a/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFSheet.java +++ b/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFSheet.java @@ -151,8 +151,7 @@ public abstract class HSLFSheet implements HSLFShapeContainer, Sheet it = dg.getChildIterator(); it.hasNext();) { - EscherRecord rec = it.next(); + for (EscherRecord rec : dg) { if (rec.getRecordId() == EscherContainerRecord.SPGR_CONTAINER) { spgr = (EscherContainerRecord) rec; break; @@ -163,13 +162,15 @@ public abstract class HSLFSheet implements HSLFShapeContainer, Sheet shapeList = new ArrayList(); - Iterator it = spgr.getChildIterator(); - if (it.hasNext()) { - // skip first item - it.next(); - } - for (; it.hasNext();) { - EscherContainerRecord sp = (EscherContainerRecord) it.next(); + boolean isFirst = true; + for (EscherRecord r : spgr) { + if (isFirst) { + // skip first item + isFirst = false; + continue; + } + + EscherContainerRecord sp = (EscherContainerRecord)r; HSLFShape sh = HSLFShapeFactory.createShape(sp, null); sh.setSheet(this); diff --git a/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFSlideShowEncrypted.java b/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFSlideShowEncrypted.java index b20af62ec9..1251904b21 100644 --- a/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFSlideShowEncrypted.java +++ b/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFSlideShowEncrypted.java @@ -376,14 +376,8 @@ public class HSLFSlideShowEncrypted implements Closeable { } catch (Exception e) { throw new EncryptedPowerPointFileException(e); } finally { - try { - if (ccos != null) { - ccos.close(); - } - los.close(); - } catch (IOException e) { - throw new EncryptedPowerPointFileException(e); - } + IOUtils.closeQuietly(ccos); + IOUtils.closeQuietly(los); } } diff --git a/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFTextParagraph.java b/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFTextParagraph.java index 4cb3a310c3..60232d50a7 100644 --- a/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFTextParagraph.java +++ b/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFTextParagraph.java @@ -980,7 +980,7 @@ public final class HSLFTextParagraph implements TextParagraph 0) { + if(lastChart.legendRecord == null && lastChart.series.size() > 0) { HSSFSeries series = lastChart.series.get(lastChart.series.size()-1); series.seriesTitleText = str; } else { lastChart.chartTitleText = str; } - } else if (r instanceof LinkedDataRecord) { - LinkedDataRecord linkedDataRecord = (LinkedDataRecord) r; - if (lastSeries != null) { - lastSeries.insertData(linkedDataRecord); - } } else if(r instanceof ValueRangeRecord){ lastChart.valueRanges.add((ValueRangeRecord)r); } else if (r instanceof Record) { - if (lastChart != null) - { - Record record = (Record) r; - for (HSSFChartType type : HSSFChartType.values()) { - if (type == HSSFChartType.Unknown) - { - continue; - } - if (record.getSid() == type.getSid()) { - lastChart.type = type ; - break; - } + Record record = (Record) r; + for (HSSFChartType type : HSSFChartType.values()) { + if (type == HSSFChartType.Unknown) { + continue; + } + if (record.getSid() == type.getSid()) { + lastChart.type = type; + break; } } }