Re: [Pki-devel] PATCH 34-1 - Restful interface to create certificate requests
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