From: Andreas Beeker Date: Sun, 17 Sep 2017 22:45:03 +0000 (+0000) Subject: #60499 - Deleting a picture that is used twice on a slide corrupt the slide X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=a51bac65b77f9524c001865398c7707f971ae07e;p=poi.git #60499 - Deleting a picture that is used twice on a slide corrupt the slide git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1808661 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/src/ooxml/java/org/apache/poi/POIXMLDocumentPart.java b/src/ooxml/java/org/apache/poi/POIXMLDocumentPart.java index c5afb15446..5b6add51ab 100644 --- a/src/ooxml/java/org/apache/poi/POIXMLDocumentPart.java +++ b/src/ooxml/java/org/apache/poi/POIXMLDocumentPart.java @@ -231,15 +231,35 @@ public class POIXMLDocumentPart { * @return the target part of the relation, or null, if none exists */ public final POIXMLDocumentPart getRelationById(String id) { - RelationPart rp = relations.get(id); + RelationPart rp = getRelationPartById(id); return (rp == null) ? null : rp.getDocumentPart(); } /** - * Returns the {@link PackageRelationship#getId()} of the + * Returns the target {@link RelationPart}, where a + * {@link PackageRelationship} is set from the {@link PackagePart} of this + * {@link POIXMLDocumentPart} to the {@link PackagePart} of the target + * {@link POIXMLDocumentPart} with a {@link PackageRelationship#getId()} + * matching the given parameter value. + * + * @param id + * The relation id to look for + * @return the target relation part, or null, if none exists + * + * @since 4.0.0 + */ + public final RelationPart getRelationPartById(String id) { + return relations.get(id); + } + + /** + * Returns the first {@link PackageRelationship#getId()} of the * {@link PackageRelationship}, that sources from the {@link PackagePart} of * this {@link POIXMLDocumentPart} to the {@link PackagePart} of the given - * parameter value. + * parameter value.

+ * + * There can be multiple references to the given {@link POIXMLDocumentPart} + * and only the first in the order of creation is returned. * * @param part * The {@link POIXMLDocumentPart} for which the according @@ -292,7 +312,11 @@ public class POIXMLDocumentPart { /** * Remove the relation to the specified part in this package and remove the - * part, if it is no longer needed. + * part, if it is no longer needed.

+ * + * If there are multiple relationships to the same part, this will only + * remove the first relationship in the order of creation. The removal + * via the part id ({@link #removeRelation(String)} is preferred. * * @param part the part which relation is to be removed from this document */ @@ -302,7 +326,11 @@ public class POIXMLDocumentPart { /** * Remove the relation to the specified part in this package and remove the - * part, if it is no longer needed and flag is set to true. + * part, if it is no longer needed and flag is set to true.

+ * + * If there are multiple relationships to the same part, this will only + * remove the first relationship in the order of creation. The removal + * via the part id ({@link #removeRelation(String,boolean)} is preferred. * * @param part * The related part, to which the relation shall be removed. @@ -311,18 +339,53 @@ public class POIXMLDocumentPart { * needed any longer. * @return true, if the relation was removed */ - protected final boolean removeRelation(POIXMLDocumentPart part, boolean removeUnusedParts){ + protected final boolean removeRelation(POIXMLDocumentPart part, boolean removeUnusedParts) { String id = getRelationId(part); - if (id == null) { + return removeRelation(id, removeUnusedParts); + } + + /** + * Remove the relation to the specified part in this package and remove the + * part, if it is no longer needed.

+ * + * If there are multiple relationships to the same part, this will only + * remove the first relationship in the order of creation. The removal + * via the part id ({@link #removeRelation(String)} is preferred. + * + * @param partId the part id which relation is to be removed from this document + * + * @since 4.0.0 + */ + protected final void removeRelation(String partId) { + removeRelation(partId, true); + } + + /** + * Remove the relation to the specified part in this package and remove the + * part, if it is no longer needed and flag is set to true.

+ * + * @param partId + * The related part id, to which the relation shall be removed. + * @param removeUnusedParts + * true, if the part shall be removed from the package if not + * needed any longer. + * @return true, if the relation was removed + * + * @since 4.0.0 + */ + private final boolean removeRelation(String partId, boolean removeUnusedParts) { + RelationPart rp = relations.get(partId); + if (rp == null) { // part is not related with this POIXMLDocumentPart return false; } + POIXMLDocumentPart part = rp.getDocumentPart(); /* decrement usage counter */ part.decrementRelationCounter(); /* remove packagepart relationship */ - getPackagePart().removeRelationship(id); + getPackagePart().removeRelationship(partId); /* remove POIXMLDocument from relations */ - relations.remove(id); + relations.remove(partId); if (removeUnusedParts) { /* if last relation to target part was removed, delete according target part */ @@ -338,6 +401,8 @@ public class POIXMLDocumentPart { return true; } + + /** * Returns the parent POIXMLDocumentPart. All parts except root have not-null parent. * diff --git a/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFSheet.java b/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFSheet.java index aed2ff7c49..148f5cd6a7 100644 --- a/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFSheet.java +++ b/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFSheet.java @@ -612,7 +612,6 @@ implements XSLFShapeContainer, Sheet { * @param pictureShape the picture shapes whose relation is to be removed */ void removePictureRelation(XSLFPictureShape pictureShape) { - POIXMLDocumentPart pd = getRelationById(pictureShape.getBlipId()); - removeRelation(pd); + removeRelation(pictureShape.getBlipId()); } } \ No newline at end of file diff --git a/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFSlide.java b/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFSlide.java index e443fa3106..cfa5873cc5 100644 --- a/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFSlide.java +++ b/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFSlide.java @@ -243,7 +243,7 @@ implements Slide { if (bgThis != null) { if (bgThis.isSetBgPr() && bgThis.getBgPr().isSetBlipFill()) { String oldId = bgThis.getBgPr().getBlipFill().getBlip().getEmbed(); - removeRelation(getRelationById(oldId)); + removeRelation(oldId); } _slide.getCSld().unsetBg(); } diff --git a/src/ooxml/testcases/org/apache/poi/TestPOIXMLDocument.java b/src/ooxml/testcases/org/apache/poi/TestPOIXMLDocument.java index 90e13aecd4..21c003c297 100644 --- a/src/ooxml/testcases/org/apache/poi/TestPOIXMLDocument.java +++ b/src/ooxml/testcases/org/apache/poi/TestPOIXMLDocument.java @@ -280,7 +280,7 @@ public final class TestPOIXMLDocument { assertNull(part.getRelationById(null)); assertNull(part.getRelationId(null)); assertFalse(part.removeRelation(null, true)); - part.removeRelation(null); + part.removeRelation((POIXMLDocumentPart)null); assertEquals("",part.toString()); part.onDocumentCreate(); //part.getTargetPart(null); diff --git a/src/ooxml/testcases/org/apache/poi/xslf/TestXSLFBugs.java b/src/ooxml/testcases/org/apache/poi/xslf/TestXSLFBugs.java index 6f8fd2121c..5a5c7bfa77 100644 --- a/src/ooxml/testcases/org/apache/poi/xslf/TestXSLFBugs.java +++ b/src/ooxml/testcases/org/apache/poi/xslf/TestXSLFBugs.java @@ -20,6 +20,7 @@ import static org.apache.poi.POITestCase.assertContains; import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -30,17 +31,24 @@ import java.awt.RenderingHints; import java.awt.geom.AffineTransform; import java.awt.geom.Rectangle2D; import java.awt.image.BufferedImage; +import java.io.ByteArrayInputStream; import java.io.File; import java.io.IOException; +import java.io.InputStream; import java.net.URI; import java.util.Collection; +import java.util.Optional; +import java.util.stream.Collectors; import javax.imageio.ImageIO; import org.apache.poi.POIDataSamples; import org.apache.poi.POIXMLDocumentPart; import org.apache.poi.POIXMLDocumentPart.RelationPart; +import org.apache.poi.openxml4j.exceptions.InvalidFormatException; import org.apache.poi.openxml4j.opc.OPCPackage; +import org.apache.poi.openxml4j.opc.PackagePartName; +import org.apache.poi.openxml4j.opc.PackagingURIHelper; import org.apache.poi.sl.draw.DrawPaint; import org.apache.poi.sl.usermodel.PaintStyle; import org.apache.poi.sl.usermodel.PaintStyle.SolidPaint; @@ -49,6 +57,7 @@ import org.apache.poi.sl.usermodel.PictureData; import org.apache.poi.sl.usermodel.PictureData.PictureType; import org.apache.poi.sl.usermodel.ShapeType; import org.apache.poi.sl.usermodel.VerticalAlignment; +import org.apache.poi.util.IOUtils; import org.apache.poi.xslf.extractor.XSLFPowerPointExtractor; import org.apache.poi.xslf.usermodel.XMLSlideShow; import org.apache.poi.xslf.usermodel.XSLFAutoShape; @@ -76,6 +85,50 @@ import org.openxmlformats.schemas.presentationml.x2006.main.CTShape; public class TestXSLFBugs { private static final POIDataSamples slTests = POIDataSamples.getSlideShowInstance(); + @Test + public void bug60499() throws IOException, InvalidFormatException { + InputStream is = slTests.openResourceAsStream("bug60499.pptx"); + byte buf[] = IOUtils.toByteArray(is); + is.close(); + + PackagePartName ppn = PackagingURIHelper.createPartName("/ppt/media/image1.png"); + + XMLSlideShow ppt1 = new XMLSlideShow(new ByteArrayInputStream(buf)); + XSLFSlide slide1 = ppt1.getSlides().get(0); + + Optional shapeToDelete1 = + slide1.getShapes().stream().filter(s -> s instanceof XSLFPictureShape).findFirst(); + + assertTrue(shapeToDelete1.isPresent()); + slide1.removeShape(shapeToDelete1.get()); + slide1.getRelationParts().stream() + .allMatch(rp -> "rId1,rId3".contains(rp.getRelationship().getId()) ); + + assertNotNull(ppt1.getPackage().getPart(ppn)); + ppt1.close(); + + XMLSlideShow ppt2 = new XMLSlideShow(new ByteArrayInputStream(buf)); + XSLFSlide slide2 = ppt2.getSlides().get(0); + + Optional shapeToDelete2 = + slide2.getShapes().stream().filter(s -> s instanceof XSLFPictureShape).skip(1).findFirst(); + assertTrue(shapeToDelete2.isPresent()); + slide2.removeShape(shapeToDelete2.get()); + slide2.getRelationParts().stream() + .allMatch(rp -> "rId1,rId2".contains(rp.getRelationship().getId()) ); + assertNotNull(ppt2.getPackage().getPart(ppn)); + ppt2.close(); + + XMLSlideShow ppt3 = new XMLSlideShow(new ByteArrayInputStream(buf)); + XSLFSlide slide3 = ppt3.getSlides().get(0); + slide3.getShapes().stream() + .filter(s -> s instanceof XSLFPictureShape) + .collect(Collectors.toList()) + .forEach(s -> slide3.removeShape(s)); + assertNull(ppt3.getPackage().getPart(ppn)); + ppt3.close(); + } + @Test public void bug51187() throws Exception { XMLSlideShow ss1 = XSLFTestDataSamples.openSampleDocument("51187.pptx"); diff --git a/test-data/slideshow/bug60499.pptx b/test-data/slideshow/bug60499.pptx new file mode 100644 index 0000000000..300873dde0 Binary files /dev/null and b/test-data/slideshow/bug60499.pptx differ