From fee6e655f6aace10c651a0308295c6d2846ac741 Mon Sep 17 00:00:00 2001 From: Henry Jen Date: Mon, 25 Nov 2024 08:53:10 -0800 Subject: [PATCH 1/6] 8303884: jlink --add-options plugin does not allow GNU style options to be provided --- .../jdk/tools/jlink/internal/TaskHelper.java | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/TaskHelper.java b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/TaskHelper.java index 23b3dfb7079f5..67bcfd9670ad2 100644 --- a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/TaskHelper.java +++ b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/TaskHelper.java @@ -535,18 +535,22 @@ public List handleOptions(T task, String[] args) throws BadArgs { } Option opt = pluginOption == null ? option : pluginOption; String param = null; + boolean potentiallyGnuOption = false; if (opt.hasArg) { if (name.startsWith("--") && name.indexOf('=') > 0) { param = name.substring(name.indexOf('=') + 1, name.length()); } else if (i + 1 < args.length) { + potentiallyGnuOption = true; param = args[++i]; } - if (param == null || param.isEmpty() - || (param.length() >= 2 && param.charAt(0) == '-' - && param.charAt(1) == '-')) { - throw new BadArgs("err.missing.arg", name). - showUsage(true); + if (param == null || param.isEmpty()) { + throw new BadArgs("err.missing.arg", name).showUsage(true); + } + if (potentiallyGnuOption && param.length() >= 2 && + param.charAt(0) == '-' && param.charAt(1) == '-' && + !param.contains(" ")) { + throw new BadArgs("err.missing.arg", name).showUsage(true); } } if (pluginOption != null) { From 70b5f9f5bd929d895ce35e92738a5ec22a1d8bf4 Mon Sep 17 00:00:00 2001 From: Henry Jen Date: Tue, 3 Dec 2024 11:37:41 -0800 Subject: [PATCH 2/6] add test --- test/jdk/tools/jlink/TaskHelperTest.java | 148 +++++++++++++++++++++++ 1 file changed, 148 insertions(+) create mode 100644 test/jdk/tools/jlink/TaskHelperTest.java diff --git a/test/jdk/tools/jlink/TaskHelperTest.java b/test/jdk/tools/jlink/TaskHelperTest.java new file mode 100644 index 0000000000000..675dbe7adf519 --- /dev/null +++ b/test/jdk/tools/jlink/TaskHelperTest.java @@ -0,0 +1,148 @@ +/* + * Copyright (c) 2024, 2024, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +import java.io.IOException; +import java.util.*; + +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +import jdk.tools.jlink.internal.PluginRepository; +import jdk.tools.jlink.internal.TaskHelper; +import jdk.tools.jlink.internal.TaskHelper.Option; +import jdk.tools.jlink.internal.TaskHelper.OptionsHelper; +import jdk.tools.jlink.plugin.Plugin; +import jdk.tools.jlink.plugin.ResourcePool; +import jdk.tools.jlink.plugin.ResourcePoolBuilder; + +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; + +import jdk.tools.jlink.internal.TaskHelper.BadArgs; + +/* + * @test + * @summary Test TaskHelper option parsing + * @bug 8303884 + * @modules jdk.jlink/jdk.tools.jlink.internal + * jdk.jlink/jdk.tools.jlink.plugin + * @run junit TaskHelperTest + */ +public class TaskHelperTest { + private static TaskHelper taskHelper; + private static OptionsHelper optionsHelper; + + private static final List> OPTIONS = List.of( + new Option<>(true, (task, opt, arg) -> { + System.out.println(arg); + argValue = arg; + }, true, "--main-expecting"), + new Option<>(false, (task, opt, arg) -> { + }, true, "--main-no-arg") + ); + + private static String argValue; + + public static class TestPluginWithRawOption implements Plugin { + @Override + public ResourcePool transform(ResourcePool in, ResourcePoolBuilder out) { + return out.build(); + } + + @Override + public boolean hasArguments() { + return true; + } + + @Override + public boolean hasRawArgument() { + return true; + } + + @Override + public String getName() { + return "raw-arg-plugin"; + } + + @Override + public void configure(Map config) { + config.forEach((k, v) -> { + System.out.println(k + " -> " + v); + }); + var v = config.get(getName()); + if (v == null) + throw new AssertionError(); + argValue = v; + } + } + + @BeforeAll + public static void setup() { + taskHelper = new TaskHelper(TaskHelper.JLINK_BUNDLE); + optionsHelper = taskHelper.newOptionsHelper(TaskHelperTest.class, OPTIONS.toArray(Option[]::new)); + PluginRepository.registerPlugin(new TestPluginWithRawOption()); + } + + @Test + public void testGnuStyleOptionAsArgValue() throws TaskHelper.BadArgs { + var validFormats = new String[][] { + { "--main-expecting=--main-no-arg", "--main-no-arg" }, + { "--main-expecting", "--main-no-arg --list", "--main-no-arg"}, + { "--main-expecting", " --main-no-arg", "--main-no-arg" }, + { "--raw-arg-plugin=--main-no-arg", "--main-no-arg" }, + { "--raw-arg-plugin", "--main-no-arg --list", "--main-no-arg"}, + { "--raw-arg-plugin", " --main-no-arg", "--main-no-arg" }, + }; + + for (var args: validFormats) { + var remaining = optionsHelper.handleOptions(this, args); + try { + // trigger Plugin::configure + taskHelper.getPluginsConfig(null, null, null); + } catch (IOException ex) { + fail("Unexpected IOException"); + } + assertTrue(remaining.isEmpty()); + assertTrue(argValue.strip().startsWith("--main-no-arg")); + // reset + argValue = null; + } + } + + @Test + public void testGnuStyleOptionAsArgValueMissing() { + var validFormats = new String[][] { + { "--main-expecting", "--main-no-arg", "--main-no-arg"}, + { "--raw-arg-plugin", "--main-no-arg", "--main-no-arg"} + }; + + for (var args: validFormats) { + try { + optionsHelper.handleOptions(this, args); + fail("Should get missing argument value"); + } catch (BadArgs ex) { + // expected + } + } + } +} \ No newline at end of file From 7dbc99f99fd9cbd71f565db239ec8327363468c8 Mon Sep 17 00:00:00 2001 From: Henry Jen Date: Thu, 5 Dec 2024 09:03:48 -0800 Subject: [PATCH 3/6] Fix style --- .../share/classes/jdk/tools/jlink/internal/TaskHelper.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/TaskHelper.java b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/TaskHelper.java index 67bcfd9670ad2..c924f9e489b04 100644 --- a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/TaskHelper.java +++ b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/TaskHelper.java @@ -548,9 +548,9 @@ public List handleOptions(T task, String[] args) throws BadArgs { throw new BadArgs("err.missing.arg", name).showUsage(true); } if (potentiallyGnuOption && param.length() >= 2 && - param.charAt(0) == '-' && param.charAt(1) == '-' && - !param.contains(" ")) { - throw new BadArgs("err.missing.arg", name).showUsage(true); + param.charAt(0) == '-' && param.charAt(1) == '-' && + !param.contains(" ")) { + throw new BadArgs("err.missing.arg", name).showUsage(true); } } if (pluginOption != null) { From 1274931a515135da43dcb5e9f4cf1c55f92d1836 Mon Sep 17 00:00:00 2001 From: Henry Jen Date: Tue, 7 Jan 2025 15:57:20 -0800 Subject: [PATCH 4/6] Use opt=value format when value is starting with -- --- .../jdk/tools/jlink/internal/TaskHelper.java | 3 +- test/jdk/tools/jlink/TaskHelperTest.java | 130 ++++++++++++++---- 2 files changed, 102 insertions(+), 31 deletions(-) diff --git a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/TaskHelper.java b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/TaskHelper.java index c924f9e489b04..033a8636a1f86 100644 --- a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/TaskHelper.java +++ b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/TaskHelper.java @@ -548,8 +548,7 @@ public List handleOptions(T task, String[] args) throws BadArgs { throw new BadArgs("err.missing.arg", name).showUsage(true); } if (potentiallyGnuOption && param.length() >= 2 && - param.charAt(0) == '-' && param.charAt(1) == '-' && - !param.contains(" ")) { + param.charAt(0) == '-' && param.charAt(1) == '-') { throw new BadArgs("err.missing.arg", name).showUsage(true); } } diff --git a/test/jdk/tools/jlink/TaskHelperTest.java b/test/jdk/tools/jlink/TaskHelperTest.java index 675dbe7adf519..c343e16a55adc 100644 --- a/test/jdk/tools/jlink/TaskHelperTest.java +++ b/test/jdk/tools/jlink/TaskHelperTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2024, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2024, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -23,9 +23,13 @@ import java.io.IOException; import java.util.*; +import java.util.stream.Stream; import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; import jdk.tools.jlink.internal.PluginRepository; import jdk.tools.jlink.internal.TaskHelper; @@ -35,6 +39,7 @@ import jdk.tools.jlink.plugin.ResourcePool; import jdk.tools.jlink.plugin.ResourcePoolBuilder; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; @@ -55,13 +60,18 @@ public class TaskHelperTest { private static final List> OPTIONS = List.of( new Option<>(true, (task, opt, arg) -> { System.out.println(arg); - argValue = arg; + mainArgValue = arg; }, true, "--main-expecting"), new Option<>(false, (task, opt, arg) -> { + mainFlag = true; }, true, "--main-no-arg") ); private static String argValue; + private static String mainArgValue; + private static boolean mainFlag = false; + + public record ArgTestCase(String cmdLine, String[] tokens, String pluginArgValue, String mainArgValue, boolean mainFlagSet) {}; public static class TestPluginWithRawOption implements Plugin { @Override @@ -103,46 +113,108 @@ public static void setup() { PluginRepository.registerPlugin(new TestPluginWithRawOption()); } - @Test - public void testGnuStyleOptionAsArgValue() throws TaskHelper.BadArgs { - var validFormats = new String[][] { - { "--main-expecting=--main-no-arg", "--main-no-arg" }, - { "--main-expecting", "--main-no-arg --list", "--main-no-arg"}, - { "--main-expecting", " --main-no-arg", "--main-no-arg" }, - { "--raw-arg-plugin=--main-no-arg", "--main-no-arg" }, - { "--raw-arg-plugin", "--main-no-arg --list", "--main-no-arg"}, - { "--raw-arg-plugin", " --main-no-arg", "--main-no-arg" }, - }; + @BeforeEach + public void reset() { + argValue = null; + mainArgValue = null; + mainFlag = false; + } - for (var args: validFormats) { - var remaining = optionsHelper.handleOptions(this, args); - try { - // trigger Plugin::configure - taskHelper.getPluginsConfig(null, null, null); - } catch (IOException ex) { - fail("Unexpected IOException"); - } - assertTrue(remaining.isEmpty()); - assertTrue(argValue.strip().startsWith("--main-no-arg")); - // reset - argValue = null; + public static Stream gnuStyleUsages() { + return Stream.of( + new ArgTestCase( + "--main-expecting=--main-no-arg --main-no-arg", + new String[] { "--main-expecting=--main-no-arg", "--main-no-arg" }, + null, + "--main-no-arg", + true + ), + new ArgTestCase( + "--main-expecting ' --main-no-arg' --main-no-arg", + new String[] { "--main-expecting", " --main-no-arg", "--main-no-arg" }, + null, + " --main-no-arg", + true + ), + new ArgTestCase( + "--raw-arg-plugin=--main-no-arg --main-no-arg", + new String[] { "--raw-arg-plugin=--main-no-arg", "--main-no-arg" }, + "--main-no-arg", + null, + true + ), + new ArgTestCase( + "--raw-arg-plugin ' --main-no-arg' --main-no-arg", + new String[] { "--raw-arg-plugin", " --main-no-arg", "--main-no-arg" }, + " --main-no-arg", + null, + true + ), + new ArgTestCase( + "--raw-arg-plugin=--main-expecting=value --main-no-arg", + new String[] { "--raw-arg-plugin=--main-expecting=value", "--main-no-arg" }, + "--main-expecting=value", + null, + true + ), + new ArgTestCase( + "--raw-arg-plugin='--main-expecting value' --main-no-arg", + new String[] { "--raw-arg-plugin=--main-expecting value", "--main-no-arg" }, + "--main-expecting value", + null, + true + ), + new ArgTestCase( + "--raw-arg-plugin='--main-expecting value' --main-expecting realValue", + new String[] { "--raw-arg-plugin=--main-expecting value", "--main-expecting", "realValue" }, + "--main-expecting value", + "realValue", + false + )); + } + + @ParameterizedTest + @MethodSource("gnuStyleUsages") + public void testGnuStyleOptionAsArgValue(ArgTestCase testCase) throws TaskHelper.BadArgs { + System.out.println("Test cmdline: " + testCase.cmdLine()); + var args = testCase.tokens(); + var remaining = optionsHelper.handleOptions(this, args); + try { + // trigger Plugin::configure + taskHelper.getPluginsConfig(null, null, null); + } catch (IOException ex) { + fail("Unexpected IOException"); } + assertTrue(remaining.isEmpty()); + assertEquals(testCase.mainFlagSet(), mainFlag); + assertEquals(testCase.pluginArgValue(), argValue); + assertEquals(testCase.mainArgValue(), mainArgValue); } @Test public void testGnuStyleOptionAsArgValueMissing() { - var validFormats = new String[][] { - { "--main-expecting", "--main-no-arg", "--main-no-arg"}, - { "--raw-arg-plugin", "--main-no-arg", "--main-no-arg"} + var invalidFormat = new String[][] { + { "--main-expecting", "--main-no-arg --list", "--main-no-arg" }, + { "--main-expecting", "--main-no-arg", "--main-no-arg" }, + { "--raw-arg-plugin", "--main-no-arg --list", "--main-no-arg" }, + { "--raw-arg-plugin", "--main-no-arg", "--main-no-arg" }, + { "--raw-arg-plugin", "--main-expecting", "value", "--main-no-arg" } }; - for (var args: validFormats) { + for (var args: invalidFormat) { try { optionsHelper.handleOptions(this, args); - fail("Should get missing argument value"); + fail("Should get missing argument value or "); } catch (BadArgs ex) { // expected } } } + + @Test + public void testRemaining() throws BadArgs { + String[] args = { "--raw-arg-plugin=--main-expecting", "value", "--main-no-arg" }; + var remaining = optionsHelper.handleOptions(this, args); + assertEquals(2, remaining.size()); + } } \ No newline at end of file From 76ce4472cd9f3b7ebf265faed2db2abccee5e5c3 Mon Sep 17 00:00:00 2001 From: Henry Jen Date: Mon, 13 Jan 2025 10:16:29 -0800 Subject: [PATCH 5/6] Use different error message for ambiguous gnu-style options as value --- .../share/classes/jdk/tools/jlink/internal/TaskHelper.java | 2 +- .../share/classes/jdk/tools/jlink/resources/jlink.properties | 1 + test/jdk/tools/jlink/TaskHelperTest.java | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/TaskHelper.java b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/TaskHelper.java index 033a8636a1f86..539d21957c1be 100644 --- a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/TaskHelper.java +++ b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/TaskHelper.java @@ -549,7 +549,7 @@ public List handleOptions(T task, String[] args) throws BadArgs { } if (potentiallyGnuOption && param.length() >= 2 && param.charAt(0) == '-' && param.charAt(1) == '-') { - throw new BadArgs("err.missing.arg", name).showUsage(true); + throw new BadArgs("err.ambiguous.arg", name).showUsage(false); } } if (pluginOption != null) { diff --git a/src/jdk.jlink/share/classes/jdk/tools/jlink/resources/jlink.properties b/src/jdk.jlink/share/classes/jdk/tools/jlink/resources/jlink.properties index a491b758ea0ee..bafcb179e1b67 100644 --- a/src/jdk.jlink/share/classes/jdk/tools/jlink/resources/jlink.properties +++ b/src/jdk.jlink/share/classes/jdk/tools/jlink/resources/jlink.properties @@ -147,6 +147,7 @@ err.dir.exists={0} already exists err.badpattern=bad pattern {0} err.unknown.option=unknown option: {0} err.missing.arg=no value given for {0} +err.ambiguous.arg=value for option {0} starts with \"--\" should use {0}= format err.internal.error=internal error: {0} {1} {2} err.invalid.arg.for.option={0} does not accept \"{1}\" argument err.option.after.class=option must be specified before classes: {0} diff --git a/test/jdk/tools/jlink/TaskHelperTest.java b/test/jdk/tools/jlink/TaskHelperTest.java index c343e16a55adc..51dea8de24a9a 100644 --- a/test/jdk/tools/jlink/TaskHelperTest.java +++ b/test/jdk/tools/jlink/TaskHelperTest.java @@ -204,7 +204,7 @@ public void testGnuStyleOptionAsArgValueMissing() { for (var args: invalidFormat) { try { optionsHelper.handleOptions(this, args); - fail("Should get missing argument value or "); + fail("Should get an ambiguous error"); } catch (BadArgs ex) { // expected } From 50a8c5ee9d101572d06e14a79c95781ac2627661 Mon Sep 17 00:00:00 2001 From: Henry Jen Date: Tue, 14 Jan 2025 08:52:24 -0800 Subject: [PATCH 6/6] Update copyright year --- .../share/classes/jdk/tools/jlink/internal/TaskHelper.java | 2 +- .../share/classes/jdk/tools/jlink/resources/jlink.properties | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/TaskHelper.java b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/TaskHelper.java index 539d21957c1be..689c8b24743e9 100644 --- a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/TaskHelper.java +++ b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/TaskHelper.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2015, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it diff --git a/src/jdk.jlink/share/classes/jdk/tools/jlink/resources/jlink.properties b/src/jdk.jlink/share/classes/jdk/tools/jlink/resources/jlink.properties index bafcb179e1b67..080d51506a646 100644 --- a/src/jdk.jlink/share/classes/jdk/tools/jlink/resources/jlink.properties +++ b/src/jdk.jlink/share/classes/jdk/tools/jlink/resources/jlink.properties @@ -1,5 +1,5 @@ # -# Copyright (c) 2015, 2024, Oracle and/or its affiliates. All rights reserved. +# Copyright (c) 2015, 2025, Oracle and/or its affiliates. All rights reserved. # DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. # # This code is free software; you can redistribute it and/or modify it