[Pki-devel] [PATCH] Ticket #888 - TPS rewrite: provide remote authority functions (patch 1: TKS functions)

[Endi Sukma Dewata]

On 3/14/2014 1:42 PM, Christina Fu wrote:
> Thank you Endi and Jack for your comments.
> Attached please find the patch that addressed your comments.
>
> thanks,
> Christina

Just a few more comments:

1. The computeSessionKey() will throw an exception if it's missing the 
RESPONSE_STATUS. The other methods will just ignore that. I think they 
should be consistent.

2. Right now the code gets a string value from the response object, 
decodes the value into TPSBuffer, then puts it back to the same response 
object under the same name. Then the new Response classes would read 
from the same response object again. It works fine but code maintainer 
would have to be aware of the current type of the attribute because it 
could change.

   // response contains String or TPSBuffer values
   Hashtable<String, Object> response = parseResponse(content);

   value = (String) response.get(IRemoteRequest.TKS_RESPONSE_SessionKey);
   if (value == null) {
       CMS.debug(...);
   } else {
       CMS.debug(...);
       response.put(IRemoteRequest.TKS_RESPONSE_SessionKey,
           Util.specialDecode(value));
   }

   return new TKSComputeSessionKeyResponse(response);

It might be easier to simply wrap the original response object with the 
new Response class, then decode the value only when the attribute is used:

   // response contains string values
   Hashtable<String, String> response = parseResponse(content);
   return new TKSComputeSessionKeyResponse(response);

   public TPSBuffer getSessionKey() {
       String value =
           response.get(IRemoteRequest.TKS_RESPONSE_SessionKey);
       if (value == null) return null;
       // decode value only when needed
       return Util.specialDecode(value));
   }

Or alternatively the new Response class can hold just the decoded values 
either in a separate hashtable or in fields:

   Hashtable<String, String> response = parseResponse(content);
   TKSComputeSessionKeyResponse res =
       new TKSComputeSessionKeyResponse(); // don't use response

   value = response.get(IRemoteRequest.TKS_RESPONSE_SessionKey);
   if (value == null) {
       CMS.debug(...);
   } else {
       CMS.debug(...);
       // store decoded value
       res.setSessionKey(Util.specialDecode(value));
   }

   return res;

Issue #1 I think should be fixed. ACK once that is addressed.

For #2, it's not a problem, but it would improve code clarity. I'll let 
you decide. Thanks.

-- 
Endi S. Dewata