directory-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Emmanuel Lecharny <elecha...@gmail.com>
Subject About the Cert generation Extended request
Date Mon, 05 Jan 2009 13:23:28 GMT
Hi Kiran,

you will be the first person able to add a new Extended request using 
the ASN.1 codec !

Ok, it's really good. Just a few things
- Where do you got the ASN.1 grammar from ? Is there a reference 
somwhere on the web, or is this just soemthing you defined from scratch ?

- There are some minor issues in the grammar class : when all the fields 
are mandatory (ie, the OPTIONAL keyword does not appears), then you 
should insure that the TLVs are not empty. Typically, if the sequence is 
empty, you will get a PDU containing 0x30 0x00. Using your current 
implementation, this will be accepted.
The problem is that your first transition contains this line :

                    CertGenContainer.grammarEndAllowed( true );
which allow the PDU to e terminated immediately.

In this case, you should just ommit this line, unless you want the 
grammar to allow empty sequences. Here, I think that the only transition 
where this line should appear is the last one : keyAlgorithm.

You should also check that if all the fields are not present, then it 
generates an error (ie, adding a test for each bad field).

- You are storing DN as String, but maybe it would be a better iead to 
check that those DNs are valid. You can use the LdapDN.isValid( dn ) to 
do so. Or you can store LdapDN instead. It's up to you.

- In tests, when expecting an exception, don't add a e.printStackTrace() 
: it's a burden when running integration tests. Also don't use '*' in 
imports.

I just commited some fixes.

Otherwise, it's pretty clean. I didn't thought someone could understand 
this portion of the code :) Thanks !

-- 
--
cordialement, regards,
Emmanuel L├ęcharny
www.iktek.com
directory.apache.org



Mime
View raw message