RFR 8172975: SecurityTools.keytool() needs to accept user input

[Weijun Wang]

On 01/19/2017 09:40 PM, Artem Smotrakov wrote:
> Hi Max,
>
> In general, looks okay.
>
> Would it be better if it called redirectInput() only if the response
> file exists? keytool() method might also delete the response file after
> reading it. These two measures may prevent situations when the response
> file is unnecessary used.

Yes, it's good to remove the file after reading it, so this means people 
need to set it for every call.

On the other hand, I still think creating an empty file and call 
redirectInput() on it is necessary when the response file does not 
exist. This prevents the keytool() call from hanging forever if it 
actually needs user input but the test has not provided any.

Also, I am feeling that the jarsigner-related calls are quite 
complicated. I suggest we use the same method for signing and verifying 
and ask the user to provide all arguments in their order. So instead of 
calling

    SecurityTools.jarsigner("x.jar", "alias", "...");

we just call

    SecurityTools.jarsigner("... x.jar alias");

This is consistent with keytool(), and we can also provide 3 overloaded 
forms.

What do you think? :-)

Thanks
Max

>
> What do you think?
>
> Artem
>
>
> On 01/18/2017 05:50 PM, Weijun Wang wrote:
>> Please review the code changes at
>>
>>    root: http://cr.openjdk.java.net/~weijun/8172975/root/webrev.00/
>>    jdk: http://cr.openjdk.java.net/~weijun/8172975/webrev.00/
>>
>> The fix is in root repo. This is not an elegant solution because it
>> uses a separate method to provide the response. This means you have to
>> clear the response if the next keytool call does not need it. This
>> also means you cannot run keytool in multiple threads.
>>
>> I didn't provide the response as an extra argument because there are
>> already many overloaded keytool() methods, and I do not want to invent
>> a new method name (say, keytoolWithResponse) and implement the same
>> number of overloaded methods.
>>
>> If you are strongly against this solution, I'll think of another way.
>>
>> The jdk change includes a test for this change, as well as a trivial
>> fix for keytool's getYesNoReply() method. Otherwise an NPE is thrown
>> with the following command:
>>
>>    cat untrusted.cert | keytool -importcert -alias a
>>
>> Thanks
>> Max
>