From e46ad7452c6abd214029d904a2224d21a82ac1e9 Mon Sep 17 00:00:00 2001 From: Andreas Beeker Date: Tue, 21 Jan 2020 22:02:10 +0000 Subject: [PATCH] Sonar fixes git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1873015 13f79535-47bb-0310-9956-ffa450edef68 --- .../apache/poi/stress/HPSFFileHandler.java | 21 +- .../apache/poi/stress/HSSFFileHandler.java | 4 +- .../apache/poi/stress/HWPFFileHandler.java | 20 +- .../apache/poi/stress/XDGFFileHandler.java | 3 +- .../apache/poi/stress/XWPFFileHandler.java | 3 +- .../poi/poifs/crypt/TestSignatureInfo.java | 246 ++++++++++-------- src/testcases/org/apache/poi/POITestCase.java | 48 +--- .../org/apache/poi/TestPOITestCase.java | 45 ---- .../apache/poi/hssf/dev/TestBiffViewer.java | 7 +- .../eventusermodel/TestHSSFEventFactory.java | 4 +- .../poi/hssf/record/TestFeatRecord.java | 17 +- .../poi/hssf/record/TestLabelRecord.java | 14 +- 12 files changed, 198 insertions(+), 234 deletions(-) diff --git a/src/integrationtest/org/apache/poi/stress/HPSFFileHandler.java b/src/integrationtest/org/apache/poi/stress/HPSFFileHandler.java index 680cac8398..cd6b601642 100644 --- a/src/integrationtest/org/apache/poi/stress/HPSFFileHandler.java +++ b/src/integrationtest/org/apache/poi/stress/HPSFFileHandler.java @@ -45,9 +45,9 @@ import org.junit.Test; public class HPSFFileHandler extends POIFSFileHandler { private static final String NL = System.getProperty("line.separator"); - + private static File copyOutput; - + static final Set EXCLUDES_HANDLE_ADD = unmodifiableHashSet( "spreadsheet/45290.xls", "spreadsheet/46904.xls", @@ -57,17 +57,17 @@ public class HPSFFileHandler extends POIFSFileHandler { "hpsf/Test_Humor-Generation.ppt", "document/word2.doc" ); - + static final Set EXCLUDES_HANDLE_FILE = unmodifiableHashSet( "hpsf/Test_Humor-Generation.ppt" ); - - + + private static Set unmodifiableHashSet(String... a) { return Collections.unmodifiableSet(new HashSet<>(Arrays.asList(a))); } - + @Override public void handleFile(InputStream stream, String path) throws Exception { Assume.assumeFalse(EXCLUDES_HANDLE_FILE.contains(path)); @@ -77,10 +77,10 @@ public class HPSFFileHandler extends POIFSFileHandler { SummaryInformation si = hpsf.getSummaryInformation(); boolean hasDSI = hasPropertyStream(poifs, DocumentSummaryInformation.DEFAULT_STREAM_NAME); boolean hasSI = hasPropertyStream(poifs, SummaryInformation.DEFAULT_STREAM_NAME); - + assertEquals(hasDSI, dsi != null); assertEquals(hasSI, si != null); - + handlePOIDocument(hpsf); } @@ -93,7 +93,7 @@ public class HPSFFileHandler extends POIFSFileHandler { return PropertySet.isPropertySetStream(dis); } } - + @Override public void handleAdditional(File file) throws Exception { Assume.assumeFalse(EXCLUDES_HANDLE_ADD.contains(file.getParentFile().getName()+"/"+file.getName())); @@ -114,10 +114,11 @@ public class HPSFFileHandler extends POIFSFileHandler { } } - + // a test-case to test this locally without executing the full TestAllFiles @Override @Test + @SuppressWarnings("java:S2699") public void test() throws Exception { String path = "test-data/hpsf/Test0313rur.adm"; try (InputStream stream = new FileInputStream(path)) { diff --git a/src/integrationtest/org/apache/poi/stress/HSSFFileHandler.java b/src/integrationtest/org/apache/poi/stress/HSSFFileHandler.java index 603231022e..b64105a93b 100644 --- a/src/integrationtest/org/apache/poi/stress/HSSFFileHandler.java +++ b/src/integrationtest/org/apache/poi/stress/HSSFFileHandler.java @@ -21,7 +21,6 @@ import static org.junit.Assert.assertNotNull; import java.io.File; import java.io.FileInputStream; -import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; import java.io.PrintStream; @@ -50,7 +49,7 @@ public class HSSFFileHandler extends SpreadsheetHandler { // So FormulaEvalTestData.xls now contains a few formulas that produce errors here. //HSSFFormulaEvaluator evaluator = new HSSFFormulaEvaluator(wb); //evaluator.evaluateAll(); - + delegate.handlePOIDocument(wb); // also try to see if some of the Records behave incorrectly @@ -122,6 +121,7 @@ public class HSSFFileHandler extends SpreadsheetHandler { // a test-case to test this locally without executing the full TestAllFiles @Test + @SuppressWarnings("java:S2699") public void testExtractor() throws Exception { handleExtracting(new File("test-data/spreadsheet/BOOK_in_capitals.xls")); } diff --git a/src/integrationtest/org/apache/poi/stress/HWPFFileHandler.java b/src/integrationtest/org/apache/poi/stress/HWPFFileHandler.java index 59f34870c6..cc10106d9e 100644 --- a/src/integrationtest/org/apache/poi/stress/HWPFFileHandler.java +++ b/src/integrationtest/org/apache/poi/stress/HWPFFileHandler.java @@ -47,25 +47,19 @@ public class HWPFFileHandler extends POIFSFileHandler { // a test-case to test this locally without executing the full TestAllFiles @Override @Test + @SuppressWarnings("java:S2699") public void test() throws Exception { File file = new File("test-data/document/52117.doc"); - InputStream stream = new FileInputStream(file); - try { + try (InputStream stream = new FileInputStream(file)) { handleFile(stream, file.getPath()); - } finally { - stream.close(); } - + handleExtracting(file); - - stream = new FileInputStream(file); - try { - try (WordExtractor extractor = new WordExtractor(stream)) { - assertNotNull(extractor.getText()); - } - } finally { - stream.close(); + + try (FileInputStream stream = new FileInputStream(file); + WordExtractor extractor = new WordExtractor(stream)) { + assertNotNull(extractor.getText()); } } diff --git a/src/integrationtest/org/apache/poi/stress/XDGFFileHandler.java b/src/integrationtest/org/apache/poi/stress/XDGFFileHandler.java index 302a5d2b76..0f00a9aeab 100644 --- a/src/integrationtest/org/apache/poi/stress/XDGFFileHandler.java +++ b/src/integrationtest/org/apache/poi/stress/XDGFFileHandler.java @@ -32,9 +32,10 @@ public class XDGFFileHandler extends AbstractFileHandler { XmlVisioDocument doc = new XmlVisioDocument(stream); new POIXMLDocumentHandler().handlePOIXMLDocument(doc); } - + // a test-case to test this locally without executing the full TestAllFiles @Test + @SuppressWarnings("java:S2699") public void test() throws Exception { try (OPCPackage pkg = OPCPackage.open("test-data/diagram/test.vsdx", PackageAccess.READ)) { XmlVisioDocument doc = new XmlVisioDocument(pkg); diff --git a/src/integrationtest/org/apache/poi/stress/XWPFFileHandler.java b/src/integrationtest/org/apache/poi/stress/XWPFFileHandler.java index 069cbee53e..582a28cadc 100644 --- a/src/integrationtest/org/apache/poi/stress/XWPFFileHandler.java +++ b/src/integrationtest/org/apache/poi/stress/XWPFFileHandler.java @@ -31,12 +31,13 @@ public class XWPFFileHandler extends AbstractFileHandler { if (POIXMLDocumentHandler.isEncrypted(stream)) return; XWPFDocument doc = new XWPFDocument(stream); - + new POIXMLDocumentHandler().handlePOIXMLDocument(doc); } // a test-case to test this locally without executing the full TestAllFiles @Test + @SuppressWarnings("java:S2699") public void test() throws Exception { File file = new File("test-data/document/51921-Word-Crash067.docx"); diff --git a/src/ooxml/testcases/org/apache/poi/poifs/crypt/TestSignatureInfo.java b/src/ooxml/testcases/org/apache/poi/poifs/crypt/TestSignatureInfo.java index 8a3fa772a9..e621f51e0d 100644 --- a/src/ooxml/testcases/org/apache/poi/poifs/crypt/TestSignatureInfo.java +++ b/src/ooxml/testcases/org/apache/poi/poifs/crypt/TestSignatureInfo.java @@ -18,9 +18,9 @@ /* ==================================================================== This product contains an ASLv2 licensed version of the OOXML signer package from the eID Applet project - http://code.google.com/p/eid-applet/source/browse/trunk/README.txt + http://code.google.com/p/eid-applet/source/browse/trunk/README.txt Copyright (C) 2008-2014 FedICT. - ================================================================= */ + ================================================================= */ package org.apache.poi.poifs.crypt; import static org.junit.Assert.assertEquals; @@ -76,9 +76,9 @@ import javax.xml.crypto.dsig.dom.DOMSignContext; import org.apache.jcp.xml.dsig.internal.dom.DOMSignedInfo; import org.apache.poi.EncryptedDocumentException; import org.apache.poi.POIDataSamples; -import org.apache.poi.POITestCase; import org.apache.poi.ooxml.util.DocumentHelper; import org.apache.poi.openxml4j.exceptions.InvalidFormatException; +import org.apache.poi.openxml4j.exceptions.OpenXML4JRuntimeException; import org.apache.poi.openxml4j.opc.OPCPackage; import org.apache.poi.openxml4j.opc.PackageAccess; import org.apache.poi.openxml4j.opc.PackageRelationshipTypes; @@ -157,7 +157,9 @@ import org.junit.AfterClass; import org.junit.Assume; import org.junit.BeforeClass; import org.junit.Ignore; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import org.w3.x2000.x09.xmldsig.ReferenceType; import org.w3.x2000.x09.xmldsig.SignatureDocument; import org.w3c.dom.Document; @@ -170,11 +172,14 @@ public class TestSignatureInfo { private KeyPair keyPair; private X509Certificate x509; + @Rule + public ExpectedException thrown = ExpectedException.none(); + @AfterClass public static void removeUserLocale() { LocaleUtil.resetUserLocale(); } - + @BeforeClass public static void initBouncy() { CryptoFunctions.registerBouncyCastle(); @@ -182,19 +187,19 @@ public class TestSignatureInfo { // Set cal to now ... only set to fixed date for debugging ... LocaleUtil.resetUserLocale(); LocaleUtil.resetUserTimeZone(); - + cal = LocaleUtil.getLocaleCalendar(LocaleUtil.TIMEZONE_UTC); assertNotNull(cal); - // don't run this test when we are using older Xerces as it triggers an XML Parser backwards compatibility issue + // don't run this test when we are using older Xerces as it triggers an XML Parser backwards compatibility issue // in the xmlsec jar file String additionalJar = System.getProperty("additionaljar"); //System.out.println("Having: " + additionalJar); - Assume.assumeTrue("Not running TestSignatureInfo because we are testing with additionaljar set to " + additionalJar, + Assume.assumeTrue("Not running TestSignatureInfo because we are testing with additionaljar set to " + additionalJar, additionalJar == null || additionalJar.trim().length() == 0); - + System.setProperty("org.apache.xml.security.ignoreLineBreaks", "true"); - + // Set line.separator for bug61182 // System.setProperty("line.separator", "\n"); } @@ -223,12 +228,12 @@ public class TestSignatureInfo { "7R4TQX/9/H6RiN34c9KldmPZZGANXzzTajZS9mR2OSvlJ+F4AgSko4htrMAKFTBu51/5SWNsO1vlRaaG48ZRJ+8PzuHQMdvS36gNpRPi7jhF1S"+ "H3B2ycI4y0VURv6SrqJNUY/X645ZFJQ+eBO+ptG7o8axf1dcqh2beiQk+GRTeZ37LVeUlaeo9vl1/+8tyBfyT2v5lFC5E19WdKIyCuZe7r99Px"+ "D/Od4Qj0TA92+DQnbCQTCMy/wwse9O4gsEebkkpPIP5GBV3Q0YBsj75XE0uSFQ1tCZSW8bNa9MUJZ/nPBfExohHlgGAAA="; - + Calendar cal = LocaleUtil.getLocaleCalendar(LocaleUtil.TIMEZONE_UTC); cal.clear(); cal.setTimeZone(LocaleUtil.TIMEZONE_UTC); cal.set(2017, Calendar.JULY, 1); - + SignatureConfig signatureConfig = prepareConfig("test", "CN=Test", pfxInput); signatureConfig.setExecutionTime(cal.getTime()); @@ -240,9 +245,9 @@ public class TestSignatureInfo { ByteArrayOutputStream bos = new ByteArrayOutputStream(100000); wb1.write(bos); wb1.close(); - + OPCPackage pkg1 = OPCPackage.open(new ByteArrayInputStream(bos.toByteArray())); - + signatureConfig.setOpcPackage(pkg1); si.confirmSignature(); assertTrue(si.verifySignature()); @@ -255,7 +260,7 @@ public class TestSignatureInfo { OPCPackage pkg2 = wb2.getPackage(); signatureConfig.setOpcPackage(pkg2); assertTrue(si.verifySignature()); - + // xmlbeans adds line-breaks depending on the system setting, so we get different // test results on Unix/Mac/Windows // if the xml documents eventually change, this test needs to be run with the @@ -279,15 +284,15 @@ public class TestSignatureInfo { "NZedY/LNTYU4nAUEUhIOg5+fKdgVtzRXKmdD3v+47E7Mb84oeiUGv9cCEE91DU3StF/JFIhjOJqavOzKnCsNcz"+ "NJ4j/inggUl1OJUsicqIGQnA7E8vzWnN1kf5lINgJLv+0PyrrX9sQZbItzxUpgqyOFYcD0trid+31nRt4wtaA="; } - + String signAct = si.getSignatureParts().iterator().next(). getSignatureDocument().getSignature().getSignatureValue().getStringValue(); assertEquals(signExp, signAct); - + pkg2.close(); wb2.close(); } - + @Test public void office2007prettyPrintedRels() throws Exception { try (OPCPackage pkg = OPCPackage.open(testdata.getFile("office2007prettyPrintedRels.docx"), PackageAccess.READ)) { @@ -299,7 +304,7 @@ public class TestSignatureInfo { assertTrue(isValid); } } - + @Test public void getSignerUnsigned() throws Exception { String[] testFiles = { @@ -308,7 +313,7 @@ public class TestSignatureInfo { "hello-world-unsigned.xlsx", "hello-world-office-2010-technical-preview-unsigned.docx" }; - + for (String testFile : testFiles) { OPCPackage pkg = OPCPackage.open(testdata.getFile(testFile), PackageAccess.READ); SignatureConfig sic = new SignatureConfig(); @@ -327,7 +332,7 @@ public class TestSignatureInfo { assertTrue(result.isEmpty()); } } - + @Test public void getSigner() throws Exception { String[] testFiles = { @@ -342,7 +347,7 @@ public class TestSignatureInfo { "Office2010-SP1-XAdES-X-L.docx", "signed.docx", }; - + for (String testFile : testFiles) { try (OPCPackage pkg = OPCPackage.open(testdata.getFile(testFile), PackageAccess.READ)) { SignatureConfig sic = new SignatureConfig(); @@ -395,7 +400,7 @@ public class TestSignatureInfo { pkg.revert(); } } - + @Test public void testSignSpreadsheet() throws Exception { String testFile = "hello-world-unsigned.xlsx"; @@ -404,55 +409,65 @@ public class TestSignatureInfo { pkg.close(); } + private static class CommitableWorkbook extends XSSFWorkbook { + CommitableWorkbook(OPCPackage pkg) throws IOException { + super(pkg); + } + public void commit() throws IOException { + super.commit(); + } + } + @Test public void testManipulation() throws Exception { // sign & validate String testFile = "hello-world-unsigned.xlsx"; - @SuppressWarnings("resource") - OPCPackage pkg = OPCPackage.open(copy(testdata.getFile(testFile)), PackageAccess.READ_WRITE); - sign(pkg, "Test", "CN=Test", 1); - - // manipulate - XSSFWorkbook wb = new XSSFWorkbook(pkg); - wb.setSheetName(0, "manipulated"); - // ... I don't know, why commit is protected ... - POITestCase.callMethod(XSSFWorkbook.class, wb, Void.class, "commit", new Class[0], new Object[0]); - - // todo: test a manipulation on a package part, which is not signed - // ... maybe in combination with #56164 - - // validate - SignatureConfig sic = new SignatureConfig(); - sic.setOpcPackage(pkg); - SignatureInfo si = new SignatureInfo(); - si.setSignatureConfig(sic); - boolean b = si.verifySignature(); - assertFalse("signature should be broken", b); - - wb.close(); + try (OPCPackage pkg = OPCPackage.open(copy(testdata.getFile(testFile)), PackageAccess.READ_WRITE)) { + sign(pkg, "Test", "CN=Test", 1); + + // manipulate + try (CommitableWorkbook wb = new CommitableWorkbook(pkg)) { + wb.setSheetName(0, "manipulated"); + // ... I don't know, why commit is protected ... + wb.commit(); + + // todo: test a manipulation on a package part, which is not signed + // ... maybe in combination with #56164 + + // validate + SignatureConfig sic = new SignatureConfig(); + sic.setOpcPackage(pkg); + SignatureInfo si = new SignatureInfo(); + si.setSignatureConfig(sic); + boolean b = si.verifySignature(); + assertFalse("signature should be broken", b); + } + thrown.expectMessage("Fail to save: an error occurs while saving the package : Zip File is close"); + thrown.expect(OpenXML4JRuntimeException.class); + } } - + @Test public void testSignSpreadsheetWithSignatureInfo() throws Exception { initKeyPair("Test", "CN=Test"); String testFile = "hello-world-unsigned.xlsx"; - OPCPackage pkg = OPCPackage.open(copy(testdata.getFile(testFile)), PackageAccess.READ_WRITE); - SignatureConfig sic = new SignatureConfig(); - sic.setOpcPackage(pkg); - sic.setKey(keyPair.getPrivate()); - sic.setSigningCertificateChain(Collections.singletonList(x509)); - SignatureInfo si = new SignatureInfo(); - si.setSignatureConfig(sic); - // hash > sha1 doesn't work in excel viewer ... - si.confirmSignature(); - List result = new ArrayList<>(); - for (SignaturePart sp : si.getSignatureParts()) { - if (sp.validate()) { - result.add(sp.getSigner()); + try (OPCPackage pkg = OPCPackage.open(copy(testdata.getFile(testFile)), PackageAccess.READ_WRITE)) { + SignatureConfig sic = new SignatureConfig(); + sic.setOpcPackage(pkg); + sic.setKey(keyPair.getPrivate()); + sic.setSigningCertificateChain(Collections.singletonList(x509)); + SignatureInfo si = new SignatureInfo(); + si.setSignatureConfig(sic); + // hash > sha1 doesn't work in excel viewer ... + si.confirmSignature(); + List result = new ArrayList<>(); + for (SignaturePart sp : si.getSignatureParts()) { + if (sp.validate()) { + result.add(sp.getSigner()); + } } + assertEquals(1, result.size()); } - assertEquals(1, result.size()); - pkg.close(); } @Test @@ -672,7 +687,7 @@ public class TestSignatureInfo { } } } - + @Test public void testCertChain() throws Exception { KeyStore keystore = KeyStore.getInstance("PKCS12"); @@ -689,32 +704,32 @@ public class TestSignatureInfo { } x509 = certChain.get(0); keyPair = new KeyPair(x509.getPublicKey(), (PrivateKey)key); - + String testFile = "hello-world-unsigned.xlsx"; - OPCPackage pkg = OPCPackage.open(copy(testdata.getFile(testFile)), PackageAccess.READ_WRITE); + try (OPCPackage pkg = OPCPackage.open(copy(testdata.getFile(testFile)), PackageAccess.READ_WRITE)) { - SignatureConfig signatureConfig = new SignatureConfig(); - signatureConfig.setKey(keyPair.getPrivate()); - signatureConfig.setSigningCertificateChain(certChain); - Calendar oldCal = LocaleUtil.getLocaleCalendar(2007, 7, 1); - signatureConfig.setExecutionTime(oldCal.getTime()); - signatureConfig.setDigestAlgo(HashAlgorithm.sha1); - signatureConfig.setOpcPackage(pkg); - - SignatureInfo si = new SignatureInfo(); - si.setSignatureConfig(signatureConfig); + SignatureConfig signatureConfig = new SignatureConfig(); + signatureConfig.setKey(keyPair.getPrivate()); + signatureConfig.setSigningCertificateChain(certChain); + Calendar oldCal = LocaleUtil.getLocaleCalendar(2007, 7, 1); + signatureConfig.setExecutionTime(oldCal.getTime()); + signatureConfig.setDigestAlgo(HashAlgorithm.sha1); + signatureConfig.setOpcPackage(pkg); + + SignatureInfo si = new SignatureInfo(); + si.setSignatureConfig(signatureConfig); + + si.confirmSignature(); + + for (SignaturePart sp : si.getSignatureParts()) { + assertTrue("Could not validate", sp.validate()); + X509Certificate signer = sp.getSigner(); + assertNotNull("signer undefined?!", signer); + List certChainRes = sp.getCertChain(); + assertEquals(3, certChainRes.size()); + } - si.confirmSignature(); - - for (SignaturePart sp : si.getSignatureParts()){ - assertTrue("Could not validate", sp.validate()); - X509Certificate signer = sp.getSigner(); - assertNotNull("signer undefined?!", signer); - List certChainRes = sp.getCertChain(); - assertEquals(3, certChainRes.size()); } - - pkg.close(); } @Test @@ -728,17 +743,17 @@ public class TestSignatureInfo { HashAlgorithm[] testAlgo = {HashAlgorithm.sha224, HashAlgorithm.sha256 , HashAlgorithm.sha384, HashAlgorithm.sha512, HashAlgorithm.ripemd160}; - + for (HashAlgorithm ha : testAlgo) { OPCPackage pkg = null; try { signatureConfig.setDigestAlgo(ha); pkg = OPCPackage.open(copy(testdata.getFile(testFile)), PackageAccess.READ_WRITE); signatureConfig.setOpcPackage(pkg); - + SignatureInfo si = new SignatureInfo(); si.setSignatureConfig(signatureConfig); - + si.confirmSignature(); boolean b = si.verifySignature(); assertTrue("Signature not correctly calculated for " + ha, b); @@ -756,28 +771,29 @@ public class TestSignatureInfo { public void bug58630() throws Exception { // test deletion of sheet 0 and signing File tpl = copy(testdata.getFile("bug58630.xlsx")); - SXSSFWorkbook wb1 = new SXSSFWorkbook((XSSFWorkbook)WorkbookFactory.create(tpl), 10); - wb1.setCompressTempFiles(true); - wb1.removeSheetAt(0); - ByteArrayOutputStream os = new ByteArrayOutputStream(); - wb1.write(os); - wb1.close(); - OPCPackage pkg = OPCPackage.open(new ByteArrayInputStream(os.toByteArray())); - - initKeyPair("Test", "CN=Test"); - SignatureConfig signatureConfig = new SignatureConfig(); - signatureConfig.setKey(keyPair.getPrivate()); - signatureConfig.setSigningCertificateChain(Collections.singletonList(x509)); - signatureConfig.setOpcPackage(pkg); - - SignatureInfo si = new SignatureInfo(); - si.setSignatureConfig(signatureConfig); - si.confirmSignature(); - assertTrue("invalid signature", si.verifySignature()); - - pkg.close(); + try (SXSSFWorkbook wb1 = new SXSSFWorkbook((XSSFWorkbook)WorkbookFactory.create(tpl), 10)) { + wb1.setCompressTempFiles(true); + wb1.removeSheetAt(0); + ByteArrayOutputStream os = new ByteArrayOutputStream(); + wb1.write(os); + wb1.close(); + try (OPCPackage pkg = OPCPackage.open(new ByteArrayInputStream(os.toByteArray()))) { + + initKeyPair("Test", "CN=Test"); + SignatureConfig signatureConfig = new SignatureConfig(); + signatureConfig.setKey(keyPair.getPrivate()); + signatureConfig.setSigningCertificateChain(Collections.singletonList(x509)); + signatureConfig.setOpcPackage(pkg); + + SignatureInfo si = new SignatureInfo(); + si.setSignatureConfig(signatureConfig); + si.confirmSignature(); + assertTrue("invalid signature", si.verifySignature()); + + } + } } - + @Test public void testMultiSign() throws Exception { cal = LocaleUtil.getLocaleCalendar(LocaleUtil.TIMEZONE_UTC); @@ -892,11 +908,11 @@ public class TestSignatureInfo { return signatureConfig; } - + private void sign(OPCPackage pkgCopy, String alias, String signerDn, int signerCount) throws Exception { SignatureConfig signatureConfig = prepareConfig(alias, signerDn, null); signatureConfig.setOpcPackage(pkgCopy); - + SignatureInfo si = new SignatureInfo(); si.setSignatureConfig(signatureConfig); @@ -912,7 +928,7 @@ public class TestSignatureInfo { // setup: key material, signature value final String signatureValue = si.signDigest(xmlSignContext, signedInfo); - + // operate: postSign si.postSign(xmlSignContext, signatureValue); @@ -930,7 +946,7 @@ public class TestSignatureInfo { private void initKeyPair(String alias, String subjectDN) throws Exception { initKeyPair(alias, subjectDN, null); } - + private void initKeyPair(String alias, String subjectDN, String pfxInput) throws Exception { final char[] password = "test".toCharArray(); File file = new File("build/test.pfx"); @@ -941,7 +957,7 @@ public class TestSignatureInfo { InputStream fis = new ByteArrayInputStream(RawDataUtil.decompress(pfxInput)); keystore.load(fis, password); fis.close(); - } else if (file.exists()) { + } else if (file.exists()) { InputStream fis = new FileInputStream(file); keystore.load(fis, password); fis.close(); @@ -960,12 +976,12 @@ public class TestSignatureInfo { cal2.add(Calendar.YEAR, 1); Date notAfter = cal2.getTime(); KeyUsage keyUsage = new KeyUsage(KeyUsage.digitalSignature); - + x509 = generateCertificate(keyPair.getPublic(), subjectDN , notBefore, notAfter, null, keyPair.getPrivate(), true, 0, null, null, keyUsage); keystore.setKeyEntry(alias, keyPair.getPrivate(), password, new Certificate[]{x509}); - + if (pfxInput == null) { FileOutputStream fos = new FileOutputStream(file); keystore.store(fos, password); @@ -997,7 +1013,7 @@ public class TestSignatureInfo { private static File copy(File input) throws IOException { String extension = input.getName().replaceAll(".*?(\\.[^.]+)?$", "$1"); - if (extension == null || extension.isEmpty()) { + if (extension.isEmpty()) { extension = ".zip"; } diff --git a/src/testcases/org/apache/poi/POITestCase.java b/src/testcases/org/apache/poi/POITestCase.java index 8dfd5e0ce0..8848a37695 100644 --- a/src/testcases/org/apache/poi/POITestCase.java +++ b/src/testcases/org/apache/poi/POITestCase.java @@ -26,10 +26,9 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; -import static org.junit.Assume.assumeTrue; +import static org.junit.Assume.assumeNoException; import java.lang.reflect.Field; -import java.lang.reflect.Method; import java.security.AccessController; import java.security.PrivilegedActionException; import java.security.PrivilegedExceptionAction; @@ -38,7 +37,6 @@ import java.util.Map; import java.util.Set; import org.apache.poi.util.Internal; -import org.apache.poi.util.SuppressForbidden; import org.mockito.internal.matchers.apachecommons.ReflectionEquals; /** @@ -47,6 +45,9 @@ import org.mockito.internal.matchers.apachecommons.ReflectionEquals; @Internal public final class POITestCase { + private POITestCase() { + } + public static void assertStartsWith(String string, String prefix) { assertNotNull(string); assertNotNull(prefix); @@ -118,52 +119,25 @@ public final class POITestCase { * Utility method to get the value of a private/protected field. * Only use this method in test cases!!! */ + @SuppressWarnings({"unused", "unchecked"}) public static R getFieldValue(final Class clazz, final T instance, final Class fieldType, final String fieldName) { assertTrue("Reflection of private fields is only allowed for POI classes.", clazz.getName().startsWith("org.apache.poi.")); try { - return AccessController.doPrivileged(new PrivilegedExceptionAction() { - @Override - @SuppressWarnings("unchecked") - @SuppressForbidden("For test usage only") - public R run() throws Exception { - Field f = clazz.getDeclaredField(fieldName); - f.setAccessible(true); - return (R) f.get(instance); - } + return AccessController.doPrivileged((PrivilegedExceptionAction) () -> { + Field f = clazz.getDeclaredField(fieldName); + f.setAccessible(true); + return (R) f.get(instance); }); } catch (PrivilegedActionException pae) { throw new RuntimeException("Cannot access field '" + fieldName + "' of class " + clazz, pae.getException()); } } - /** - * Utility method to call a private/protected method. - * Only use this method in test cases!!! - */ - public static R callMethod(final Class clazz, final T instance, final Class returnType, final String methodName, - final Class[] parameterTypes, final Object[] parameters) { - assertTrue("Reflection of private methods is only allowed for POI classes.", clazz.getName().startsWith("org.apache.poi.")); - try { - return AccessController.doPrivileged(new PrivilegedExceptionAction() { - @Override - @SuppressWarnings("unchecked") - @SuppressForbidden("For test usage only") - public R run() throws Exception { - Method m = clazz.getDeclaredMethod(methodName, parameterTypes); - m.setAccessible(true); - return (R) m.invoke(instance, parameters); - } - }); - } catch (PrivilegedActionException pae) { - throw new RuntimeException("Cannot access method '" + methodName + "' of class " + clazz, pae.getException()); - } - } - /** * Utility method to shallow compare all fields of the objects * Only use this method in test cases!!! */ - public static void assertReflectEquals(final Object expected, Object actual) throws Exception { + public static void assertReflectEquals(final Object expected, Object actual) { // as long as ReflectionEquals is provided by Mockito, use it ... otherwise use commons.lang for the tests // JaCoCo Code Coverage adds its own field, don't look at this one here @@ -217,7 +191,7 @@ public final class POITestCase { * be raised when the bug is fixed */ public static void skipTest(Throwable e) { - assumeTrue("This test currently fails with " + e, false); + assumeNoException("This test currently fails with", e); } /** * @see #skipTest(Throwable) diff --git a/src/testcases/org/apache/poi/TestPOITestCase.java b/src/testcases/org/apache/poi/TestPOITestCase.java index 42310738fa..60f5fb9cb7 100644 --- a/src/testcases/org/apache/poi/TestPOITestCase.java +++ b/src/testcases/org/apache/poi/TestPOITestCase.java @@ -18,20 +18,14 @@ package org.apache.poi; import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertTrue; -import java.io.ByteArrayInputStream; import java.io.IOException; import java.util.Collections; import java.util.Locale; import java.util.Map; -import org.apache.poi.poifs.filesystem.DirectoryNode; -import org.apache.poi.poifs.filesystem.DocumentEntry; -import org.apache.poi.poifs.filesystem.EntryNode; import org.apache.poi.poifs.filesystem.POIFSFileSystem; import org.apache.poi.poifs.property.PropertyTable; -import org.junit.Ignore; import org.junit.Test; /** @@ -55,37 +49,27 @@ public final class TestPOITestCase { @Test public void assertContains() { POITestCase.assertContains("There is a needle in this haystack", "needle"); - /*try { - POITestCase.assertContains("There is gold in this haystack", "needle"); - fail("found a needle"); - } catch (final junit.framework.AssertionFailedError e) { - // expected - }*/ } @Test public void assertContainsIgnoreCase_Locale() { POITestCase.assertContainsIgnoreCase("There is a Needle in this haystack", "needlE", Locale.ROOT); - // FIXME: test failing case } @Test public void assertContainsIgnoreCase() { POITestCase.assertContainsIgnoreCase("There is a Needle in this haystack", "needlE"); - // FIXME: test failing case } @Test public void assertNotContained() { POITestCase.assertNotContained("There is a needle in this haystack", "gold"); - // FIXME: test failing case } @Test public void assertMapContains() { Map haystack = Collections.singletonMap("needle", "value"); POITestCase.assertContains(haystack, "needle"); - // FIXME: test failing case } @@ -100,33 +84,4 @@ public final class TestPOITestCase { assertNotNull(actual); } } - - /** - * Utility method to call a private/protected method. - * Only use this method in test cases!!! - */ - @Ignore - @Test - public void callMethod() throws IOException { - try (POIFSFileSystem fs = new POIFSFileSystem()) { - DirectoryNode root = fs.getRoot(); - DocumentEntry doc = fs.createDocument(new ByteArrayInputStream(new byte[]{1, 2, 3}), "foobaa"); - boolean actual = POITestCase.callMethod(DirectoryNode.class, root, boolean.class, "deleteEntry", new Class[]{EntryNode.class}, new Object[]{doc}); - assertTrue(actual); - } - } - - /** - * Utility method to shallow compare all fields of the objects - * Only use this method in test cases!!! - */ - @Ignore - @Test - public void assertReflectEquals() throws Exception { - /* - final Object expected; - final Object actual; - POITestCase.assertReflectEquals(expected, actual); - */ - } } diff --git a/src/testcases/org/apache/poi/hssf/dev/TestBiffViewer.java b/src/testcases/org/apache/poi/hssf/dev/TestBiffViewer.java index 32fa81383e..a62431bc55 100644 --- a/src/testcases/org/apache/poi/hssf/dev/TestBiffViewer.java +++ b/src/testcases/org/apache/poi/hssf/dev/TestBiffViewer.java @@ -37,10 +37,10 @@ public class TestBiffViewer extends BaseXLSIteratingTest { @BeforeClass public static void setup() { EXCLUDED.clear(); - EXCLUDED.put("35897-type4.xls", EncryptedDocumentException.class); // unsupported crypto api header + EXCLUDED.put("35897-type4.xls", EncryptedDocumentException.class); // unsupported crypto api header EXCLUDED.put("51832.xls", EncryptedDocumentException.class); - EXCLUDED.put("xor-encryption-abc.xls", EncryptedDocumentException.class); - EXCLUDED.put("password.xls", EncryptedDocumentException.class); + EXCLUDED.put("xor-encryption-abc.xls", EncryptedDocumentException.class); + EXCLUDED.put("password.xls", EncryptedDocumentException.class); EXCLUDED.put("46904.xls", OldExcelFormatException.class); EXCLUDED.put("59074.xls", OldExcelFormatException.class); EXCLUDED.put("testEXCEL_2.xls", OldExcelFormatException.class); // Biff 2 / Excel 2, pre-OLE2 @@ -68,6 +68,7 @@ public class TestBiffViewer extends BaseXLSIteratingTest { @Test @Ignore("only used for manual tests") + @SuppressWarnings("java:S2699") public void testOneFile() throws Exception { POIDataSamples samples = POIDataSamples.getSpreadSheetInstance(); runOneFile(samples.getFile("43493.xls")); diff --git a/src/testcases/org/apache/poi/hssf/eventusermodel/TestHSSFEventFactory.java b/src/testcases/org/apache/poi/hssf/eventusermodel/TestHSSFEventFactory.java index f3d4569963..cb1015f729 100644 --- a/src/testcases/org/apache/poi/hssf/eventusermodel/TestHSSFEventFactory.java +++ b/src/testcases/org/apache/poi/hssf/eventusermodel/TestHSSFEventFactory.java @@ -36,7 +36,6 @@ import org.apache.poi.hssf.record.DVRecord; import org.apache.poi.hssf.record.EOFRecord; import org.apache.poi.hssf.record.FeatHdrRecord; import org.apache.poi.hssf.record.NumberRecord; -import org.apache.poi.hssf.record.Record; import org.apache.poi.hssf.record.SelectionRecord; import org.apache.poi.hssf.record.WindowTwoRecord; import org.apache.poi.hssf.record.crypto.Biff8EncryptionKey; @@ -104,17 +103,20 @@ public final class TestHSSFEventFactory { * (the test file was provided in a reopen of bug #42844) */ @Test + @SuppressWarnings("java:S2699") public void testUnknownContinueRecords() throws Exception { openSample("42844.xls"); } @Test + @SuppressWarnings("java:S2699") public void testWithDifferentWorkbookName() throws Exception { openSample("BOOK_in_capitals.xls"); openSample("WORKBOOK_in_capitals.xls"); } @Test(expected = EncryptedDocumentException.class) + @SuppressWarnings("java:S2699") public void testWithPasswordProtectedWorkbooksNoPass() throws Exception { // Without a password, can't be read openSample("xor-encryption-abc.xls"); diff --git a/src/testcases/org/apache/poi/hssf/record/TestFeatRecord.java b/src/testcases/org/apache/poi/hssf/record/TestFeatRecord.java index b2e75a642b..fe56976545 100644 --- a/src/testcases/org/apache/poi/hssf/record/TestFeatRecord.java +++ b/src/testcases/org/apache/poi/hssf/record/TestFeatRecord.java @@ -23,6 +23,8 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import java.io.IOException; + import org.apache.poi.hssf.HSSFTestDataSamples; import org.apache.poi.hssf.model.InternalSheet; import org.apache.poi.hssf.model.InternalWorkbook; @@ -181,9 +183,16 @@ public final class TestFeatRecord { * cloning sheets with feat records */ @Test - public void testCloneSheetWithFeatRecord() { - HSSFWorkbook wb = - HSSFTestDataSamples.openSampleWorkbook("46136-WithWarnings.xls"); - wb.cloneSheet(0); + public void testCloneSheetWithFeatRecord() throws IOException { + try (HSSFWorkbook wb = + HSSFTestDataSamples.openSampleWorkbook("46136-WithWarnings.xls")) { + HSSFSheet src = wb.getSheetAt(0); + HSSFSheet dst = wb.cloneSheet(0); + + InternalSheet isrc = HSSFTestHelper.getSheetForTest(src); + InternalSheet idst = HSSFTestHelper.getSheetForTest(dst); + + assertEquals(isrc.getRecords().size(), idst.getRecords().size()); + } } } diff --git a/src/testcases/org/apache/poi/hssf/record/TestLabelRecord.java b/src/testcases/org/apache/poi/hssf/record/TestLabelRecord.java index f244a71b26..ec5ed36689 100644 --- a/src/testcases/org/apache/poi/hssf/record/TestLabelRecord.java +++ b/src/testcases/org/apache/poi/hssf/record/TestLabelRecord.java @@ -17,9 +17,12 @@ package org.apache.poi.hssf.record; +import static org.junit.Assert.assertEquals; + import java.io.IOException; import org.apache.poi.hssf.HSSFTestDataSamples; +import org.apache.poi.hssf.usermodel.HSSFSheet; import org.apache.poi.hssf.usermodel.HSSFWorkbook; import org.junit.Test; @@ -30,8 +33,15 @@ public final class TestLabelRecord { @Test public void testEmptyString() throws IOException { - try (HSSFWorkbook wb = HSSFTestDataSamples.openSampleWorkbook("ex42570-20305.xls")) { - HSSFTestDataSamples.writeOutAndReadBack(wb); + try (HSSFWorkbook wb1 = HSSFTestDataSamples.openSampleWorkbook("ex42570-20305.xls"); + HSSFWorkbook wb2 = HSSFTestDataSamples.writeOutAndReadBack(wb1)) { + HSSFSheet s1 = wb1.getSheetAt(0); + HSSFSheet s2 = wb2.getSheetAt(0); + for (int c=0; c<2; c++) { + for (int r=0; r<146; r++) { + assertEquals(s1.getRow(r).getCell(c).getNumericCellValue(), s2.getRow(r).getCell(c).getNumericCellValue(), 0); + } + } } } } -- 2.39.5