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

Weijun Wang weijun.wang at oracle.com
Wed Jun 3 14:57:30 UTC 2020



> On Jun 3, 2020, at 10:14 PM, sha.jiang at oracle.com wrote:
> 
> Hi Max,
> 
> On 2020/6/3 17:17, Weijun Wang wrote:
>> TimeStampCheck.java:
>> 
>> - Can you please inline all those private getXyz() calls in Interceptor into getRespParam()? I would like to see all these customizations in one place. Just add a blank line in between and it will be clean enough for me.
> OK
> 
>> 
>> 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.

> 
>> 
>> 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?

> 
>> 
>> 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?

> 
>> 
>> 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.

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?

Thanks,
Max

> 
> Best regards,
> John Jiang
> 




More information about the security-dev mailing list