From 85084ee058032a1d55df42076d2d3d1f86d44b4a Mon Sep 17 00:00:00 2001 From: Andreas Beeker Date: Wed, 4 Jan 2017 01:06:30 +0000 Subject: [PATCH] SonarQube fixes git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1777252 13f79535-47bb-0310-9956-ffa450edef68 --- .../usermodel/examples/EmbeddedObjects.java | 33 ++- .../apache/poi/stress/HSSFFileHandler.java | 3 + .../usermodel/HSSFEvaluationWorkbook.java | 31 +-- .../poifs/crypt/ChunkedCipherInputStream.java | 5 +- .../crypt/cryptoapi/CryptoAPIDecryptor.java | 66 +++--- .../apache/poi/openxml4j/opc/ZipPackage.java | 188 +++++++++--------- .../poi/openxml4j/util/ZipSecureFile.java | 8 +- .../org/apache/poi/TestPOIXMLDocument.java | 4 +- .../poi/hwpf/model/CHPFormattedDiskPage.java | 27 +-- .../org/apache/poi/hwpf/model/PlexOfCps.java | 18 +- .../apache/poi/hwpf/usermodel/Picture.java | 5 +- 11 files changed, 190 insertions(+), 198 deletions(-) diff --git a/src/examples/src/org/apache/poi/xssf/usermodel/examples/EmbeddedObjects.java b/src/examples/src/org/apache/poi/xssf/usermodel/examples/EmbeddedObjects.java index 5d367316b3..4f65e1a73b 100644 --- a/src/examples/src/org/apache/poi/xssf/usermodel/examples/EmbeddedObjects.java +++ b/src/examples/src/org/apache/poi/xssf/usermodel/examples/EmbeddedObjects.java @@ -16,14 +16,14 @@ ==================================================================== */ package org.apache.poi.xssf.usermodel.examples; +import java.io.Closeable; import java.io.InputStream; -import org.apache.poi.hslf.usermodel.HSLFSlideShowImpl; +import org.apache.poi.hslf.usermodel.HSLFSlideShow; import org.apache.poi.hssf.usermodel.HSSFWorkbook; import org.apache.poi.hwpf.HWPFDocument; -import org.apache.poi.openxml4j.opc.OPCPackage; import org.apache.poi.openxml4j.opc.PackagePart; -import org.apache.poi.xslf.usermodel.XSLFSlideShow; +import org.apache.poi.xslf.usermodel.XMLSlideShow; import org.apache.poi.xssf.usermodel.XSSFWorkbook; import org.apache.poi.xwpf.usermodel.XWPFDocument; @@ -35,37 +35,32 @@ public class EmbeddedObjects { XSSFWorkbook workbook = new XSSFWorkbook(args[0]); for (PackagePart pPart : workbook.getAllEmbedds()) { String contentType = pPart.getContentType(); + InputStream is = pPart.getInputStream(); + Closeable document; if (contentType.equals("application/vnd.ms-excel")) { // Excel Workbook - either binary or OpenXML - HSSFWorkbook embeddedWorkbook = new HSSFWorkbook(pPart.getInputStream()); - embeddedWorkbook.close(); + document = new HSSFWorkbook(is); } else if (contentType.equals("application/vnd.openxmlformats-officedocument.spreadsheetml.sheet")) { // Excel Workbook - OpenXML file format - XSSFWorkbook embeddedWorkbook = new XSSFWorkbook(pPart.getInputStream()); - embeddedWorkbook.close(); + document = new XSSFWorkbook(is); } else if (contentType.equals("application/msword")) { // Word Document - binary (OLE2CDF) file format - HWPFDocument document = new HWPFDocument(pPart.getInputStream()); - document.close(); + document = new HWPFDocument(is); } else if (contentType.equals("application/vnd.openxmlformats-officedocument.wordprocessingml.document")) { // Word Document - OpenXML file format - XWPFDocument document = new XWPFDocument(pPart.getInputStream()); - document.close(); + document = new XWPFDocument(is); } else if (contentType.equals("application/vnd.ms-powerpoint")) { // PowerPoint Document - binary file format - HSLFSlideShowImpl slideShow = new HSLFSlideShowImpl(pPart.getInputStream()); - slideShow.close(); + document = new HSLFSlideShow(is); } else if (contentType.equals("application/vnd.openxmlformats-officedocument.presentationml.presentation")) { // PowerPoint Document - OpenXML file format - OPCPackage docPackage = OPCPackage.open(pPart.getInputStream()); - XSLFSlideShow slideShow = new XSLFSlideShow(docPackage); - slideShow.close(); + document = new XMLSlideShow(is); } else { // Any other type of embedded object. - System.out.println("Unknown Embedded Document: " + contentType); - InputStream inputStream = pPart.getInputStream(); - inputStream.close(); + document = is; } + document.close(); + is.close(); } workbook.close(); } diff --git a/src/integrationtest/org/apache/poi/stress/HSSFFileHandler.java b/src/integrationtest/org/apache/poi/stress/HSSFFileHandler.java index fcea580586..bef4043b04 100644 --- a/src/integrationtest/org/apache/poi/stress/HSSFFileHandler.java +++ b/src/integrationtest/org/apache/poi/stress/HSSFFileHandler.java @@ -37,6 +37,9 @@ public class HSSFFileHandler extends SpreadsheetHandler { handleWorkbook(wb); // TODO: some documents fail currently... + // Note - as of Bugzilla 48036 (svn r828244, r828247) POI is capable of evaluating + // IntesectionPtg. However it is still not capable of parsing it. + // So FormulaEvalTestData.xls now contains a few formulas that produce errors here. //HSSFFormulaEvaluator evaluator = new HSSFFormulaEvaluator(wb); //evaluator.evaluateAll(); diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFEvaluationWorkbook.java b/src/java/org/apache/poi/hssf/usermodel/HSSFEvaluationWorkbook.java index 1feb96c3e1..b0fa3e28f7 100644 --- a/src/java/org/apache/poi/hssf/usermodel/HSSFEvaluationWorkbook.java +++ b/src/java/org/apache/poi/hssf/usermodel/HSSFEvaluationWorkbook.java @@ -17,7 +17,6 @@ package org.apache.poi.hssf.usermodel; -import org.apache.poi.hssf.model.HSSFFormulaParser; import org.apache.poi.hssf.model.InternalWorkbook; import org.apache.poi.hssf.record.NameRecord; import org.apache.poi.hssf.record.aggregates.FormulaRecordAggregate; @@ -26,10 +25,8 @@ import org.apache.poi.ss.formula.EvaluationCell; import org.apache.poi.ss.formula.EvaluationName; import org.apache.poi.ss.formula.EvaluationSheet; import org.apache.poi.ss.formula.EvaluationWorkbook; -import org.apache.poi.ss.formula.FormulaParseException; import org.apache.poi.ss.formula.FormulaParsingWorkbook; import org.apache.poi.ss.formula.FormulaRenderingWorkbook; -import org.apache.poi.ss.formula.FormulaType; import org.apache.poi.ss.formula.SheetIdentifier; import org.apache.poi.ss.formula.SheetRangeIdentifier; import org.apache.poi.ss.formula.ptg.Area3DPtg; @@ -42,15 +39,12 @@ import org.apache.poi.ss.usermodel.Table; import org.apache.poi.ss.util.AreaReference; import org.apache.poi.ss.util.CellReference; import org.apache.poi.util.Internal; -import org.apache.poi.util.POILogFactory; -import org.apache.poi.util.POILogger; /** * Internal POI use only */ @Internal public final class HSSFEvaluationWorkbook implements FormulaRenderingWorkbook, EvaluationWorkbook, FormulaParsingWorkbook { - private static POILogger logger = POILogFactory.getLogger(HSSFEvaluationWorkbook.class); private final HSSFWorkbook _uBook; private final InternalWorkbook _iBook; @@ -115,6 +109,7 @@ public final class HSSFEvaluationWorkbook implements FormulaRenderingWorkbook, E * @param sheetIndex the 0-based index of the sheet this formula belongs to. * The sheet index is required to resolve sheet-level names. -1 means workbook-global names */ + @Override public EvaluationName getName(String name, int sheetIndex) { for(int i=0; i < _iBook.getNumNames(); i++) { NameRecord nr = _iBook.getNameRecord(i); @@ -226,22 +221,12 @@ public final class HSSFEvaluationWorkbook implements FormulaRenderingWorkbook, E } @Override - @SuppressWarnings("unused") public Ptg[] getFormulaTokens(EvaluationCell evalCell) { HSSFCell cell = ((HSSFEvaluationCell)evalCell).getHSSFCell(); - if (false) { - // re-parsing the formula text also works, but is a waste of time - // It is useful from time to time to run all unit tests with this code - // to make sure that all formulas POI can evaluate can also be parsed. - try { - return HSSFFormulaParser.parse(cell.getCellFormula(), _uBook, FormulaType.CELL, _uBook.getSheetIndex(cell.getSheet())); - } catch (FormulaParseException e) { - // Note - as of Bugzilla 48036 (svn r828244, r828247) POI is capable of evaluating - // IntesectionPtg. However it is still not capable of parsing it. - // So FormulaEvalTestData.xls now contains a few formulas that produce errors here. - logger.log( POILogger.ERROR, e.getMessage()); - } - } + // re-parsing the formula text also works, but is a waste of time + // return HSSFFormulaParser.parse(cell.getCellFormula(), _uBook, FormulaType.CELL, _uBook.getSheetIndex(cell.getSheet())); + // It is useful within the tests to make sure that all formulas POI can evaluate can also be parsed. + // see HSSFFileHandler.handleFile instead FormulaRecordAggregate fra = (FormulaRecordAggregate) cell.getCellValueRecord(); return fra.getFormulaTokens(); } @@ -259,21 +244,27 @@ public final class HSSFEvaluationWorkbook implements FormulaRenderingWorkbook, E _nameRecord = nameRecord; _index = index; } + @Override public Ptg[] getNameDefinition() { return _nameRecord.getNameDefinition(); } + @Override public String getNameText() { return _nameRecord.getNameText(); } + @Override public boolean hasFormula() { return _nameRecord.hasFormula(); } + @Override public boolean isFunctionName() { return _nameRecord.isFunctionName(); } + @Override public boolean isRange() { return _nameRecord.hasFormula(); // TODO - is this right? } + @Override public NamePtg createPtg() { return new NamePtg(_index); } diff --git a/src/java/org/apache/poi/poifs/crypt/ChunkedCipherInputStream.java b/src/java/org/apache/poi/poifs/crypt/ChunkedCipherInputStream.java index db3d724e4c..df27d58484 100644 --- a/src/java/org/apache/poi/poifs/crypt/ChunkedCipherInputStream.java +++ b/src/java/org/apache/poi/poifs/crypt/ChunkedCipherInputStream.java @@ -180,7 +180,10 @@ public abstract class ChunkedCipherInputStream extends LittleEndianInputStream { initCipherForBlock(cipher, index); if (lastIndex != index) { - super.skip((index - lastIndex) << chunkBits); + long skipN = (index - lastIndex) << chunkBits; + if (super.skip(skipN) < skipN) { + throw new EOFException("buffer underrun"); + }; } lastIndex = index + 1; diff --git a/src/java/org/apache/poi/poifs/crypt/cryptoapi/CryptoAPIDecryptor.java b/src/java/org/apache/poi/poifs/crypt/cryptoapi/CryptoAPIDecryptor.java index 1a31d2a0c5..edbd6ea3e5 100644 --- a/src/java/org/apache/poi/poifs/crypt/cryptoapi/CryptoAPIDecryptor.java +++ b/src/java/org/apache/poi/poifs/crypt/cryptoapi/CryptoAPIDecryptor.java @@ -18,6 +18,7 @@ package org.apache.poi.poifs.crypt.cryptoapi; import java.io.ByteArrayOutputStream; +import java.io.EOFException; import java.io.IOException; import java.io.InputStream; import java.security.GeneralSecurityException; @@ -168,37 +169,42 @@ public class CryptoAPIDecryptor extends Decryptor implements Cloneable { dis.close(); CryptoAPIDocumentInputStream sbis = new CryptoAPIDocumentInputStream(this, bos.toByteArray()); LittleEndianInputStream leis = new LittleEndianInputStream(sbis); - int streamDescriptorArrayOffset = (int) leis.readUInt(); - /* int streamDescriptorArraySize = (int) */ leis.readUInt(); - sbis.skip(streamDescriptorArrayOffset - 8L); - sbis.setBlock(0); - int encryptedStreamDescriptorCount = (int) leis.readUInt(); - StreamDescriptorEntry entries[] = new StreamDescriptorEntry[encryptedStreamDescriptorCount]; - for (int i = 0; i < encryptedStreamDescriptorCount; i++) { - StreamDescriptorEntry entry = new StreamDescriptorEntry(); - entries[i] = entry; - entry.streamOffset = (int) leis.readUInt(); - entry.streamSize = (int) leis.readUInt(); - entry.block = leis.readUShort(); - int nameSize = leis.readUByte(); - entry.flags = leis.readUByte(); - // boolean isStream = StreamDescriptorEntry.flagStream.isSet(entry.flags); - entry.reserved2 = leis.readInt(); - entry.streamName = StringUtil.readUnicodeLE(leis, nameSize); - leis.readShort(); - assert(entry.streamName.length() == nameSize); - } - - for (StreamDescriptorEntry entry : entries) { - sbis.seek(entry.streamOffset); - sbis.setBlock(entry.block); - InputStream is = new BoundedInputStream(sbis, entry.streamSize); - fsOut.createDocument(is, entry.streamName); - is.close(); + try { + int streamDescriptorArrayOffset = (int) leis.readUInt(); + /* int streamDescriptorArraySize = (int) */ leis.readUInt(); + long skipN = streamDescriptorArrayOffset - 8L; + if (sbis.skip(skipN) < skipN) { + throw new EOFException("buffer underrun"); + } + sbis.setBlock(0); + int encryptedStreamDescriptorCount = (int) leis.readUInt(); + StreamDescriptorEntry entries[] = new StreamDescriptorEntry[encryptedStreamDescriptorCount]; + for (int i = 0; i < encryptedStreamDescriptorCount; i++) { + StreamDescriptorEntry entry = new StreamDescriptorEntry(); + entries[i] = entry; + entry.streamOffset = (int) leis.readUInt(); + entry.streamSize = (int) leis.readUInt(); + entry.block = leis.readUShort(); + int nameSize = leis.readUByte(); + entry.flags = leis.readUByte(); + // boolean isStream = StreamDescriptorEntry.flagStream.isSet(entry.flags); + entry.reserved2 = leis.readInt(); + entry.streamName = StringUtil.readUnicodeLE(leis, nameSize); + leis.readShort(); + assert(entry.streamName.length() == nameSize); + } + + for (StreamDescriptorEntry entry : entries) { + sbis.seek(entry.streamOffset); + sbis.setBlock(entry.block); + InputStream is = new BoundedInputStream(sbis, entry.streamSize); + fsOut.createDocument(is, entry.streamName); + is.close(); + } + } finally { + IOUtils.closeQuietly(leis); + IOUtils.closeQuietly(sbis); } - - leis.close(); - sbis.close(); sbis = null; return fsOut; } diff --git a/src/ooxml/java/org/apache/poi/openxml4j/opc/ZipPackage.java b/src/ooxml/java/org/apache/poi/openxml4j/opc/ZipPackage.java index 7a147b0524..d20cea0cb4 100644 --- a/src/ooxml/java/org/apache/poi/openxml4j/opc/ZipPackage.java +++ b/src/ooxml/java/org/apache/poi/openxml4j/opc/ZipPackage.java @@ -45,6 +45,7 @@ import org.apache.poi.openxml4j.util.ZipEntrySource; import org.apache.poi.openxml4j.util.ZipFileZipEntrySource; import org.apache.poi.openxml4j.util.ZipInputStreamZipEntrySource; import org.apache.poi.openxml4j.util.ZipSecureFile.ThresholdInputStream; +import org.apache.poi.util.IOUtils; import org.apache.poi.util.POILogFactory; import org.apache.poi.util.POILogger; import org.apache.poi.util.TempFile; @@ -56,7 +57,7 @@ public final class ZipPackage extends OPCPackage { private static final String MIMETYPE = "mimetype"; private static final String SETTINGS_XML = "settings.xml"; - private static final POILogger logger = POILogFactory.getLogger(ZipPackage.class); + private static final POILogger LOG = POILogFactory.getLogger(ZipPackage.class); /** * Zip archive, as either a file on disk, @@ -74,7 +75,7 @@ public final class ZipPackage extends OPCPackage { try { this.contentTypeManager = new ZipContentTypeManager(null, this); } catch (InvalidFormatException e) { - logger.log(POILogger.WARN,"Could not parse ZipPackage", e); + LOG.log(POILogger.WARN,"Could not parse ZipPackage", e); } } @@ -98,11 +99,7 @@ public final class ZipPackage extends OPCPackage { try { this.zipArchive = new ZipInputStreamZipEntrySource(zis); } catch (final IOException e) { - try { - zis.close(); - } catch (final IOException e2) { - throw new IOException("Failed to close zip input stream while cleaning up. " + e.getMessage(), e2); - } + IOUtils.closeQuietly(zis); throw new IOException("Failed to read zip entry source", e); } } @@ -141,7 +138,7 @@ public final class ZipPackage extends OPCPackage { if (access == PackageAccess.WRITE) { throw new InvalidOperationException("Can't open the specified file: '" + file + "'", e); } - logger.log(POILogger.ERROR, "Error in zip file "+file+" - falling back to stream processing (i.e. ignoring zip central directory)"); + LOG.log(POILogger.ERROR, "Error in zip file "+file+" - falling back to stream processing (i.e. ignoring zip central directory)"); ze = openZipEntrySourceStream(file); } this.zipArchive = ze; @@ -163,13 +160,13 @@ public final class ZipPackage extends OPCPackage { // read from the file input stream return openZipEntrySourceStream(fis); } catch (final Exception e) { - try { - // abort: close the file input stream - fis.close(); - } catch (final IOException e2) { - throw new InvalidOperationException("Could not close the specified file input stream from file: '" + file + "'", e2); + // abort: close the file input stream + IOUtils.closeQuietly(fis); + if (e instanceof InvalidOperationException) { + throw (InvalidOperationException)e; + } else { + throw new InvalidOperationException("Failed to read the file input stream from file: '" + file + "'", e); } - throw new InvalidOperationException("Failed to read the file input stream from file: '" + file + "'", e); } } @@ -189,13 +186,13 @@ public final class ZipPackage extends OPCPackage { // read from the zip input stream return openZipEntrySourceStream(zis); } catch (final Exception e) { - try { - // abort: close the zip input stream - zis.close(); - } catch (final IOException e2) { - throw new InvalidOperationException("Failed to read the zip entry source stream and could not close the zip input stream", e2); + // abort: close the zip input stream + IOUtils.closeQuietly(zis); + if (e instanceof InvalidOperationException) { + throw (InvalidOperationException)e; + } else { + throw new InvalidOperationException("Failed to read the zip entry source stream", e); } - throw new InvalidOperationException("Failed to read the zip entry source stream", e); } } @@ -293,7 +290,7 @@ public final class ZipPackage extends OPCPackage { // Fallback exception throw new InvalidFormatException( - "Package should contain a content type part [M1.13]"); + "Package should contain a content type part [M1.13]"); } // Now create all the relationships @@ -304,7 +301,9 @@ public final class ZipPackage extends OPCPackage { while (entries.hasMoreElements()) { ZipEntry entry = entries.nextElement(); PackagePartName partName = buildPartName(entry); - if(partName == null) continue; + if(partName == null) { + continue; + } // Only proceed for Relationships at this stage String contentType = contentTypeManager.getContentType(partName); @@ -323,7 +322,9 @@ public final class ZipPackage extends OPCPackage { while (entries.hasMoreElements()) { ZipEntry entry = entries.nextElement(); PackagePartName partName = buildPartName(entry); - if(partName == null) continue; + if(partName == null) { + continue; + } String contentType = contentTypeManager.getContentType(partName); if (contentType != null && contentType.equals(ContentTypes.RELATIONSHIPS_PART)) { @@ -338,9 +339,8 @@ public final class ZipPackage extends OPCPackage { } } else { throw new InvalidFormatException( - "The part " - + partName.getURI().getPath() - + " does not have any content type ! Rule: Package require content types when retrieving a part from a package. [M.1.14]"); + "The part " + partName.getURI().getPath() + + " does not have any content type ! Rule: Package require content types when retrieving a part from a package. [M.1.14]"); } } @@ -363,7 +363,7 @@ public final class ZipPackage extends OPCPackage { .getOPCNameFromZipItemName(entry.getName())); } catch (Exception e) { // We assume we can continue, even in degraded mode ... - logger.log(POILogger.WARN,"Entry " + LOG.log(POILogger.WARN,"Entry " + entry.getName() + " is not valid, so this part won't be add to the package.", e); return null; @@ -383,17 +383,18 @@ public final class ZipPackage extends OPCPackage { @Override protected PackagePart createPartImpl(PackagePartName partName, String contentType, boolean loadRelationships) { - if (contentType == null) + if (contentType == null) { throw new IllegalArgumentException("contentType"); + } - if (partName == null) + if (partName == null) { throw new IllegalArgumentException("partName"); + } try { - return new MemoryPackagePart(this, partName, contentType, - loadRelationships); + return new MemoryPackagePart(this, partName, contentType, loadRelationships); } catch (InvalidFormatException e) { - logger.log(POILogger.WARN, e); + LOG.log(POILogger.WARN, e); return null; } } @@ -406,8 +407,9 @@ public final class ZipPackage extends OPCPackage { */ @Override protected void removePartImpl(PackagePartName partName) { - if (partName == null) + if (partName == null) { throw new IllegalArgumentException("partUri"); + } } /** @@ -428,43 +430,39 @@ public final class ZipPackage extends OPCPackage { // Flush the package flush(); + if (this.originalPackagePath == null || "".equals(this.originalPackagePath)) { + return; + } + // Save the content - if (this.originalPackagePath != null - && !"".equals(this.originalPackagePath)) { - File targetFile = new File(this.originalPackagePath); - if (targetFile.exists()) { - // Case of a package previously open - - File tempFile = TempFile.createTempFile( - generateTempFileName(FileHelper - .getDirectory(targetFile)), ".tmp"); - - // Save the final package to a temporary file - try { - save(tempFile); - } finally { - try { - // Close the current zip file, so we can - // overwrite it on all platforms - this.zipArchive.close(); - // Copy the new file over the old one - FileHelper.copyFile(tempFile, targetFile); - } finally { - // Either the save operation succeed or not, we delete the - // temporary file - if (!tempFile.delete()) { - logger - .log(POILogger.WARN,"The temporary file: '" - + targetFile.getAbsolutePath() - + "' cannot be deleted ! Make sure that no other application use it."); - } - } + File targetFile = new File(this.originalPackagePath); + if (!targetFile.exists()) { + throw new InvalidOperationException( + "Can't close a package not previously open with the open() method !"); + } + + // Case of a package previously open + String tempFileName = generateTempFileName(FileHelper.getDirectory(targetFile)); + File tempFile = TempFile.createTempFile(tempFileName, ".tmp"); + + // Save the final package to a temporary file + try { + save(tempFile); + } finally { + // Close the current zip file, so we can overwrite it on all platforms + IOUtils.closeQuietly(this.zipArchive); + try { + // Copy the new file over the old one + FileHelper.copyFile(tempFile, targetFile); + } finally { + // Either the save operation succeed or not, we delete the temporary file + if (!tempFile.delete()) { + LOG.log(POILogger.WARN, "The temporary file: '" + + targetFile.getAbsolutePath() + + "' cannot be deleted ! Make sure that no other application use it."); } - } else { - throw new InvalidOperationException( - "Can't close a package not previously open with the open() method !"); } - } + } } /** @@ -488,8 +486,9 @@ public final class ZipPackage extends OPCPackage { @Override protected void revertImpl() { try { - if (this.zipArchive != null) - this.zipArchive.close(); + if (this.zipArchive != null) { + this.zipArchive.close(); + } } catch (IOException e) { // Do nothing, user dont have to know } @@ -526,16 +525,17 @@ public final class ZipPackage extends OPCPackage { final ZipOutputStream zos; try { - if (!(outputStream instanceof ZipOutputStream)) - zos = new ZipOutputStream(outputStream); - else - zos = (ZipOutputStream) outputStream; + if (!(outputStream instanceof ZipOutputStream)) { + zos = new ZipOutputStream(outputStream); + } else { + zos = (ZipOutputStream) outputStream; + } // If the core properties part does not exist in the part list, // we save it as well if (this.getPartsByRelationshipType(PackageRelationshipTypes.CORE_PROPERTIES).size() == 0 && this.getPartsByRelationshipType(PackageRelationshipTypes.CORE_PROPERTIES_ECMA376).size() == 0 ) { - logger.log(POILogger.DEBUG,"Save core properties part"); + LOG.log(POILogger.DEBUG,"Save core properties part"); // Ensure that core properties are added if missing getPackageProperties(); @@ -555,42 +555,36 @@ public final class ZipPackage extends OPCPackage { } // Save package relationships part. - logger.log(POILogger.DEBUG,"Save package relationships"); + LOG.log(POILogger.DEBUG,"Save package relationships"); ZipPartMarshaller.marshallRelationshipPart(this.getRelationships(), PackagingURIHelper.PACKAGE_RELATIONSHIPS_ROOT_PART_NAME, zos); // Save content type part. - logger.log(POILogger.DEBUG,"Save content types part"); + LOG.log(POILogger.DEBUG,"Save content types part"); this.contentTypeManager.save(zos); // Save parts. for (PackagePart part : getParts()) { // If the part is a relationship part, we don't save it, it's // the source part that will do the job. - if (part.isRelationshipPart()) - continue; - - logger.log(POILogger.DEBUG,"Save part '" - + ZipHelper.getZipItemNameFromOPCName(part - .getPartName().getName()) + "'"); - PartMarshaller marshaller = partMarshallers - .get(part._contentType); + if (part.isRelationshipPart()) { + continue; + } + + final PackagePartName ppn = part.getPartName(); + LOG.log(POILogger.DEBUG,"Save part '" + ZipHelper.getZipItemNameFromOPCName(ppn.getName()) + "'"); + PartMarshaller marshaller = partMarshallers.get(part._contentType); + String errMsg = "The part " + ppn.getURI() + " failed to be saved in the stream with marshaller "; + if (marshaller != null) { if (!marshaller.marshall(part, zos)) { - throw new OpenXML4JException( - "The part " - + part.getPartName().getURI() - + " fail to be saved in the stream with marshaller " - + marshaller); + throw new OpenXML4JException(errMsg + marshaller); } } else { - if (!defaultPartMarshaller.marshall(part, zos)) - throw new OpenXML4JException( - "The part " - + part.getPartName().getURI() - + " fail to be saved in the stream with marshaller " - + defaultPartMarshaller); + if (!defaultPartMarshaller.marshall(part, zos)) { + throw new OpenXML4JException(errMsg + defaultPartMarshaller); + } } } zos.close(); @@ -599,8 +593,8 @@ public final class ZipPackage extends OPCPackage { throw e; } catch (Exception e) { throw new OpenXML4JRuntimeException( - "Fail to save: an error occurs while saving the package : " - + e.getMessage(), e); + "Fail to save: an error occurs while saving the package : " + + e.getMessage(), e); } } diff --git a/src/ooxml/java/org/apache/poi/openxml4j/util/ZipSecureFile.java b/src/ooxml/java/org/apache/poi/openxml4j/util/ZipSecureFile.java index 0040d64fa9..7a34c379b1 100644 --- a/src/ooxml/java/org/apache/poi/openxml4j/util/ZipSecureFile.java +++ b/src/ooxml/java/org/apache/poi/openxml4j/util/ZipSecureFile.java @@ -45,7 +45,7 @@ import org.apache.poi.util.SuppressForbidden; * and {@link #setMinInflateRatio(double)}. */ public class ZipSecureFile extends ZipFile { - private final static POILogger logger = POILogFactory.getLogger(ZipSecureFile.class); + private static final POILogger LOG = POILogFactory.getLogger(ZipSecureFile.class); private static double MIN_INFLATE_RATIO = 0.01d; private static long MAX_ENTRY_SIZE = 0xFFFFFFFFL; @@ -169,9 +169,9 @@ public class ZipSecureFile extends ZipFile { public static ThresholdInputStream addThreshold(final InputStream zipIS) throws IOException { ThresholdInputStream newInner; if (zipIS instanceof InflaterInputStream) { - newInner = AccessController.doPrivileged(new PrivilegedAction() { + newInner = AccessController.doPrivileged(new PrivilegedAction() { // NOSONAR @SuppressForbidden("TODO: Fix this to not use reflection (it will break in Java 9)! " + - "Better would be to wrap *before* instead of tyring to insert wrapper afterwards.") + "Better would be to wrap *before* instead of trying to insert wrapper afterwards.") public ThresholdInputStream run() { try { Field f = FilterInputStream.class.getDeclaredField("in"); @@ -181,7 +181,7 @@ public class ZipSecureFile extends ZipFile { f.set(zipIS, newInner); return newInner; } catch (Exception ex) { - logger.log(POILogger.WARN, "SecurityManager doesn't allow manipulation via reflection for zipbomb detection - continue with original input stream", ex); + LOG.log(POILogger.WARN, "SecurityManager doesn't allow manipulation via reflection for zipbomb detection - continue with original input stream", ex); } return null; } diff --git a/src/ooxml/testcases/org/apache/poi/TestPOIXMLDocument.java b/src/ooxml/testcases/org/apache/poi/TestPOIXMLDocument.java index eeb0e55119..432e9f0936 100644 --- a/src/ooxml/testcases/org/apache/poi/TestPOIXMLDocument.java +++ b/src/ooxml/testcases/org/apache/poi/TestPOIXMLDocument.java @@ -144,7 +144,7 @@ public final class TestPOIXMLDocument { // FIXME: A better exception class (IOException?) and message should be raised // indicating that the document could not be written because the output stream is closed. // see {@link org.apache.poi.openxml4j.opc.ZipPackage#saveImpl(java.io.OutputStream)} - if (e.getMessage().matches("Fail to save: an error occurs while saving the package : The part .+ fail to be saved in the stream with marshaller .+")) { + if (e.getMessage().matches("Fail to save: an error occurs while saving the package : The part .+ failed to be saved in the stream with marshaller .+")) { // expected } else { throw e; @@ -330,6 +330,7 @@ public final class TestPOIXMLDocument { thread.setContextClassLoader(cl.getParent()); XMLSlideShow ppt = new XMLSlideShow(is); ppt.getSlides().get(0).getShapes(); + ppt.close(); } finally { thread.setContextClassLoader(cl); is.close(); @@ -347,6 +348,7 @@ public final class TestPOIXMLDocument { POIXMLTypeLoader.setClassLoader(cl); XMLSlideShow ppt = new XMLSlideShow(is); ppt.getSlides().get(0).getShapes(); + ppt.close(); } finally { thread.setContextClassLoader(cl); POIXMLTypeLoader.setClassLoader(null); diff --git a/src/scratchpad/src/org/apache/poi/hwpf/model/CHPFormattedDiskPage.java b/src/scratchpad/src/org/apache/poi/hwpf/model/CHPFormattedDiskPage.java index 8c63dce6c3..5d7e929e21 100644 --- a/src/scratchpad/src/org/apache/poi/hwpf/model/CHPFormattedDiskPage.java +++ b/src/scratchpad/src/org/apache/poi/hwpf/model/CHPFormattedDiskPage.java @@ -24,6 +24,7 @@ import java.util.List; import org.apache.poi.hwpf.sprm.SprmBuffer; import org.apache.poi.util.Internal; import org.apache.poi.util.LittleEndian; +import org.apache.poi.util.RecordFormatException; /** * Represents a CHP fkp. The style properties for paragraph and character runs @@ -62,6 +63,7 @@ public final class CHPFormattedDiskPage extends FormattedDiskPage * {@link #CHPFormattedDiskPage(byte[], int, CharIndexTranslator)} * instead */ + @Deprecated public CHPFormattedDiskPage( byte[] documentStream, int offset, int fcMin, TextPieceTable tpt ) { @@ -122,6 +124,7 @@ public final class CHPFormattedDiskPage extends FormattedDiskPage * @param index The index of the chpx to get. * @return a chpx grpprl. */ + @Override protected byte[] getGrpprl(int index) { int chpxOffset = 2 * LittleEndian.getUByte(_fkp, _offset + (((_crun + 1) * 4) + index)); @@ -169,8 +172,7 @@ public final class CHPFormattedDiskPage extends FormattedDiskPage // the grpprl size byte and the grpprl. totalSize += ( FC_SIZE + 2 + grpprlLength ); // if size is uneven we will have to add one so the first grpprl - // falls - // on a word boundary + // falls on a word boundary if ( totalSize > 511 + ( index % 2 ) ) { totalSize -= ( FC_SIZE + 2 + grpprlLength ); @@ -184,6 +186,10 @@ public final class CHPFormattedDiskPage extends FormattedDiskPage } } + if (index == 0) { + throw new RecordFormatException("empty grpprl entry."); + } + // see if we couldn't fit some if ( index != size ) { @@ -197,15 +203,13 @@ public final class CHPFormattedDiskPage extends FormattedDiskPage offsetOffset = ( FC_SIZE * index ) + FC_SIZE; // grpprlOffset = offsetOffset + index + (grpprlOffset % 2); - CHPX chpx = null; - for ( int x = 0; x < index; x++ ) - { - chpx = _chpxList.get( x ); - byte[] grpprl = chpx.getGrpprl(); - - LittleEndian.putInt( buf, fcOffset, - translator.getByteIndex( chpx.getStart() ) ); + int chpxEnd = 0; + for ( CHPX chpx : _chpxList.subList(0, index)) { + int chpxStart = translator.getByteIndex( chpx.getStart() ); + chpxEnd = translator.getByteIndex( chpx.getEnd() ); + LittleEndian.putInt( buf, fcOffset, chpxStart ); + byte[] grpprl = chpx.getGrpprl(); grpprlOffset -= ( 1 + grpprl.length ); grpprlOffset -= ( grpprlOffset % 2 ); buf[offsetOffset] = (byte) ( grpprlOffset / 2 ); @@ -216,8 +220,7 @@ public final class CHPFormattedDiskPage extends FormattedDiskPage fcOffset += FC_SIZE; } // put the last chpx's end in - LittleEndian.putInt( buf, fcOffset, - translator.getByteIndex( chpx.getEnd() ) ); + LittleEndian.putInt( buf, fcOffset, chpxEnd ); return buf; } diff --git a/src/scratchpad/src/org/apache/poi/hwpf/model/PlexOfCps.java b/src/scratchpad/src/org/apache/poi/hwpf/model/PlexOfCps.java index 62d59aa107..034ea4f700 100644 --- a/src/scratchpad/src/org/apache/poi/hwpf/model/PlexOfCps.java +++ b/src/scratchpad/src/org/apache/poi/hwpf/model/PlexOfCps.java @@ -30,12 +30,9 @@ import org.apache.poi.util.LittleEndian; * front that relate to an array of arbitrary data structures in the back. *

* See page 184 of official documentation for details - * - * @author Ryan Ackley */ public final class PlexOfCps { private int _iMac; - private int _offset; private int _cbStruct; private List _props; @@ -104,20 +101,19 @@ public final class PlexOfCps { byte[] buf = new byte[bufSize]; - GenericPropertyNode node = null; + int nodeEnd = 0; for (int x = 0; x < size; x++) { - node = _props.get(x); - + GenericPropertyNode node = _props.get(x); + nodeEnd = node.getEnd(); + // put the starting offset of the property into the plcf. - LittleEndian.putInt(buf, (LittleEndian.INT_SIZE * x), - node.getStart()); + LittleEndian.putInt(buf, (LittleEndian.INT_SIZE * x), node.getStart()); // put the struct into the plcf - System.arraycopy(node.getBytes(), 0, buf, cpBufSize - + (x * _cbStruct), _cbStruct); + System.arraycopy(node.getBytes(), 0, buf, cpBufSize + (x * _cbStruct), _cbStruct); } // put the ending offset of the last property into the plcf. - LittleEndian.putInt(buf, LittleEndian.INT_SIZE * size, node.getEnd()); + LittleEndian.putInt(buf, LittleEndian.INT_SIZE * size, nodeEnd); return buf; } diff --git a/src/scratchpad/src/org/apache/poi/hwpf/usermodel/Picture.java b/src/scratchpad/src/org/apache/poi/hwpf/usermodel/Picture.java index 7eb3af2834..ed06d35506 100644 --- a/src/scratchpad/src/org/apache/poi/hwpf/usermodel/Picture.java +++ b/src/scratchpad/src/org/apache/poi/hwpf/usermodel/Picture.java @@ -108,13 +108,12 @@ public final class Picture * DataStream */ public Picture( int dataBlockStartOfsset, byte[] _dataStream, boolean fillBytes ) { // NOSONAR - _picfAndOfficeArtData = new PICFAndOfficeArtData( _dataStream, - dataBlockStartOfsset ); + _picfAndOfficeArtData = new PICFAndOfficeArtData( _dataStream, dataBlockStartOfsset ); _picf = _picfAndOfficeArtData.getPicf(); this.dataBlockStartOfsset = dataBlockStartOfsset; - if ( _picfAndOfficeArtData != null && _picfAndOfficeArtData.getBlipRecords() != null) { + if ( _picfAndOfficeArtData.getBlipRecords() != null) { _blipRecords = _picfAndOfficeArtData.getBlipRecords(); } else { _blipRecords = Collections.emptyList(); -- 2.39.5