RFR[15] JDK-8244683: A TSA server used by tests

[sha.jiang at oracle.com]

On 2020/6/3 22:57, Weijun Wang wrote:
>>
>>> TsaParam.java:
>>>
>>> - Please group the fields into different area. Looks like some are fields in TSTInfo, and some are server internals.
>> One usage of this class is carrying all parameters delivered by HTTP
>> request. Do you mean they should be grouped in different classes?
> No. Just add some comment lines inside the class and separate them into different groups.
This class is also used for carrying TSA request fields, such as version,
digestAlgo and hashedMessage, and especially certReq is in request only.

>
>>> The overall design is good, but I have a feeling that the interface and implementation can be further separated. I can see that there are a lot of methods and classes that will not be touched by TimeStampCheck at all. Should these things be taken out to be another implementation which is parallel to the one in TimeStampCheck? Maybe TsaHandler::createSigner will be abstract.
>> All RespInterceptor methods are used by TimeStampCheck.
>> DefaultRespInterceptor is used by standalone TSA server. In this style, no
>> more RespInterceptor implementation is needed.
> See below.
>
>>> There are quite some public constructors in the classes. Are they meant to be called by a user? Or by a child class? Or just internal?
>> The public constructors in TsaServer can be called by applications for
>> making standalone servers.
> How about the other classes?
TsaSigner has two constructors.
One is for applications without new interceptor, the other is used by
TimeStampCheck which needs to provide a custom interceptor.

Please consider this TSA server will be used as standalone style,
namely, no new interceptor is needed. It's not introduced for some
specific test, like TimeStampCheck.

>>> Also, for those protected methods (Ex: parseRequestParam and createResponse in TsaSigner), do you meant to override it in the future?
>> Possibly.
>> For example, a sub parseRequestParam() may need to parse extensions from
>> request. A test may directly twist the byte[] returned by createResponse(),
>> or even an overridden createResponse() just returns an empty byte[].
> Can we make them internal at the moment until someone really need to extend it?
Yes, we can.
But as a test lib, is it bad to think about a bit further?

>>> Can you give examples on how to use this server? Will it always use DefaultRespInterceptor? If so, will the default impl in RespInterceptor.java ever be used?
>> I'll use this server as standalone-like service. The custom TSA fields
>> just be delivered by HTTP request query string. I won't provide a custom
>> RespInterceptor, and DefaultRespInterceptor is enough to me.
> You can put DefaultRespInterceptor in the standalone server.
An application just needs to new TsaServer() with a keystore and start it,
then a standalone server is ready. No more interceptor is needed. Otherwise,
every application may need to provide an interceptor like 
DefaultRespInterceptor.

> If you change TsaSigner.java
>
>    80     public TsaSigner(SignerEntry signerEntry, byte[] requestData,
>    81             TsaParam param) {
>    82         this(signerEntry, requestData,
>    83                 new DefaultRespInterceptor<TsaParam>(param));
>
> to use the even more "default" RespInterceptor, i.e. new RespInterceptor(){}, is it still useable?
The param is needed. Or, the TSA fields cannot be changed without
introducing a new interceptor.

Could I launch a standalone TSA server at first, and then run jarsigner
separately for manually checking some cases?

For example, jarsigner ... -tsa "http://host:port?version=2" ...
In the above command, the standalone TSA server will send a response
with version is 2.

Best regards,
John Jiang