From a731bf3ab28f7f7760421e5f9b90916bc3cf627c Mon Sep 17 00:00:00 2001 From: Simon Steiner Date: Thu, 27 Jul 2023 10:07:49 +0100 Subject: FOP-2903: Do not delete files on syntax errors using command line by Dave Roxburgh --- .../org/apache/fop/cli/CommandLineOptions.java | 21 +++--- .../src/main/java/org/apache/fop/cli/Main.java | 21 ++++-- .../fop/cli/CommandLineOptions2TestCase.java | 74 ++++++++++++++++++++++ 3 files changed, 104 insertions(+), 12 deletions(-) create mode 100644 fop-core/src/test/java/org/apache/fop/cli/CommandLineOptions2TestCase.java diff --git a/fop-core/src/main/java/org/apache/fop/cli/CommandLineOptions.java b/fop-core/src/main/java/org/apache/fop/cli/CommandLineOptions.java index f62b38670..8d824b960 100644 --- a/fop-core/src/main/java/org/apache/fop/cli/CommandLineOptions.java +++ b/fop-core/src/main/java/org/apache/fop/cli/CommandLineOptions.java @@ -102,6 +102,8 @@ public class CommandLineOptions { private File iffile; /* area tree input file */ private File imagefile; + /* output filename */ + private String outfilename; /* output file */ private File outfile; /* input mode */ @@ -403,6 +405,9 @@ public class CommandLineOptions { System.exit(1); } } + if (outfilename != null) { + outfile = new File(outfilename); + } return true; } // end parseOptions @@ -527,7 +532,7 @@ public class CommandLineOptions { if (isSystemInOutFile(filename)) { this.useStdOut = true; } else { - outfile = new File(filename); + outfilename = filename; } } @@ -1351,15 +1356,15 @@ public class CommandLineOptions { log.info("not set"); } else if (MimeConstants.MIME_FOP_AWT_PREVIEW.equals(outputmode)) { log.info("awt on screen"); - if (outfile != null) { + if (outfilename != null) { log.error("awt mode, but outfile is set:"); - log.error("out file: " + outfile.toString()); + log.error("out file: " + outfilename); } } else if (MimeConstants.MIME_FOP_PRINT.equals(outputmode)) { log.info("print directly"); - if (outfile != null) { + if (outfilename != null) { log.error("print mode, but outfile is set:"); - log.error("out file: " + outfile.toString()); + log.error("out file: " + outfilename); } } else if (MimeConstants.MIME_FOP_AREA_TREE.equals(outputmode)) { log.info("area tree"); @@ -1369,17 +1374,17 @@ public class CommandLineOptions { if (isOutputToStdOut()) { log.info("output file: to stdout"); } else { - log.info("output file: " + outfile.toString()); + log.info("output file: " + outfilename); } } else if (MimeConstants.MIME_FOP_IF.equals(outputmode)) { log.info("intermediate format"); - log.info("output file: " + outfile.toString()); + log.info("output file: " + outfilename); } else { log.info(outputmode); if (isOutputToStdOut()) { log.info("output file: to stdout"); } else { - log.info("output file: " + outfile.toString()); + log.info("output file: " + outfilename); } } diff --git a/fop-core/src/main/java/org/apache/fop/cli/Main.java b/fop-core/src/main/java/org/apache/fop/cli/Main.java index b8ba68df9..a14d20c78 100644 --- a/fop-core/src/main/java/org/apache/fop/cli/Main.java +++ b/fop-core/src/main/java/org/apache/fop/cli/Main.java @@ -146,14 +146,19 @@ public final class Main { } } + public static void startFOP(String[] args) { + startFOP(args, new SystemWrapper()); + } + /** * Executes FOP with the given arguments. If no argument is provided, returns its * version number as well as a short usage statement; if '-v' is provided, returns its * version number alone; if '-h' is provided, returns its short help message. * * @param args command-line arguments + * @param systemWrapper Object on which exit() is to be called. */ - public static void startFOP(String[] args) { + public static void startFOP(String[] args, SystemWrapper systemWrapper) { //System.out.println("static CCL: " // + Thread.currentThread().getContextClassLoader().toString()); //System.out.println("static CL: " + Fop.class.getClassLoader().toString()); @@ -165,7 +170,7 @@ public final class Main { options = new CommandLineOptions(); if (!options.parse(args)) { // @SuppressFBWarnings("DM_EXIT") - System.exit(0); + systemWrapper.exit(0); } foUserAgent = options.getFOUserAgent(); @@ -192,7 +197,7 @@ public final class Main { // AWTRenderer closes with window shutdown, so exit() should not // be called here if (!MimeConstants.MIME_FOP_AWT_PREVIEW.equals(outputFormat)) { - System.exit(0); + systemWrapper.exit(0); } } catch (Exception e) { if (options != null) { @@ -201,7 +206,7 @@ public final class Main { options.getOutputFile().delete(); } } - System.exit(1); + systemWrapper.exit(1); } } @@ -217,4 +222,12 @@ public final class Main { } } + /** + * Wrapper to support dependency injection. + */ + public static class SystemWrapper { + public void exit(int status) { + System.exit(status); + } + } } diff --git a/fop-core/src/test/java/org/apache/fop/cli/CommandLineOptions2TestCase.java b/fop-core/src/test/java/org/apache/fop/cli/CommandLineOptions2TestCase.java new file mode 100644 index 000000000..518069154 --- /dev/null +++ b/fop-core/src/test/java/org/apache/fop/cli/CommandLineOptions2TestCase.java @@ -0,0 +1,74 @@ +/* + * 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.fop.cli; + +import java.io.File; +import java.io.IOException; + +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; + +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; + +import org.apache.fop.apps.FOPException; + +public class CommandLineOptions2TestCase { + + @Rule + public ExpectedException exceptionRule = ExpectedException.none(); + + /** + * Check that the parser detects the case where the -xsl switch has been omitted. + * This detection prevents the XSL file being deleted. + * @throws FOPException + * @throws IOException + */ + @Test + public void testXslSwitchMissing() throws FOPException, IOException { + final String xslFilename = "../fop/examples/embedding/xml/xslt/projectteam2fo.xsl"; + final String outputFilename = "out.pdf"; + exceptionRule.expect(FOPException.class); + exceptionRule.expectMessage("Don't know what to do with " + outputFilename); + // -xsl switch omitted. + String cmdLine = "-xml ../fop/examples/embedding/xml/xml/projectteam.xml " + xslFilename + " " + outputFilename; + String[] args = cmdLine.split(" "); + CommandLineOptions clo = new CommandLineOptions(); + clo.parse(args); + } + + /** + * Check that the XSL file is not deleted in the case where the -xsl switch has been omitted. + * @throws FOPException + * @throws IOException + */ + @Test + public void testXslFileNotDeleted() { + Main.SystemWrapper mockSystemWrapper = mock(Main.SystemWrapper.class); + final String xslFilename = "../fop/examples/embedding/xml/xslt/projectteam2fo.xsl"; + // -xsl switch omitted. + String cmdLine = "-xml ../fop/examples/embedding/xml/xml/projectteam.xml " + xslFilename + " out.pdf"; + String[] args = cmdLine.split(" "); + Main.startFOP(args, mockSystemWrapper); + verify(mockSystemWrapper).exit(1); + File file = new File(xslFilename); + assertTrue(file.exists()); + } +} -- cgit v1.2.3