[Pki-devel] PATCH 34-1 - Restful interface to create certificate requests

[Endi Sukma Dewata]

On 6/26/2012 3:47 PM, Ade Lee wrote:
> Please review.
>
> Tests done:
> 1. Cert issuance and approval from UI (manual, maul dual cert, agent
> authenticated server cert)
> 2. Cert issuance in installation of other subsystems
> 3. Cert issuance (user and server) from RA.
> 4. Cert issuance and approval from restful interface using CATest
> -- two step (create cert request/ agent approval) user and server certs
> -- single step (agent approved user and server certs)

Some comments, more to follow. Some of these might have been discussed 
previously.

1. Some fields in the ProfileProcessor are initialized to null in the 
field declaration. This is not necessary because it's null by default.

2. The code in ProfileProcessor.java:1450 might fail because statEvents 
could still be null if an error happens before the first startTiming() 
is called. It might be better to initialize statEvents in the field 
declaration instead of in startTiming(). Alternatively, it's possible to 
track the timing without statEvents using a separate try-finally block 
for each startTiming() & and endTiming() pair:

   startTiming("enrollment");
   try {
       ...
       startTiming("request_population");
       try {
           ...
       } finally {
           endTiming("request_population");
       }
       ...
   } finally {
       endTiming("enrollment");
   }

3. Not sure if this is a problem. In ProfileProcessor.java:721 the 
errorCode is initialized to null before the loop. Inside the loop the 
errorCode is set when there's an exception but never reset to null 
again. So if one request fails and the next one succeeds it will still 
have the errorCode of the previous failure. The errorCode determines 
whether to call markAsServiced() or updateRequest(). The original 
servlet has the same behavior.

4. The serial_num in ProfileProcessor.java:412 seems to be used for 
renewal only. It might be better to move it into processRenewal().

5. In ProfileProcessor the ps.getProfile() is called multiple times to 
get the profile. Usually it's using the profileId specified in the 
request (line 415) but it can be overriden by the configuration (lines 
460 & 555). However, the profile inputs are always obtained from the 
profile specified in the request (line 417). So it's possible the inputs 
won't match the profile. It might be better to check the configuration 
when creating the EnrollmentRequestData then subsequent code will use 
the one specified in it.

-- 
Endi S. Dewata