Repository: maven Updated Branches: refs/heads/master c8a200a72 -> 8a51f9e51
[MNG-5681] Properties on command line with leading or trailing quotes are stripped Refactored out cleanArgs method from CLIManager into separate class and added appropriate tests which proves the solution of the issue. Project: http://git-wip-us.apache.org/repos/asf/maven/repo Commit: http://git-wip-us.apache.org/repos/asf/maven/commit/8a51f9e5 Tree: http://git-wip-us.apache.org/repos/asf/maven/tree/8a51f9e5 Diff: http://git-wip-us.apache.org/repos/asf/maven/diff/8a51f9e5 Branch: refs/heads/master Commit: 8a51f9e5121a4cff75fbc7ddaf0b7eea9c9d4e10 Parents: c8a200a Author: Karl Heinz Marbaise <khmarba...@apache.org> Authored: Wed Oct 7 16:41:29 2015 +0200 Committer: Karl Heinz Marbaise <khmarba...@apache.org> Committed: Wed Oct 7 17:13:43 2015 +0200 ---------------------------------------------------------------------- .../java/org/apache/maven/cli/CLIManager.java | 98 +-------------- .../org/apache/maven/cli/CleanArgument.java | 122 +++++++++++++++++++ .../maven/cli/CLIManagerDocumentationTest.java | 107 ++++++++++++++++ .../org/apache/maven/cli/CLIManagerTest.java | 108 ---------------- .../org/apache/maven/cli/CleanArgumentTest.java | 61 ++++++++++ pom.xml | 28 ++--- 6 files changed, 305 insertions(+), 219 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/maven/blob/8a51f9e5/maven-embedder/src/main/java/org/apache/maven/cli/CLIManager.java ---------------------------------------------------------------------- diff --git a/maven-embedder/src/main/java/org/apache/maven/cli/CLIManager.java b/maven-embedder/src/main/java/org/apache/maven/cli/CLIManager.java index b870c66..f461835 100644 --- a/maven-embedder/src/main/java/org/apache/maven/cli/CLIManager.java +++ b/maven-embedder/src/main/java/org/apache/maven/cli/CLIManager.java @@ -21,8 +21,6 @@ package org.apache.maven.cli; import java.io.PrintStream; import java.io.PrintWriter; -import java.util.ArrayList; -import java.util.List; import org.apache.commons.cli.CommandLine; import org.apache.commons.cli.CommandLineParser; @@ -154,107 +152,13 @@ public class CLIManager throws ParseException { // We need to eat any quotes surrounding arguments... - String[] cleanArgs = cleanArgs( args ); + String[] cleanArgs = CleanArgument.cleanArgs( args ); CommandLineParser parser = new GnuParser(); return parser.parse( options, cleanArgs ); } - private String[] cleanArgs( String[] args ) - { - List<String> cleaned = new ArrayList<>(); - - StringBuilder currentArg = null; - - for ( String arg : args ) - { - boolean addedToBuffer = false; - - if ( arg.startsWith( "\"" ) ) - { - // if we're in the process of building up another arg, push it and start over. - // this is for the case: "-Dfoo=bar "-Dfoo2=bar two" (note the first unterminated quote) - if ( currentArg != null ) - { - cleaned.add( currentArg.toString() ); - } - - // start building an argument here. - currentArg = new StringBuilder( arg.substring( 1 ) ); - addedToBuffer = true; - } - - // this has to be a separate "if" statement, to capture the case of: "-Dfoo=bar" - if ( arg.endsWith( "\"" ) ) - { - String cleanArgPart = arg.substring( 0, arg.length() - 1 ); - - // if we're building an argument, keep doing so. - if ( currentArg != null ) - { - // if this is the case of "-Dfoo=bar", then we need to adjust the buffer. - if ( addedToBuffer ) - { - currentArg.setLength( currentArg.length() - 1 ); - } - // otherwise, we trim the trailing " and append to the buffer. - else - { - // TODO: introducing a space here...not sure what else to do but collapse whitespace - currentArg.append( ' ' ).append( cleanArgPart ); - } - - cleaned.add( currentArg.toString() ); - } - else - { - cleaned.add( cleanArgPart ); - } - - currentArg = null; - - continue; - } - - // if we haven't added this arg to the buffer, and we ARE building an argument - // buffer, then append it with a preceding space...again, not sure what else to - // do other than collapse whitespace. - // NOTE: The case of a trailing quote is handled by nullifying the arg buffer. - if ( !addedToBuffer ) - { - if ( currentArg != null ) - { - currentArg.append( ' ' ).append( arg ); - } - else - { - cleaned.add( arg ); - } - } - } - - if ( currentArg != null ) - { - cleaned.add( currentArg.toString() ); - } - - int cleanedSz = cleaned.size(); - - String[] cleanArgs; - - if ( cleanedSz == 0 ) - { - cleanArgs = args; - } - else - { - cleanArgs = cleaned.toArray( new String[cleanedSz] ); - } - - return cleanArgs; - } - public void displayHelp( PrintStream stdout ) { stdout.println(); http://git-wip-us.apache.org/repos/asf/maven/blob/8a51f9e5/maven-embedder/src/main/java/org/apache/maven/cli/CleanArgument.java ---------------------------------------------------------------------- diff --git a/maven-embedder/src/main/java/org/apache/maven/cli/CleanArgument.java b/maven-embedder/src/main/java/org/apache/maven/cli/CleanArgument.java new file mode 100644 index 0000000..192ed3b --- /dev/null +++ b/maven-embedder/src/main/java/org/apache/maven/cli/CleanArgument.java @@ -0,0 +1,122 @@ +package org.apache.maven.cli; + +/* + * 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. + */ + +import java.util.ArrayList; +import java.util.List; + +public class CleanArgument +{ + public static String[] cleanArgs( String[] args ) + { + List<String> cleaned = new ArrayList<>(); + + StringBuilder currentArg = null; + + for ( String arg : args ) + { + boolean addedToBuffer = false; + + if ( arg.startsWith( "\"" ) ) + { + // if we're in the process of building up another arg, push it and start over. + // this is for the case: "-Dfoo=bar "-Dfoo2=bar two" (note the first unterminated quote) + if ( currentArg != null ) + { + cleaned.add( currentArg.toString() ); + } + + // start building an argument here. + currentArg = new StringBuilder( arg.substring( 1 ) ); + addedToBuffer = true; + } + + // this has to be a separate "if" statement, to capture the case of: "-Dfoo=bar" + if ( addedToBuffer && arg.endsWith( "\"" ) ) + { + String cleanArgPart = arg.substring( 0, arg.length() - 1 ); + + // if we're building an argument, keep doing so. + if ( currentArg != null ) + { + // if this is the case of "-Dfoo=bar", then we need to adjust the buffer. + if ( addedToBuffer ) + { + currentArg.setLength( currentArg.length() - 1 ); + } + // otherwise, we trim the trailing " and append to the buffer. + else + { + // TODO: introducing a space here...not sure what else to do but collapse whitespace + currentArg.append( ' ' ).append( cleanArgPart ); + } + + cleaned.add( currentArg.toString() ); + } + else + { + cleaned.add( cleanArgPart ); + } + + currentArg = null; + addedToBuffer = false; + continue; + } + + // if we haven't added this arg to the buffer, and we ARE building an argument + // buffer, then append it with a preceding space...again, not sure what else to + // do other than collapse whitespace. + // NOTE: The case of a trailing quote is handled by nullifying the arg buffer. + if ( !addedToBuffer ) + { + if ( currentArg != null ) + { + currentArg.append( ' ' ).append( arg ); + } + else + { + cleaned.add( arg ); + } + } + } + + if ( currentArg != null ) + { + cleaned.add( currentArg.toString() ); + } + + int cleanedSz = cleaned.size(); + + String[] cleanArgs; + + if ( cleanedSz == 0 ) + { + cleanArgs = args; + } + else + { + cleanArgs = cleaned.toArray( new String[cleanedSz] ); + } + + return cleanArgs; + } + + +} http://git-wip-us.apache.org/repos/asf/maven/blob/8a51f9e5/maven-embedder/src/test/java/org/apache/maven/cli/CLIManagerDocumentationTest.java ---------------------------------------------------------------------- diff --git a/maven-embedder/src/test/java/org/apache/maven/cli/CLIManagerDocumentationTest.java b/maven-embedder/src/test/java/org/apache/maven/cli/CLIManagerDocumentationTest.java new file mode 100644 index 0000000..a06f0a1 --- /dev/null +++ b/maven-embedder/src/test/java/org/apache/maven/cli/CLIManagerDocumentationTest.java @@ -0,0 +1,107 @@ +package org.apache.maven.cli; + +/* + * 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. + */ + +import java.io.File; +import java.io.IOException; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.Comparator; +import java.util.List; + +import org.apache.commons.cli.Option; +import org.codehaus.plexus.PlexusTestCase; +import org.codehaus.plexus.util.FileUtils; + +/** + * Pseudo test to generate documentation fragment about supported CLI options. TODO such documentation generation code + * should not be necessary as unit test but should be run during site generation (Velocity? Doxia macro?) + */ +public class CLIManagerDocumentationTest + extends PlexusTestCase +{ + private final static String LS = System.getProperty( "line.separator" ); + + private static class OptionComparator + implements Comparator<Option> + { + public int compare( Option opt1, Option opt2 ) + { + return opt1.getOpt().compareToIgnoreCase( opt2.getOpt() ); + } + } + + private static class CLIManagerExtension + extends CLIManager + { + public Collection<Option> getOptions() + { + List<Option> optList = new ArrayList<>( options.getOptions() ); + Collections.sort( optList, new OptionComparator() ); + return optList; + } + } + + public String getOptionsAsHtml() + { + StringBuilder sb = new StringBuilder(); + boolean a = true; + sb.append( "<table border='1' class='zebra-striped'><tr class='a'><th><b>Options</b></th><th><b>Description</b></th></tr>" ); + for ( Option option : new CLIManagerExtension().getOptions() ) + { + a = !a; + sb.append( "<tr class='" ).append( a ? 'a' : 'b' ).append( "'><td><code>-<a name='" ); + sb.append( option.getOpt() ); + sb.append( "'>" ); + sb.append( option.getOpt() ); + sb.append( "</a>,--<a name='" ); + sb.append( option.getLongOpt() ); + sb.append( "'>" ); + sb.append( option.getLongOpt() ); + sb.append( "</a>" ); + if ( option.hasArg() ) + { + if ( option.hasArgName() ) + { + sb.append( " <" ).append( option.getArgName() ).append( ">" ); + } + else + { + sb.append( ' ' ); + } + } + sb.append( "</code></td><td>" ); + sb.append( option.getDescription() ); + sb.append( "</td></tr>" ); + sb.append( LS ); + } + sb.append( "</table>" ); + return sb.toString(); + } + + public void testOptionsAsHtml() + throws IOException + { + File options = getTestFile( "target/test-classes/options.html" ); + FileUtils.fileWrite( options, "UTF-8", getOptionsAsHtml() ); + } + +} http://git-wip-us.apache.org/repos/asf/maven/blob/8a51f9e5/maven-embedder/src/test/java/org/apache/maven/cli/CLIManagerTest.java ---------------------------------------------------------------------- diff --git a/maven-embedder/src/test/java/org/apache/maven/cli/CLIManagerTest.java b/maven-embedder/src/test/java/org/apache/maven/cli/CLIManagerTest.java deleted file mode 100644 index 1e71837..0000000 --- a/maven-embedder/src/test/java/org/apache/maven/cli/CLIManagerTest.java +++ /dev/null @@ -1,108 +0,0 @@ -package org.apache.maven.cli; - -/* - * 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. - */ - -import java.io.File; -import java.io.IOException; -import java.util.ArrayList; -import java.util.Collection; -import java.util.Collections; -import java.util.Comparator; -import java.util.List; - -import org.apache.commons.cli.Option; -import org.codehaus.plexus.PlexusTestCase; -import org.codehaus.plexus.util.FileUtils; - -/** - * Pseudo test to generate documentation fragment about supported CLI options. - * TODO such documentation generation code should not be necessary as unit test but should be run - * during site generation (Velocity? Doxia macro?) - */ -public class CLIManagerTest - extends PlexusTestCase -{ - private final static String LS = System.getProperty( "line.separator" ); - - private static class OptionComparator - implements Comparator<Option> - { - public int compare( Option opt1, Option opt2 ) - { - return opt1.getOpt().compareToIgnoreCase( opt2.getOpt() ); - } - } - - private static class CLIManagerExtension - extends CLIManager - { - public Collection<Option> getOptions() - { - @SuppressWarnings( "unchecked" ) - List<Option> optList = new ArrayList<>( options.getOptions() ); - Collections.sort( optList, new OptionComparator() ); - return optList; - } - } - - public String getOptionsAsHtml() - { - StringBuilder sb = new StringBuilder(); - boolean a = true; - sb.append( "<table border='1' class='zebra-striped'><tr class='a'><th><b>Options</b></th><th><b>Description</b></th></tr>" ); - for ( Option option : new CLIManagerExtension().getOptions() ) - { - a = !a; - sb.append( "<tr class='" ).append( a ? 'a' : 'b' ).append( "'><td><code>-<a name='" ); - sb.append( option.getOpt() ); - sb.append( "'>" ); - sb.append( option.getOpt() ); - sb.append( "</a>,--<a name='" ); - sb.append( option.getLongOpt() ); - sb.append( "'>" ); - sb.append( option.getLongOpt() ); - sb.append( "</a>" ); - if ( option.hasArg() ) - { - if ( option.hasArgName() ) - { - sb.append( " <" ).append( option.getArgName() ).append( ">" ); - } - else - { - sb.append( ' ' ); - } - } - sb.append( "</code></td><td>" ); - sb.append( option.getDescription() ); - sb.append( "</td></tr>" ); - sb.append( LS ); - } - sb.append( "</table>" ); - return sb.toString(); - } - - public void testOptionsAsHtml() - throws IOException - { - File options = getTestFile( "target/test-classes/options.html" ); - FileUtils.fileWrite( options, "UTF-8", getOptionsAsHtml() ); - } -} http://git-wip-us.apache.org/repos/asf/maven/blob/8a51f9e5/maven-embedder/src/test/java/org/apache/maven/cli/CleanArgumentTest.java ---------------------------------------------------------------------- diff --git a/maven-embedder/src/test/java/org/apache/maven/cli/CleanArgumentTest.java b/maven-embedder/src/test/java/org/apache/maven/cli/CleanArgumentTest.java new file mode 100644 index 0000000..874170e --- /dev/null +++ b/maven-embedder/src/test/java/org/apache/maven/cli/CleanArgumentTest.java @@ -0,0 +1,61 @@ +package org.apache.maven.cli; + +/* + * 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. + */ + +import static org.junit.Assert.assertEquals; + +import org.junit.Test; + +/** + * @author Karl Heinz Marbaise <khmarba...@apache.org> + */ +public class CleanArgumentTest +{ + @Test + public void cleanArgsShouldRemoveWrongSurroundingQuotes() + { + String[] args = { "\"-Dfoo=bar", "\"-Dfoo2=bar two\"" }; + String[] cleanArgs = CleanArgument.cleanArgs( args ); + assertEquals( args.length, cleanArgs.length ); + assertEquals( "-Dfoo=bar", cleanArgs[0] ); + assertEquals( "-Dfoo2=bar two", cleanArgs[1] ); + } + + @Test + public void testCleanArgsShouldNotTouchCorrectlyQuotedArgumentsUsingDoubleQuotes() + { + String information = "-Dinformation=\"The Information is important.\""; + String[] args = { information }; + String[] cleanArgs = CleanArgument.cleanArgs( args ); + assertEquals( args.length, cleanArgs.length ); + assertEquals( information, cleanArgs[0] ); + } + + @Test + public void testCleanArgsShouldNotTouchCorrectlyQuotedArgumentsUsingSingleQuotes() + { + String information = "-Dinformation='The Information is important.'"; + String[] args = { information }; + String[] cleanArgs = CleanArgument.cleanArgs( args ); + assertEquals( args.length, cleanArgs.length ); + assertEquals( information, cleanArgs[0] ); + } + +} http://git-wip-us.apache.org/repos/asf/maven/blob/8a51f9e5/pom.xml ---------------------------------------------------------------------- diff --git a/pom.xml b/pom.xml index a9a0c33..75cb34e 100644 --- a/pom.xml +++ b/pom.xml @@ -1,19 +1,19 @@ <?xml version="1.0" encoding="UTF-8"?> - <!-- - 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. - --> +<!-- + 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. +--> <project xmlns="http://maven.apache.org/POM/4.0.0"; xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd";> <modelVersion>4.0.0</modelVersion>
