[ 
https://issues.apache.org/jira/browse/CLI-345?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Ruiqi Dong updated CLI-345:
---------------------------
    Issue Type: Improvement  (was: Bug)

> OptionBuilder.create(String) does not validate short option length
> ------------------------------------------------------------------
>
>                 Key: CLI-345
>                 URL: https://issues.apache.org/jira/browse/CLI-345
>             Project: Commons CLI
>          Issue Type: Improvement
>          Components: Options definition
>    Affects Versions: 1.10.0
>            Reporter: Ruiqi Dong
>            Priority: Minor
>             Fix For: 1.10.0
>
>         Attachments: OptionBuilder.java, OptionBuilderTest.java
>
>   Original Estimate: 0.5h
>  Remaining Estimate: 0.5h
>
> The {{OptionBuilder.create(String opt)}} method does not validate that the 
> provided string is a single character, which violates the command-line 
> convention that short options should be single characters (e.g., {{{}-a{}}}, 
> {{{}-b{}}}). This allows creation of invalid multi-character short options 
> like {{-ab}} or {{{}-abc{}}}.
>  
> *Steps to Reproduce:*
> // This should throw IllegalArgumentException but doesn't
> Option opt = OptionBuilder.withDescription("test option").create("ab");
>  
> *Expected Behavior:* According to command-line conventions and API 
> consistency, short options should be single characters only. 
> {{OptionBuilder.create(String)}} should throw {{IllegalArgumentException}} 
> when the string length is not 1. This would be consistent with 
> {{OptionBuilder.create(char)}} which naturally enforces single character.
> *Actual Behavior:* The method accepts multi-character strings without 
> validation, allowing creation of non-standard short options.
>  
> *Test Case:*
> @Test
> voidtestCreateWithMultiCharacterShortOptionShouldThrowException(){
> // This test reveals the bug: multi-character short options should not be 
> allowed
>  
> // Test with 2-character string
> assertThrows(IllegalArgumentException.class, () -> {
> OptionBuilder.withDescription("test option")
> .create("ab");
> }, "Creating short option with 2 characters should throw 
> IllegalArgumentException");
>  
> // Test with 3-character string
> assertThrows(IllegalArgumentException.class, () -> {
> OptionBuilder.withDescription("test option")
> .create("abc");
> }, "Creating short option with 3 characters should throw 
> IllegalArgumentException");
>  
> // Test with empty string
> assertThrows(IllegalArgumentException.class, () -> {
> OptionBuilder.withDescription("test option")
> .create("");
> }, "Creating short option with empty string should throw 
> IllegalArgumentException");
>  
> // This should work - single character
> assertDoesNotThrow(() -> {
> Optionopt = OptionBuilder.withDescription("valid option")
> .create("a");
> assertEquals("a", opt.getOpt());
> }, "Creating short option with single character should not throw exception");
>  
> // This should also work - using char overload
> assertDoesNotThrow(() -> {
> Optionopt = OptionBuilder.withDescription("valid option")
> .create('b');
> assertEquals("b", opt.getOpt());
> }, "Creating short option with char should not throw exception");
> }
>  
> *Test Results:*
> [*INFO*] Running org.apache.commons.cli.{*}OptionBuilderTest{*}{*}{*}
> [*ERROR*] *Tests* {*}run: 1{*}, *Failures: 1*, Errors: 0, Skipped: 0, Time 
> elapsed: 0.001 s *<<< FAILURE!* -- in 
> org.apache.commons.cli.{*}OptionBuilderTest{*}{*}{*}
> [*ERROR*] 
> org.apache.commons.cli.OptionBuilderTest.testCreateWithMultiCharacterShortOptionShouldThrowException
>  -- Time elapsed: 0.001 s <<< FAILURE!
> org.opentest4j.AssertionFailedError: Creating short option with 2 characters 
> should throw IllegalArgumentException ==> Expected 
> java.lang.IllegalArgumentException to be thrown, but nothing was thrown.
>  at 
> org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:152)
>  at org.junit.jupiter.api.AssertThrows.assertThrows(AssertThrows.java:73)
>  at org.junit.jupiter.api.AssertThrows.assertThrows(AssertThrows.java:39)
>  at org.junit.jupiter.api.Assertions.assertThrows(Assertions.java:3153)
>  at 
> org.apache.commons.cli.OptionBuilderTest.testCreateWithMultiCharacterShortOptionShouldThrowException(OptionBuilderTest.java:30)
>  at java.lang.reflect.Method.invoke(Method.java:498)
>  at java.util.ArrayList.forEach(ArrayList.java:1259)
>  at java.util.ArrayList.forEach(ArrayList.java:1259)
>  
> *Suggested Fix:*
> Add validation in {{{}OptionBuilder.create(String opt){}}}:
> public static Option create(final String opt) throws IllegalArgumentException 
> {
>     if (opt != null && opt.length() != 1) {
>         throw new IllegalArgumentException("Short option must be a single 
> character");
>     }
>     // ... rest of the method
> }



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to