diff options
12 files changed, 352 insertions, 62 deletions
diff --git a/build.gradle b/build.gradle index 738abd8c5d..7f64600b0c 100644 --- a/build.gradle +++ b/build.gradle @@ -34,7 +34,7 @@ plugins { id 'com.dorongold.task-tree' version '4.0.1' id 'org.nosphere.apache.rat' version '0.8.1' id 'distribution' - id 'com.github.spotbugs' version '6.1.12' + id 'com.github.spotbugs' version '6.1.13' id 'de.thetaphi.forbiddenapis' version '3.9' id 'org.sonarqube' version '4.0.0.2929' id 'org.cyclonedx.bom' version '2.3.1' @@ -67,7 +67,7 @@ configurations { } dependencies { - antLibs("org.junit.jupiter:junit-jupiter:5.12.2") + antLibs("org.junit.jupiter:junit-jupiter:5.13.0") antLibs("org.apache.ant:ant-junitlauncher:1.10.15") } @@ -77,7 +77,7 @@ ant.taskdef(name: "junit", wrapper { - gradleVersion = '8.14' + gradleVersion = '8.14.1' } group = 'org.apache.poi' @@ -107,12 +107,12 @@ subprojects { apply plugin: 'com.adarshr.test-logger' ext { - bouncyCastleVersion = '1.80' + bouncyCastleVersion = '1.81' commonsCodecVersion = '1.18.0' commonsCompressVersion = '1.27.1' commonsIoVersion = '2.19.0' commonsMathVersion = '3.6.1' - junitVersion = '5.12.2' + junitVersion = '5.13.0' log4jVersion = '2.24.3' mockitoVersion = '4.11.0' hamcrestVersion = '3.0' @@ -121,7 +121,7 @@ subprojects { graphics2dVersion = '3.0.3' pdfboxVersion = '3.0.5' saxonVersion = '12.7' - xmlSecVersion = '3.0.5' + xmlSecVersion = '3.0.6' apiGuardianVersion = '1.1.2' jdkVersion = (project.properties['jdkVersion'] ?: '8') as int @@ -143,7 +143,7 @@ subprojects { resolutionStrategy { force "commons-io:commons-io:${commonsIoVersion}" force 'org.slf4j:slf4j-api:2.0.17' - force 'com.fasterxml.woodstox:woodstox-core:7.1.0' + force 'com.fasterxml.woodstox:woodstox-core:7.1.1' } } } @@ -172,7 +172,7 @@ subprojects { dependencies { testImplementation "org.junit.jupiter:junit-jupiter:${junitVersion}" - testRuntimeOnly 'org.junit.platform:junit-platform-launcher:1.12.2' + testRuntimeOnly 'org.junit.platform:junit-platform-launcher:1.13.0' testImplementation "org.mockito:mockito-core:${mockitoVersion}" testImplementation "org.hamcrest:hamcrest:${hamcrestVersion}" testImplementation "org.apache.logging.log4j:log4j-core:${log4jVersion}" @@ -422,7 +422,7 @@ subprojects { jvmArgs += [ // see https://github.com/java9-modularity/gradle-modules-plugin/issues/97 // opposed to the recommendation there, it doesn't work to add ... to the dependencies - // testRuntimeOnly 'org.junit.platform:junit-platform-launcher:1.12.2' + // testRuntimeOnly 'org.junit.platform:junit-platform-launcher:1.13.0' // gradles gradle-worker.jar is still not a JPMS module and thus runs as unnamed module '--add-exports','org.junit.platform.commons/org.junit.platform.commons.util=org.apache.poi.poi', '--add-exports','org.junit.platform.commons/org.junit.platform.commons.util=ALL-UNNAMED', @@ -270,14 +270,14 @@ under the License. <dependency prefix="main.com.zaxxer" artifact="com.zaxxer:SparseBitSet:1.3" usage="main"/> <dependency prefix="main.log4j-api" artifact="org.apache.logging.log4j:log4j-api:2.24.3" usage="main"/> - <dependency prefix="main.junit-api" artifact="org.junit.jupiter:junit-jupiter-api:5.12.2" usage="main-tests"/> - <dependency prefix="main.junit-jengine" artifact="org.junit.jupiter:junit-jupiter-engine:5.12.2" usage="main-tests"/> - <dependency prefix="main.junit-params" artifact="org.junit.jupiter:junit-jupiter-params:5.12.2" usage="main-tests"/> + <dependency prefix="main.junit-api" artifact="org.junit.jupiter:junit-jupiter-api:5.13.0" usage="main-tests"/> + <dependency prefix="main.junit-jengine" artifact="org.junit.jupiter:junit-jupiter-engine:5.13.0" usage="main-tests"/> + <dependency prefix="main.junit-params" artifact="org.junit.jupiter:junit-jupiter-params:5.13.0" usage="main-tests"/> <dependency prefix="main.junit-opentest4j" artifact="org.opentest4j:opentest4j:1.2.0" usage="main-tests"/> <dependency prefix="main.junit-apiguardian" artifact="org.apiguardian:apiguardian-api:1.1.2" usage="main-tests"/> - <dependency prefix="main.junit-pcommons" artifact="org.junit.platform:junit-platform-commons:1.12.2" usage="main-tests"/> - <dependency prefix="main.junit-pengine" artifact="org.junit.platform:junit-platform-engine:1.12.2" usage="main-tests"/> - <dependency prefix="main.junit-plauncher" artifact="org.junit.platform:junit-platform-launcher:1.12.2" usage="main-tests"/> + <dependency prefix="main.junit-pcommons" artifact="org.junit.platform:junit-platform-commons:1.13.0" usage="main-tests"/> + <dependency prefix="main.junit-pengine" artifact="org.junit.platform:junit-platform-engine:1.13.0" usage="main-tests"/> + <dependency prefix="main.junit-plauncher" artifact="org.junit.platform:junit-platform-launcher:1.13.0" usage="main-tests"/> <dependency prefix="main.jmh" artifact="org.openjdk.jmh:jmh-core:1.35" usage="main-tests"/> @@ -295,12 +295,12 @@ under the License. <dependency prefix="main.antlauncher" artifact="org.apache.ant:ant-launcher:1.10.15" usage="excelant"/> <!-- xml signature libs - not part of the distribution --> - <dependency prefix="dsig.xmlsec" artifact="org.apache.santuario:xmlsec:3.0.5" usage="ooxml-provided"/> - <dependency prefix="dsig.bouncycastle-prov" artifact="org.bouncycastle:bcprov-jdk18on:1.80" usage="ooxml-provided"/> - <dependency prefix="dsig.bouncycastle-pkix" artifact="org.bouncycastle:bcpkix-jdk18on:1.80" usage="ooxml-provided"/> - <dependency prefix="dsig.bouncycastle-util" artifact="org.bouncycastle:bcutil-jdk18on:1.80" usage="ooxml-provided"/> + <dependency prefix="dsig.xmlsec" artifact="org.apache.santuario:xmlsec:3.0.6" usage="ooxml-provided"/> + <dependency prefix="dsig.bouncycastle-prov" artifact="org.bouncycastle:bcprov-jdk18on:1.81" usage="ooxml-provided"/> + <dependency prefix="dsig.bouncycastle-pkix" artifact="org.bouncycastle:bcpkix-jdk18on:1.81" usage="ooxml-provided"/> + <dependency prefix="dsig.bouncycastle-util" artifact="org.bouncycastle:bcutil-jdk18on:1.81" usage="ooxml-provided"/> <!-- only used for signing the release - not used with the ooxml signatures --> - <dependency prefix="dsig.bouncycastle-bcpg" artifact="org.bouncycastle:bcpg-jdk18on:1.80" usage="util"/> + <dependency prefix="dsig.bouncycastle-bcpg" artifact="org.bouncycastle:bcpg-jdk18on:1.81" usage="util"/> <dependency prefix="ooxml.test.stax2" artifact="org.codehaus.woodstox:stax2-api:4.2.1" usage="ooxml-provided"/> <!-- svg/batik/pdf libs - not part of the distribution - move batik to its own directory because of JPMS module-path issues --> diff --git a/gradle/wrapper/gradle-wrapper.properties b/gradle/wrapper/gradle-wrapper.properties index ca025c83a7..002b867c48 100644 --- a/gradle/wrapper/gradle-wrapper.properties +++ b/gradle/wrapper/gradle-wrapper.properties @@ -1,6 +1,6 @@ distributionBase=GRADLE_USER_HOME distributionPath=wrapper/dists -distributionUrl=https\://services.gradle.org/distributions/gradle-8.14-bin.zip +distributionUrl=https\://services.gradle.org/distributions/gradle-8.14.1-bin.zip networkTimeout=10000 validateDistributionUrl=true zipStoreBase=GRADLE_USER_HOME diff --git a/osgi/README.md b/osgi/README.md index 00a93ada8a..d6042126b6 100644 --- a/osgi/README.md +++ b/osgi/README.md @@ -25,11 +25,11 @@ Available in Maven Central: https://mvnrepository.com/artifact/net.sf.saxon/Saxo 3. Apache XML Security for Java, Bouncy Castle and XML Commons Resolver These are required to sign or validate signed Office documents. The OSGi bundles are available in Maven Central: - - Apache XML Security for Java: https://mvnrepository.com/artifact/org.apache.santuario/xmlsec/3.0.5 + - Apache XML Security for Java: https://mvnrepository.com/artifact/org.apache.santuario/xmlsec/3.0.6 - XML Commons Resolver: https://mvnrepository.com/artifact/xml-resolver/xml-resolver/1.2-osgi - - Bouncy Castle: https://mvnrepository.com/artifact/org.bouncycastle/bcprov-ext-jdk18on/1.77, https://mvnrepository.com/artifact/org.bouncycastle/bcpkix-jdk18on/1.77 + - Bouncy Castle: https://mvnrepository.com/artifact/org.bouncycastle/bcprov-ext-jdk18on/1.81, https://mvnrepository.com/artifact/org.bouncycastle/bcpkix-jdk18on/1.81 4. PDFBox and PDFBox Graphics2D Required to render to PDF documents. The required jars can be downloaded from: diff --git a/poi-ooxml/src/main/java/org/apache/poi/poifs/crypt/dsig/SignatureInfo.java b/poi-ooxml/src/main/java/org/apache/poi/poifs/crypt/dsig/SignatureInfo.java index 4b487fcc9e..2e6f7aa99f 100644 --- a/poi-ooxml/src/main/java/org/apache/poi/poifs/crypt/dsig/SignatureInfo.java +++ b/poi-ooxml/src/main/java/org/apache/poi/poifs/crypt/dsig/SignatureInfo.java @@ -154,7 +154,7 @@ import org.w3c.dom.events.MutationEvent; * <p>To use SignatureInfo and its sibling classes, you'll need to have the following libs * in the classpath:</p> * <ul> - * <li>BouncyCastle bcpkix and bcprov (tested against 1.80)</li> + * <li>BouncyCastle bcpkix and bcprov (tested against 1.81)</li> * <li>Apache Santuario "xmlsec" (tested against 3.0.x)</li> * <li>and log4j-api (tested against 2.22.x)</li> * </ul> diff --git a/poi-ooxml/src/main/java/org/apache/poi/xssf/usermodel/XSSFSheet.java b/poi-ooxml/src/main/java/org/apache/poi/xssf/usermodel/XSSFSheet.java index b8166a880e..c851eda3ec 100644 --- a/poi-ooxml/src/main/java/org/apache/poi/xssf/usermodel/XSSFSheet.java +++ b/poi-ooxml/src/main/java/org/apache/poi/xssf/usermodel/XSSFSheet.java @@ -3926,7 +3926,15 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet, OoxmlSheetEx if(minCell != Integer.MAX_VALUE) { cellRangeAddress = new CellRangeAddress(getFirstRowNum(), getLastRowNum(), minCell, maxCell); } + } else { + // sort columns + for(Map.Entry<Integer, XSSFRow> entry : _rows.entrySet()) { + XSSFRow row = entry.getValue(); + // sorting happens in XSSFRow.fixupCTCells + row.onDocumentWrite(); + } } + if (cellRangeAddress != null) { if (worksheet.isSetDimension()) { worksheet.getDimension().setRef(cellRangeAddress.formatAsString()); diff --git a/poi-ooxml/src/test/java/org/apache/poi/xssf/streaming/TestOutOfOrderColumns.java b/poi-ooxml/src/test/java/org/apache/poi/xssf/streaming/TestOutOfOrderColumns.java new file mode 100644 index 0000000000..b7b6e67030 --- /dev/null +++ b/poi-ooxml/src/test/java/org/apache/poi/xssf/streaming/TestOutOfOrderColumns.java @@ -0,0 +1,122 @@ +/*
+ * ====================================================================
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ * ====================================================================
+ */
+
+package org.apache.poi.xssf.streaming;
+
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.io.IOException;
+import java.io.InputStream;
+
+import org.apache.commons.io.output.UnsynchronizedByteArrayOutputStream;
+import org.apache.poi.ss.usermodel.Row;
+import org.apache.poi.ss.usermodel.Sheet;
+import org.apache.poi.xssf.usermodel.XSSFSheet;
+import org.apache.poi.xssf.usermodel.XSSFWorkbook;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Test creates cells in reverse column order in XSSF and SXSSF and expects
+ * saved files to have fixed the order.
+ *
+ * This is necessary because if columns in the saved file are out of order
+ * Excel will show a repair dialog when opening the file and removing data.
+ */
+public final class TestOutOfOrderColumns {
+
+ @Test
+ void outOfOrderColumnsXSSF() throws IOException {
+ try (
+ XSSFWorkbook wb = new XSSFWorkbook();
+ UnsynchronizedByteArrayOutputStream bos = UnsynchronizedByteArrayOutputStream.builder().get()
+ ) {
+ XSSFSheet sheet = wb.createSheet();
+
+ Row row = sheet.createRow(0);
+ // create cells in reverse order
+ row.createCell(1).setCellValue("def");
+ row.createCell(0).setCellValue("abc");
+
+ wb.write(bos);
+
+ validateOrder(bos.toInputStream());
+ }
+ }
+
+ @Test
+ void outOfOrderColumnsSXSSF() throws IOException {
+ try (
+ SXSSFWorkbook wb = new SXSSFWorkbook();
+ UnsynchronizedByteArrayOutputStream bos = UnsynchronizedByteArrayOutputStream.builder().get()
+ ) {
+ Sheet sheet = wb.createSheet();
+
+ Row row = sheet.createRow(0);
+ // create cells in reverse order
+ row.createCell(1).setCellValue("xyz");
+ row.createCell(0).setCellValue("uvw");
+
+ wb.write(bos);
+
+ validateOrder(bos.toInputStream());
+ }
+ }
+
+ @Test
+ /** this is the problematic case, as XSSF column sorting is skipped when saving with SXSSF. */
+ void mixOfXSSFAndSXSSF() throws IOException {
+ try (
+ XSSFWorkbook wb = new XSSFWorkbook();
+ UnsynchronizedByteArrayOutputStream bos = UnsynchronizedByteArrayOutputStream.builder().get()
+ ) {
+ XSSFSheet sheet = wb.createSheet();
+
+ Row row1 = sheet.createRow(0);
+ // create cells in reverse order
+ row1.createCell(1).setCellValue("def");
+ row1.createCell(0).setCellValue("abc");
+
+ try (SXSSFWorkbook sxssfWorkbook = new SXSSFWorkbook(wb)) {
+ Sheet sSheet = sxssfWorkbook.getSheetAt(0);
+ Row row2 = sSheet.createRow(1);
+ // create cells in reverse order
+ row2.createCell(1).setCellValue("xyz");
+ row2.createCell(0).setCellValue("uvw");
+
+ sxssfWorkbook.write(bos);
+
+ validateOrder(bos.toInputStream());
+ }
+ }
+ }
+
+ private void validateOrder(InputStream is) throws IOException {
+ // test if saved cells are in order
+ try (XSSFWorkbook xssfWorkbook = new XSSFWorkbook(is)) {
+ XSSFSheet xssfSheet = xssfWorkbook.getSheetAt(0);
+
+ Row resultRow = xssfSheet.getRow(0);
+ // POI doesn't show stored order because _cells TreeMap sorts it automatically.
+ // The only way to test is to compare the xml.
+ String s = resultRow.toString();
+ assertTrue(s.matches("(?s).*A1.*B1.*"), "unexpected order: " + s);
+ }
+ }
+
+}
diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hslf/usermodel/HSLFShapePlaceholderDetails.java b/poi-scratchpad/src/main/java/org/apache/poi/hslf/usermodel/HSLFShapePlaceholderDetails.java index 8a467dfedc..348d901235 100644 --- a/poi-scratchpad/src/main/java/org/apache/poi/hslf/usermodel/HSLFShapePlaceholderDetails.java +++ b/poi-scratchpad/src/main/java/org/apache/poi/hslf/usermodel/HSLFShapePlaceholderDetails.java @@ -47,6 +47,68 @@ import org.apache.poi.util.LocaleUtil; * @since POI 4.0.0 */ public class HSLFShapePlaceholderDetails extends HSLFPlaceholderDetails { + + static void updateSPRecord(final HSLFSimpleShape shape, final Placeholder placeholder) { + final EscherSpRecord spRecord = shape.getEscherChild(EscherSpRecord.RECORD_ID); + int flags = spRecord.getFlags(); + if (placeholder == null) { + flags ^= EscherSpRecord.FLAG_HAVEMASTER; + } else { + flags |= EscherSpRecord.FLAG_HAVEANCHOR | EscherSpRecord.FLAG_HAVEMASTER; + } + spRecord.setFlags(flags); + + // Placeholders can't be grouped + shape.setEscherProperty(EscherPropertyTypes.PROTECTION__LOCKAGAINSTGROUPING, (placeholder == null ? -1 : 262144)); + } + + static void removePlaceholder(final HSLFSimpleShape shape) { + final HSLFEscherClientDataRecord clientData = shape.getClientData(false); + if (clientData != null) { + clientData.removeChild(OEPlaceholderAtom.class); + clientData.removeChild(RoundTripHFPlaceholder12.class); + // remove client data if the placeholder was the only child to be carried + if (clientData.getChildRecords().isEmpty()) { + shape.getSpContainer().removeChildRecord(clientData); + } + } + } + + static OEPlaceholderAtom createOEPlaceholderAtom(final HSLFEscherClientDataRecord clientData) { + return createOEPlaceholderAtom(clientData, (byte) 0); + } + + static OEPlaceholderAtom createOEPlaceholderAtom(final HSLFEscherClientDataRecord clientData, + final byte placeholderId) { + OEPlaceholderAtom oePlaceholderAtom = new OEPlaceholderAtom(); + oePlaceholderAtom.setPlaceholderSize((byte)OEPlaceholderAtom.PLACEHOLDER_FULLSIZE); + // TODO: placement id only "SHOULD" be unique ... check other placeholders on sheet for unique id + oePlaceholderAtom.setPlacementId(-1); + oePlaceholderAtom.setPlaceholderId(placeholderId); + clientData.addChild(oePlaceholderAtom); + return oePlaceholderAtom; + } + + static byte getPlaceholderId(final HSLFSheet sheet, final Placeholder placeholder) { + if (placeholder == null) { + return 0; + } + byte phId; + // TODO: implement/switch NotesMaster + if (sheet instanceof HSLFSlideMaster) { + phId = (byte) placeholder.nativeSlideMasterId; + } else if (sheet instanceof HSLFNotes) { + phId = (byte) placeholder.nativeNotesId; + } else { + phId = (byte) placeholder.nativeSlideId; + } + + if (phId == -2) { + throw new HSLFException("Placeholder " + placeholder.name() + " not supported for this sheet type (" + sheet.getClass() + ")"); + } + return phId; + } + private enum PlaceholderContainer { slide, master, notes, notesMaster } @@ -78,7 +140,7 @@ public class HSLFShapePlaceholderDetails extends HSLFPlaceholderDetails { @Override public Placeholder getPlaceholder() { - updatePlaceholderAtom(false); + updatePlaceholderAtom(null, false); final int phId; if (oePlaceholderAtom != null) { phId = oePlaceholderAtom.getPlaceholderId(); @@ -105,17 +167,7 @@ public class HSLFShapePlaceholderDetails extends HSLFPlaceholderDetails { @Override public void setPlaceholder(final Placeholder placeholder) { - final EscherSpRecord spRecord = shape.getEscherChild(EscherSpRecord.RECORD_ID); - int flags = spRecord.getFlags(); - if (placeholder == null) { - flags ^= EscherSpRecord.FLAG_HAVEMASTER; - } else { - flags |= EscherSpRecord.FLAG_HAVEANCHOR | EscherSpRecord.FLAG_HAVEMASTER; - } - spRecord.setFlags(flags); - - // Placeholders can't be grouped - shape.setEscherProperty(EscherPropertyTypes.PROTECTION__LOCKAGAINSTGROUPING, (placeholder == null ? -1 : 262144)); + updateSPRecord(shape, placeholder); if (placeholder == null) { removePlaceholder(); @@ -123,7 +175,7 @@ public class HSLFShapePlaceholderDetails extends HSLFPlaceholderDetails { } // init client data - updatePlaceholderAtom(true); + updatePlaceholderAtom(placeholder, true); final byte phId = getPlaceholderId(placeholder); oePlaceholderAtom.setPlaceholderId(phId); @@ -158,7 +210,7 @@ public class HSLFShapePlaceholderDetails extends HSLFPlaceholderDetails { if (ph == null || size == null) { return; } - updatePlaceholderAtom(true); + updatePlaceholderAtom(ph, true); final byte ph_size; switch (size) { @@ -209,20 +261,12 @@ public class HSLFShapePlaceholderDetails extends HSLFPlaceholderDetails { } private void removePlaceholder() { - final HSLFEscherClientDataRecord clientData = shape.getClientData(false); - if (clientData != null) { - clientData.removeChild(OEPlaceholderAtom.class); - clientData.removeChild(RoundTripHFPlaceholder12.class); - // remove client data if the placeholder was the only child to be carried - if (clientData.getChildRecords().isEmpty()) { - shape.getSpContainer().removeChildRecord(clientData); - } - } + removePlaceholder(shape); oePlaceholderAtom = null; roundTripHFPlaceholder12 = null; } - private void updatePlaceholderAtom(final boolean create) { + private void updatePlaceholderAtom(final Placeholder placeholder, final boolean create) { localDateTime = null; if (shape instanceof HSLFTextBox) { EscherTextboxWrapper txtBox = ((HSLFTextBox)shape).getEscherTextboxWrapper(); @@ -255,11 +299,8 @@ public class HSLFShapePlaceholderDetails extends HSLFPlaceholderDetails { } if (oePlaceholderAtom == null) { - oePlaceholderAtom = new OEPlaceholderAtom(); - oePlaceholderAtom.setPlaceholderSize((byte)OEPlaceholderAtom.PLACEHOLDER_FULLSIZE); - // TODO: placement id only "SHOULD" be unique ... check other placeholders on sheet for unique id - oePlaceholderAtom.setPlacementId(-1); - clientData.addChild(oePlaceholderAtom); + final byte phId = getPlaceholderId(shape.getSheet(), placeholder); + oePlaceholderAtom = createOEPlaceholderAtom(clientData, phId); } if (roundTripHFPlaceholder12 == null) { roundTripHFPlaceholder12 = new RoundTripHFPlaceholder12(); diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hslf/usermodel/HSLFSimpleShape.java b/poi-scratchpad/src/main/java/org/apache/poi/hslf/usermodel/HSLFSimpleShape.java index da930b6899..aa85a65a92 100644 --- a/poi-scratchpad/src/main/java/org/apache/poi/hslf/usermodel/HSLFSimpleShape.java +++ b/poi-scratchpad/src/main/java/org/apache/poi/hslf/usermodel/HSLFSimpleShape.java @@ -20,6 +20,9 @@ package org.apache.poi.hslf.usermodel; import java.awt.Color; import org.apache.logging.log4j.Logger; +import org.apache.poi.hslf.record.HSLFEscherClientDataRecord; +import org.apache.poi.hslf.record.OEPlaceholderAtom; +import org.apache.poi.hslf.record.RoundTripHFPlaceholder12; import org.apache.poi.logging.PoiLogManager; import org.apache.poi.ddf.AbstractEscherOptRecord; import org.apache.poi.ddf.EscherChildAnchorRecord; @@ -53,6 +56,11 @@ import org.apache.poi.sl.usermodel.StrokeStyle.LineDash; import org.apache.poi.util.LittleEndian; import org.apache.poi.util.Units; +import static org.apache.poi.hslf.usermodel.HSLFShapePlaceholderDetails.createOEPlaceholderAtom; +import static org.apache.poi.hslf.usermodel.HSLFShapePlaceholderDetails.getPlaceholderId; +import static org.apache.poi.hslf.usermodel.HSLFShapePlaceholderDetails.removePlaceholder; +import static org.apache.poi.hslf.usermodel.HSLFShapePlaceholderDetails.updateSPRecord; + /** * An abstract simple (non-group) shape. * This is the parent class for all primitive shapes like Line, Rectangle, etc. @@ -80,6 +88,8 @@ public abstract class HSLFSimpleShape extends HSLFShape implements SimpleShape<H */ protected HSLFHyperlink _hyperlink; + protected HSLFShapePlaceholderDetails _placeholderDetails; + /** * Create a SimpleShape object and initialize it from the supplied Record container. * @@ -564,10 +574,12 @@ public abstract class HSLFSimpleShape extends HSLFShape implements SimpleShape<H @Override public HSLFShapePlaceholderDetails getPlaceholderDetails() { - return new HSLFShapePlaceholderDetails(this); + if (_placeholderDetails == null) { + _placeholderDetails = new HSLFShapePlaceholderDetails(this); + } + return _placeholderDetails; } - @Override public Placeholder getPlaceholder() { return getPlaceholderDetails().getPlaceholder(); @@ -575,9 +587,62 @@ public abstract class HSLFSimpleShape extends HSLFShape implements SimpleShape<H @Override public void setPlaceholder(Placeholder placeholder) { - getPlaceholderDetails().setPlaceholder(placeholder); - } + // reset the placeholder details so that the next call to getPlaceholderDetails() will reinitialize it + _placeholderDetails = null; + updateSPRecord(this, placeholder); + if (placeholder == null) { + removePlaceholder(this); + return; + } + + HSLFEscherClientDataRecord clientData = getClientData(true); + + // OEPlaceholderAtom tells powerpoint that this shape is a placeholder + OEPlaceholderAtom oep = null; + RoundTripHFPlaceholder12 rtp = null; + for (org.apache.poi.hslf.record.Record r : clientData.getHSLFChildRecords()) { + if (r instanceof OEPlaceholderAtom) { + oep = (OEPlaceholderAtom) r; + break; + } + if (r instanceof RoundTripHFPlaceholder12) { + rtp = (RoundTripHFPlaceholder12) r; + break; + } + } + + /** + * Extract from MSDN: + * + * There is a special case when the placeholder does not have a position in the layout. + * This occurs when the user has moved the placeholder from its original position. + * In this case the placeholder ID is -1. + */ + final byte phId = getPlaceholderId(getSheet(), placeholder); + + switch (placeholder) { + case HEADER: + case FOOTER: + if (rtp == null) { + rtp = new RoundTripHFPlaceholder12(); + rtp.setPlaceholderId(phId); + clientData.addChild(rtp); + } + if (oep != null) { + clientData.removeChild(OEPlaceholderAtom.class); + } + break; + default: + if (rtp != null) { + clientData.removeChild(RoundTripHFPlaceholder12.class); + } + if (oep == null) { + oep = createOEPlaceholderAtom(clientData, phId); + } + break; + } + } @Override public void setStrokeStyle(Object... styles) { diff --git a/poi-scratchpad/src/test/java/org/apache/poi/hslf/model/TestLine.java b/poi-scratchpad/src/test/java/org/apache/poi/hslf/model/TestLine.java index 0384b19043..e26793bde5 100644 --- a/poi-scratchpad/src/test/java/org/apache/poi/hslf/model/TestLine.java +++ b/poi-scratchpad/src/test/java/org/apache/poi/hslf/model/TestLine.java @@ -18,6 +18,8 @@ package org.apache.poi.hslf.model; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertNotNull; import java.awt.Color; import java.awt.Rectangle; @@ -27,9 +29,11 @@ import java.util.Arrays; import org.apache.poi.hslf.HSLFTestDataSamples; import org.apache.poi.hslf.usermodel.HSLFLine; +import org.apache.poi.hslf.usermodel.HSLFPlaceholder; import org.apache.poi.hslf.usermodel.HSLFShape; import org.apache.poi.hslf.usermodel.HSLFSlide; import org.apache.poi.hslf.usermodel.HSLFSlideShow; +import org.apache.poi.hslf.usermodel.HSLFTextBox; import org.apache.poi.sl.usermodel.StrokeStyle.LineCompound; import org.apache.poi.sl.usermodel.StrokeStyle.LineDash; import org.junit.jupiter.api.Test; @@ -62,9 +66,14 @@ public final class TestLine { @Test void testCreateLines() throws IOException { + final String title = "Lines tester"; try (HSLFSlideShow ppt1 = new HSLFSlideShow()) { HSLFSlide slide1 = ppt1.createSlide(); - slide1.addTitle().setText("Lines tester"); + HSLFTextBox titleBox = slide1.addTitle(); + titleBox.setText(title); + assertInstanceOf(HSLFPlaceholder.class, titleBox); + HSLFPlaceholder pl = (HSLFPlaceholder) titleBox; + assertNotNull(pl.getPlaceholder()); for (Object[] line : lines) { HSLFLine hslfLine = new HSLFLine(); @@ -85,6 +94,7 @@ public final class TestLine { try (HSLFSlideShow ppt2 = HSLFTestDataSamples.writeOutAndReadBack(ppt1)) { HSLFSlide slide2 = ppt2.getSlides().get(0); + assertEquals(title, slide2.getTitle()); int idx = 0; for (HSLFShape shape : slide2.getShapes().subList(1,14)) { diff --git a/poi/src/main/java/org/apache/poi/ss/util/CellUtil.java b/poi/src/main/java/org/apache/poi/ss/util/CellUtil.java index d0ee0db55b..b2879fb7fd 100644 --- a/poi/src/main/java/org/apache/poi/ss/util/CellUtil.java +++ b/poi/src/main/java/org/apache/poi/ss/util/CellUtil.java @@ -280,6 +280,7 @@ public final class CellUtil { map.put(LEFT_BORDER_COLOR, CellPropertyType.LEFT_BORDER_COLOR); map.put(RIGHT_BORDER_COLOR, CellPropertyType.RIGHT_BORDER_COLOR); map.put(TOP_BORDER_COLOR, CellPropertyType.TOP_BORDER_COLOR); + map.put(DATA_FORMAT, CellPropertyType.DATA_FORMAT); map.put(FILL_BACKGROUND_COLOR, CellPropertyType.FILL_BACKGROUND_COLOR); map.put(FILL_FOREGROUND_COLOR, CellPropertyType.FILL_FOREGROUND_COLOR); map.put(FILL_BACKGROUND_COLOR_COLOR, CellPropertyType.FILL_BACKGROUND_COLOR_COLOR); @@ -289,10 +290,11 @@ public final class CellUtil { map.put(HIDDEN, CellPropertyType.HIDDEN); map.put(INDENTION, CellPropertyType.INDENTION); map.put(LOCKED, CellPropertyType.LOCKED); + map.put(QUOTE_PREFIXED, CellPropertyType.QUOTE_PREFIXED); map.put(ROTATION, CellPropertyType.ROTATION); - map.put(VERTICAL_ALIGNMENT, CellPropertyType.VERTICAL_ALIGNMENT); map.put(SHRINK_TO_FIT, CellPropertyType.SHRINK_TO_FIT); - map.put(QUOTE_PREFIXED, CellPropertyType.QUOTE_PREFIXED); + map.put(VERTICAL_ALIGNMENT, CellPropertyType.VERTICAL_ALIGNMENT); + map.put(WRAP_TEXT, CellPropertyType.WRAP_TEXT); namePropertyMap = Collections.unmodifiableMap(map); } @@ -572,7 +574,7 @@ public final class CellUtil { @Deprecated @Removal(version = "7.0.0") public static void setCellStyleProperties(Cell cell, Map<String, Object> properties) { - EnumMap<CellPropertyType, Object> strPropMap = new EnumMap<>(CellPropertyType.class); + final EnumMap<CellPropertyType, Object> strPropMap = new EnumMap<>(CellPropertyType.class); properties.forEach((k, v) -> strPropMap.put(namePropertyMap.get(k), v)); setCellStyleProperties(cell, strPropMap, false); } @@ -685,10 +687,16 @@ public final class CellUtil { * @param cell The cell that is to be changed. * @param property The name of the property that is to be changed. * @param propertyValue The value of the property that is to be changed. - * + * @throws NullPointerException if {@code cell} or {@code property} is null * @since POI 5.4.0 */ public static void setCellStyleProperty(Cell cell, CellPropertyType property, Object propertyValue) { + if (cell == null) { + throw new NullPointerException("Cell must not be null"); + } + if (property == null) { + throw new NullPointerException("CellPropertyType must not be null"); + } boolean disableNullColorCheck = false; final Map<CellPropertyType, Object> propMap; if (CellPropertyType.FILL_FOREGROUND_COLOR_COLOR.equals(property) && propertyValue == null) { diff --git a/poi/src/test/java/org/apache/poi/ss/util/TestCellUtil.java b/poi/src/test/java/org/apache/poi/ss/util/TestCellUtil.java new file mode 100644 index 0000000000..b7defbef99 --- /dev/null +++ b/poi/src/test/java/org/apache/poi/ss/util/TestCellUtil.java @@ -0,0 +1,36 @@ +/* ==================================================================== + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership. + The ASF licenses this file to You under the Apache License, Version 2.0 + (the "License"); you may not use this file except in compliance with + the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +==================================================================== */ +package org.apache.poi.ss.util; + +import org.apache.poi.ss.usermodel.CellPropertyType; +import org.junit.jupiter.api.Test; + +import java.util.Arrays; + +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Test for CellUtil constants + */ +class TestCellUtil { + @Test + void testNamePropertyMap() { + Arrays.stream(CellPropertyType.values()).forEach(cellPropertyType -> + assertTrue(CellUtil.namePropertyMap.containsValue(cellPropertyType), + "missing " + cellPropertyType)); + } +} |