You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@poi.apache.org by fa...@apache.org on 2026/10/26 19:38:51 UTC
(poi) branch trunk updated: OPC: replaced zip parts forward to their memory part; XSLF picture checksum tracks part writes (#1491)
This is an automated email from the ASF dual-hosted git repository.
pjfanning pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/poi.git
The following commit(s) were added to refs/heads/trunk by this push:
new 9e2f58b411 OPC: replaced zip parts forward to their memory part; XSLF picture checksum tracks part writes (#1491)
9e2f58b411 is described below
commit 9e2f58b411e8befcaba42786e9c736e70870e280
Author: PJ Fanning <pj...@users.noreply.github.com>
AuthorDate: Mon Oct 26 19:38:45 2026 +0000
OPC: replaced zip parts forward to their memory part; XSLF picture checksum tracks part writes (#1491)
* OPC: replaced zip parts forward to their memory part, add modification count
Writing to a ZipPackagePart replaces it in the package with a new
MemoryPackagePart, but POIXMLDocumentParts keep a reference to the zip
part. Reads through that reference returned the original zip entry, so
e.g. XSLFPictureData.setData on a loaded slideshow was not visible through
getData() and findPictureData missed the new bytes.
The replaced ZipPackagePart now forwards reads, writes, size and clear()
to the part that replaced it. PackagePart.getModificationCount is
incremented each time an output stream is obtained; XSLFPictureData uses
it to drop its cached checksum and dimensions when the part is written to
outside of setData. setData now clears the part first, as in-memory
parts append to their existing content.
Co-Authored-By: Claude Opus 5.5 <no...@anthropic.com>
* XSLFPictureData.hasData: drop a stale cached checksum too
hasData (from #1490) reads the cached checksum directly, so it needs the
same modification-count check as cacheProperties.
Co-Authored-By: Claude Opus 5.5 <no...@anthropic.com>
---------
Co-authored-by: Claude Opus 5.5 <no...@anthropic.com>
---
.../org/apache/poi/openxml4j/opc/PackagePart.java | 29 +++++++++++++-
.../apache/poi/openxml4j/opc/ZipPackagePart.java | 33 +++++++++++++++-
.../apache/poi/xslf/usermodel/XSLFPictureData.java | 19 +++++++++
.../org/apache/poi/openxml4j/opc/TestPackage.java | 45 ++++++++++++++++++++++
.../poi/xslf/usermodel/TestXSLFSlideShow.java | 33 ++++++++++++++++
5 files changed, 157 insertions(+), 2 deletions(-)
diff --git a/poi-ooxml/src/main/java/org/apache/poi/openxml4j/opc/PackagePart.java b/poi-ooxml/src/main/java/org/apache/poi/openxml4j/opc/PackagePart.java
index 70f5654ddd..0729a206a7 100644
--- a/poi-ooxml/src/main/java/org/apache/poi/openxml4j/opc/PackagePart.java
+++ b/poi-ooxml/src/main/java/org/apache/poi/openxml4j/opc/PackagePart.java
@@ -63,6 +63,11 @@ public abstract class PackagePart implements RelationshipSource, Comparable<Pack
*/
private PackageRelationshipCollection _relationships;
+ /**
+ * Number of times an output stream was obtained for this part.
+ */
+ private long _modificationCount;
+
/**
* Constructor.
@@ -521,7 +526,13 @@ public abstract class PackagePart implements RelationshipSource, Comparable<Pack
OutputStream outStream;
// If this part is a zip package part (read only by design) we convert
// this part into a MemoryPackagePart instance for write purpose.
- if (this instanceof ZipPackagePart) {
+ if (this instanceof ZipPackagePart zipPart) {
+ PackagePart replacement = zipPart.getReplacement();
+ if (replacement != null) {
+ // this part was already replaced, write to the replacement
+ return replacement.getOutputStream();
+ }
+
// Delete logically this part
_container.removePart(this._partName);
@@ -533,8 +544,12 @@ public abstract class PackagePart implements RelationshipSource, Comparable<Pack
"Can't create a temporary part !");
}
part._relationships = this._relationships;
+ part._modificationCount = this._modificationCount + 1;
+ // callers may still hold this part, so forward to the memory part from now on
+ zipPart.setReplacement(part);
outStream = part.getOutputStreamImpl();
} else {
+ _modificationCount++;
outStream = this.getOutputStreamImpl();
}
return outStream;
@@ -648,6 +663,18 @@ public abstract class PackagePart implements RelationshipSource, Comparable<Pack
return -1;
}
+ /**
+ * Returns a counter that changes whenever the content of this part may have changed.
+ * It is incremented each time {@link #getOutputStream()} is called, so callers that
+ * cache information derived from the content can detect that it is out of date.
+ *
+ * @return the modification count of this part
+ * @since 6.0.1
+ */
+ public long getModificationCount() {
+ return _modificationCount;
+ }
+
@Override
public String toString() {
return "Name: " + this._partName + " - Content Type: "
diff --git a/poi-ooxml/src/main/java/org/apache/poi/openxml4j/opc/ZipPackagePart.java b/poi-ooxml/src/main/java/org/apache/poi/openxml4j/opc/ZipPackagePart.java
index 8774043a14..a0ebbbbc8c 100644
--- a/poi-ooxml/src/main/java/org/apache/poi/openxml4j/opc/ZipPackagePart.java
+++ b/poi-ooxml/src/main/java/org/apache/poi/openxml4j/opc/ZipPackagePart.java
@@ -42,6 +42,11 @@ public class ZipPackagePart extends PackagePart {
*/
private ZipArchiveEntry zipEntry;
+ /**
+ * The part that took over the content of this part, once it was written to.
+ */
+ private PackagePart replacement;
+
/**
* Constructor.
*
@@ -100,6 +105,9 @@ public class ZipPackagePart extends PackagePart {
*/
@Override
protected InputStream getInputStreamImpl() throws IOException {
+ if (replacement != null) {
+ return replacement.getInputStream();
+ }
// We use the getInputStream() method from java.util.zip.ZipFile
// class which return an InputStream to this part zip entry.
return ((ZipPackage) _container).getZipArchive()
@@ -119,7 +127,30 @@ public class ZipPackagePart extends PackagePart {
@Override
public long getSize() {
- return zipEntry.getSize();
+ return replacement != null ? replacement.getSize() : zipEntry.getSize();
+ }
+
+ @Override
+ public void clear() {
+ if (replacement != null) {
+ replacement.clear();
+ }
+ }
+
+ @Override
+ public long getModificationCount() {
+ return replacement != null ? replacement.getModificationCount() : super.getModificationCount();
+ }
+
+ /**
+ * @return the part that replaced this part when it was written to, or {@code null}
+ */
+ /* package */ PackagePart getReplacement() {
+ return replacement;
+ }
+
+ /* package */ void setReplacement(PackagePart replacement) {
+ this.replacement = replacement;
}
@Override
diff --git a/poi-ooxml/src/main/java/org/apache/poi/xslf/usermodel/XSLFPictureData.java b/poi-ooxml/src/main/java/org/apache/poi/xslf/usermodel/XSLFPictureData.java
index 3b1a4a07d7..2cf6d9da0f 100644
--- a/poi-ooxml/src/main/java/org/apache/poi/xslf/usermodel/XSLFPictureData.java
+++ b/poi-ooxml/src/main/java/org/apache/poi/xslf/usermodel/XSLFPictureData.java
@@ -64,6 +64,8 @@ public final class XSLFPictureData extends POIXMLDocumentPart implements Picture
}
private Long checksum;
+ // modification count of the package part when checksum and origSize were cached
+ private long cachedModificationCount;
// original image dimensions (for formats supported by BufferedImage)
private Dimension origSize;
@@ -164,6 +166,7 @@ public final class XSLFPictureData extends POIXMLDocumentPart implements Picture
* @return true, if this picture holds the same bytes as {@code data}
*/
boolean hasData(byte[] data, long dataChecksum) {
+ dropStaleProperties();
byte[] ownData = null;
if (checksum == null) {
ownData = getData();
@@ -178,10 +181,23 @@ public final class XSLFPictureData extends POIXMLDocumentPart implements Picture
return Arrays.equals(ownData, data);
}
+ /**
+ * Forget the cached image properties, if the part was written to outside of setData
+ */
+ private void dropStaleProperties() {
+ long modificationCount = getPackagePart().getModificationCount();
+ if (modificationCount != cachedModificationCount) {
+ checksum = null;
+ origSize = null;
+ cachedModificationCount = modificationCount;
+ }
+ }
+
/**
* Determine and cache image properties
*/
protected void cacheProperties() {
+ dropStaleProperties();
if (origSize == null || checksum == null) {
byte[] data = getData();
checksum = IOUtils.calculateChecksum(data);
@@ -226,11 +242,14 @@ public final class XSLFPictureData extends POIXMLDocumentPart implements Picture
}
public void setData(byte[] data) throws IOException {
+ // in-memory parts append to their content, so remove the old data first
+ getPackagePart().clear();
try (OutputStream os = getPackagePart().getOutputStream()) {
os.write(data);
}
// recalculate now since we already have the data bytes available anyhow
checksum = IOUtils.calculateChecksum(data);
+ cachedModificationCount = getPackagePart().getModificationCount();
origSize = null; // need to recalculate image size
}
diff --git a/poi-ooxml/src/test/java/org/apache/poi/openxml4j/opc/TestPackage.java b/poi-ooxml/src/test/java/org/apache/poi/openxml4j/opc/TestPackage.java
index eaf3befdaa..dd6f8ab25f 100644
--- a/poi-ooxml/src/test/java/org/apache/poi/openxml4j/opc/TestPackage.java
+++ b/poi-ooxml/src/test/java/org/apache/poi/openxml4j/opc/TestPackage.java
@@ -24,11 +24,14 @@ import static org.apache.poi.openxml4j.OpenXML4JTestDataSamples.getSampleFileNam
import static org.apache.poi.openxml4j.OpenXML4JTestDataSamples.openSampleStream;
import static org.apache.poi.openxml4j.opc.PackagingURIHelper.createPartName;
import static org.apache.poi.xssf.usermodel.XSSFRelation.NS_WORDPROCESSINGML;
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNotSame;
import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertSame;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
@@ -658,6 +661,48 @@ public final class TestPackage {
}
}
+ @Test
+ void writeToZipPartThroughOldReference() throws IOException, InvalidFormatException {
+ byte[] data1 = "first".getBytes(StandardCharsets.US_ASCII);
+ byte[] data2 = "second".getBytes(StandardCharsets.US_ASCII);
+ try (InputStream is = openSampleStream("sample.docx");
+ OPCPackage pkg = OPCPackage.open(is)) {
+ PackagePartName name = createPartName("/word/fontTable.xml");
+ PackagePart zipPart = pkg.getPart(name);
+ assertEquals(ZipPackagePart.class, zipPart.getClass());
+ assertEquals(0, zipPart.getModificationCount());
+ int numParts = pkg.getParts().size();
+
+ try (OutputStream os = zipPart.getOutputStream()) {
+ os.write(data1);
+ }
+ PackagePart memoryPart = pkg.getPart(name);
+ assertNotSame(zipPart, memoryPart);
+ assertEquals(1, zipPart.getModificationCount());
+ assertEquals(1, memoryPart.getModificationCount());
+ // the old reference reads the new content
+ try (InputStream partIs = zipPart.getInputStream()) {
+ assertArrayEquals(data1, IOUtils.toByteArray(partIs));
+ }
+ assertEquals(data1.length, zipPart.getSize());
+
+ // writing through the old reference again updates the memory part
+ zipPart.clear();
+ try (OutputStream os = zipPart.getOutputStream()) {
+ os.write(data2);
+ }
+ assertSame(memoryPart, pkg.getPart(name));
+ assertEquals(2, zipPart.getModificationCount());
+ try (InputStream partIs = memoryPart.getInputStream()) {
+ assertArrayEquals(data2, IOUtils.toByteArray(partIs));
+ }
+ try (InputStream partIs = zipPart.getInputStream()) {
+ assertArrayEquals(data2, IOUtils.toByteArray(partIs));
+ }
+ assertEquals(numParts, pkg.getParts().size());
+ }
+ }
+
@Test
void replaceContentType() throws IOException, InvalidFormatException {
try (InputStream is = openSampleStream("sample.xlsx");
diff --git a/poi-ooxml/src/test/java/org/apache/poi/xslf/usermodel/TestXSLFSlideShow.java b/poi-ooxml/src/test/java/org/apache/poi/xslf/usermodel/TestXSLFSlideShow.java
index 07054933db..a0aee124ff 100644
--- a/poi-ooxml/src/test/java/org/apache/poi/xslf/usermodel/TestXSLFSlideShow.java
+++ b/poi-ooxml/src/test/java/org/apache/poi/xslf/usermodel/TestXSLFSlideShow.java
@@ -20,6 +20,7 @@ import java.awt.Dimension;
import java.awt.Rectangle;
import java.io.ByteArrayInputStream;
import java.io.IOException;
+import java.io.OutputStream;
import java.nio.charset.StandardCharsets;
import java.util.List;
import java.util.regex.Pattern;
@@ -458,4 +459,36 @@ class TestXSLFSlideShow {
}
}
}
+
+ @Test
+ void changePictureDataOfLoadedSlideShow() throws IOException {
+ byte[] data1 = "first".getBytes(StandardCharsets.US_ASCII);
+ byte[] data2 = "second".getBytes(StandardCharsets.US_ASCII);
+ byte[] data3 = "third".getBytes(StandardCharsets.US_ASCII);
+ try (XMLSlideShow ppt = new XMLSlideShow()) {
+ XSLFPictureData newPic = ppt.addPicture(data3, PictureData.PictureType.PNG);
+ newPic.setData(data1);
+ assertArrayEquals(data1, newPic.getData());
+ assertSame(newPic, ppt.findPictureData(data1));
+ try (XMLSlideShow ppt2 = XSLFTestDataSamples.writeOutAndReadBack(ppt)) {
+ XSLFPictureData pic = ppt2.findPictureData(data1);
+ assertNotNull(pic);
+
+ pic.setData(data2);
+ assertArrayEquals(data2, pic.getData());
+ assertNull(ppt2.findPictureData(data1));
+ assertSame(pic, ppt2.findPictureData(data2));
+
+ // writing to the part directly must not leave a stale checksum behind
+ pic.getPackagePart().clear();
+ try (OutputStream os = pic.getPackagePart().getOutputStream()) {
+ os.write(data3);
+ }
+ assertArrayEquals(data3, pic.getData());
+ assertNull(ppt2.findPictureData(data2));
+ assertSame(pic, ppt2.findPictureData(data3));
+ assertEquals(1, ppt2.getPictureData().size());
+ }
+ }
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@poi.apache.org
For additional commands, e-mail: commits-help@poi.apache.org