You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@directory.apache.org by Emmanuel Lecharny <el...@gmail.com> on 2009/01/05 14:23:28 UTC
About the Cert generation Extended request
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
Re: About the Cert generation Extended request
Posted by ayyagarikiran <ay...@gmail.com>.
hi Emmanuel,
Emmanuel Lecharny wrote:
> 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 ?
yep, created on my own referring the existing code and the wiki doc
>
> - 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 );
ah ok, I didn't get this part but assumed this as a way to tell the decoder to continue to the next transition, will fix it
> 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).
+1
>
> - 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.
>
+1, think this makes more sense and I can throw error as early as possible
> - 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.
>
ah, my bad, will take them out ( was lazy to add each method of Assert to the static imports ;) )
> I just commited some fixes.
ah great, thanks
>
> Otherwise, it's pretty clean. I didn't thought someone could understand
> this portion of the code :) Thanks !
>
Thanks a lot for your time and comments, I appreciate them :)
Kiran Ayyagari