farkhalit commented on PR #432:
URL: https://github.com/apache/commons-cli/pull/432#issuecomment-5031190015
Because resolving a class name currently executes code from that class.
`Converter.CLASS` is `Class::forName`, the one-arg overload, which
initializes the class. So any `Class`-typed option runs the static initializer
of whatever name shows up in argv, during parsing, before the caller has even
looked at the returned `Class`.
Repro against master:
```java
public class Evil {
static { System.out.println("STATIC INITIALIZER RAN"); }
public Evil() {}
}
public class Repro {
public static void main(String[] a) throws Exception {
Options o = PatternOptionBuilder.parsePattern("c+");
CommandLine cl = new DefaultParser().parse(o, new String[] {"-c",
"Evil"});
System.out.println("before getParsedOptionValue");
Class<?> k = cl.getParsedOptionValue("c");
System.out.println("got class object: " + k.getName());
}
}
```
`javac -cp target/classes -d . Evil.java Repro.java && java -cp
.:target/classes Repro` prints `before getParsedOptionValue`, then `STATIC
INITIALIZER RAN`, then `got class object: Evil`. Same path via
`TypeHandler.createClass`.
With the three-arg overload and `initialize` false, the initializer line is
gone and the `Class` object is still returned; initialization happens on first
real use instead. Same defining class loader, same `ClassNotFoundException`
contract. `Converter.OBJECT` is unchanged since
`getConstructor().newInstance()` initializes anyway.
Happy to drop it if you consider running argv-named static initializers at
parse time acceptable, but it looked wrong to me.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
