[Pki-devel] <pki-devel> [PATCH] 0004- Further work on TPS Processor, format operation.patch

[Christina Fu]

I find it easier to just apply the patches to the tree to have a better 
context.  Some of the things I've found may be from the past.

* TPSEngine -
   - you have defined a number TKS_RESPONSE_xxx constants which I have 
defined in IRemoteRequest.java.  I think IRemoteRequest.java is a better 
place since I have TKS server side using those constants as well.  If 
agreed, how about removing those 6 constants from TPSEngine?  I checked 
and i see in your code you are not using them yet anyway, so it should 
not be too much trouble.

* TPSFormatProcessor
  - In our old tps, I have always questioned the existance of the 
RA_Format_Processor as well as the other xxx_Processor's other than the 
RA_Processor.cpp and  RA_Enroll_Processor.cpp where the real code were in.
   My question is, is there going to be an attempt of putting real 
format code into the TPSFormatProcessor, etc, or we are merely going to 
do a direct conversion of C++ to Java?  The reason why I ask is because 
the TPSFormatProcessor and all probably serve not much purpose unless we 
put something useful in them.  We can probably leave them as is for now 
and think about it later.  I just want to bring it up so we can keep it 
in mind to ponder on.

* Would it make sense for APDUResponse to carry a boolean status so that 
checkTokenPDUResponse() only needs to be processed once and status set 
at the time?  Anyway, if there is concern of veering off to far from the 
old tps course then we can keep the return rc style.

* I don' t see in the format() where you pass in the skipAuth value.

* I may have missed it, but I don't see you checking if cplc_data < 47 
(a magic number in old tps).

* why do you change the variable name token_status to status?  I think 
token_status, or if you wish, tokenStatus, is more descriptive.

* why do you check if (session == null) in the middle of assigning major 
and minor versions?  Could you not have checked this earlier, like near 
the beginning of format()?

* the old tps might be wrong, but I just want to know why is it that it 
would assign 0 to all those versions if token_status == NULL, while you 
bumped it out?

* perhaps we want to consider keeping the tokenType somewhere we can 
retrieve (not re-calculated and re-retrieved) easily during a session, 
instead of passing it around as a parameter.  Where is a good place to 
do that?

that's it for now.
Christina

On 03/13/2014 04:29 PM, John Magne wrote:
> Ticket #895 https://fedorahosted.org/pki/ticket/895
>
>   Further work on TPS Processor, format operation.
>
> This patch gets a bit farther on the TPS format operation, just before applet upgrade, which will also need secure channel functionality.
>
> Also, patch provides some misc clean up and functionality.
>
> 1. Method to calculate the token type.
> 2. Some added convenience methods to get various config params for the Format operation.
> 3. More progress for the format operation up until we attempt to upgrade the applet.
> 4. Added TPSException that holds a message and end op return code. Can be used to throw from anywhere and the return code makes it back to the client.
>
>
> ---
>
>
> _______________________________________________
> Pki-devel mailing list
> Pki-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/pki-devel

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/pki-devel/attachments/20140318/45555c6b/attachment.htm>