You are viewing a plain text version of this content. The canonical link for it is here.
Posted to kerby@directory.apache.org by "Zheng, Kai" <ka...@intel.com> on 2015/12/24 01:31:17 UTC
RE: [kerby] ASN1 Quick review
Thanks Emmanuel for the review and great comments! The questions are hard but fortunately I'm still kept in the loop so my pleasure to address them. My comments are embedded and marked by [Kai].
-----Original Message-----
From: Emmanuel Lécharny [mailto:elecharny@gmail.com]
Sent: Wednesday, December 23, 2015 8:54 PM
To: Apache Directory Developers List <de...@directory.apache.org>
Subject: [kerby] ASN1 Quick review
Hi band,
I'm having a quick look at the kerby-asn1 code, as I wanted to paly around the idea of porting the LDAP codec to use this piece of nice code. AFAIU, when you want to declare an object that can be encoded or decoded, you have to extend the correct Asn1Object child. Looking at the Ticket object, here is what I see :
[Kai] Glad you want a try. This is also something I wish to help with as discussed before, but am not able to do it immediately because of bandwidth. Sorry for that. I certainly would and will also be able to try to provide any help if needed.
...
import static org.apache.kerby.kerberos.kerb.type.ticket.Ticket.MyEnum.*;
...
public class Ticket extends KrbAppSequenceType {
public static final int TKT_KVNO = KrbConstant.KRB_V5;
public static final int TAG = 1;
protected enum MyEnum implements EnumType {
TKT_VNO,
REALM,
SNAME,
ENC_PART;
@Override
public int getValue() {
return ordinal();
}
@Override
public String getName() {
return name();
}
}
static Asn1FieldInfo[] fieldInfos = new Asn1FieldInfo[] {
new ExplicitField(TKT_VNO, 0, Asn1Integer.class),
new ExplicitField(REALM, 1, KerberosString.class),
new ExplicitField(SNAME, 2, PrincipalName.class),
new ExplicitField(ENC_PART, 3, EncryptedData.class)
};
I like the idea of defining the fields this way, except that I would suggest some slight modifications :
- get read of the import ...Ticket.MyEnum.*;
[Kai] Hmmm, I guess you mean "get rid of", good point. It's like this because, initially we didn't use enum, but int constants. And some weeks ago when we want to dump user defined type objects like this with meaningful field info, we switched to use enum because it can give a friendly name. We were in a hurry at that time and wanted to do it as less effort as possible, thus it's like this now: the enum constant is rather like int constant and used as before.
- make the enum private (there is no reason we wuld like to expose it anyway)
[Kai] Well, actually there are some reasons to make it protected and disciplined exposed. Such user defined types can be extended and these fields may be accessed there. Ref. child classes of ContentInfo (also some other examples I remembered when defining protected int constants).
- name it something more meaningful, like TicketField inseatd if MyEnum
[Kai] Right agree. Again it was like this because it's out as a simple pattern in a whole project scope replacement.
- use it with the enum name, like in new ExplicitField(TicketField.TKT_VNO, 0, Asn1Integer.class)
[Kai] Agree.
<side note>
I find the "import static xxx.*;" atrocious, most of the time. It makes it barely readable : you have to go to the top of your file to actually
*know* where the constant comes from... That may seems of for small files, but otherwise... I accept it for Asserts, because it's really clear, in a test context - pretty much like annotations.
</side note>
[Kai] I thought you may be right, but ...
Here is what I come with :
public class Ticket extends KrbAppSequenceType {
public static final int TKT_KVNO = KrbConstant.KRB_V5;
public static final int TAG = 1;
private enum TicketField implements EnumType {
TKT_VNO,
REALM,
SNAME,
ENC_PART;
@Override
public int getValue() {
return ordinal();
}
@Override
public String getName() {
return name();
}
}
static Asn1FieldInfo[] fieldInfos = new Asn1FieldInfo[] {
new ExplicitField(TicketField.TKT_VNO, 0, Asn1Integer.class),
new ExplicitField(TicketField.REALM, 1, KerberosString.class),
new ExplicitField(TicketField.SNAME, 2, PrincipalName.class),
new ExplicitField(TicketField.ENC_PART, 3, EncryptedData.class)
};
(note that it's just a suggestion at this point...)
[Kai] They're very good suggestions and should become true, though a tiresome work because there are so many user defined types now. I'm wondering if any contributor would help with such.
Now, let's foduc on the fields declarations. Here, we create ExplicitFields, passing a Xxx.class as the third parameter, and a number as the second parameter.
First of all, as the enum being used has an implicit ordering, can't we simply avoid passing those numbers ? There is already an ExplicitField constructor that takes only 2 parameters, deducing the number from teh enum...
[Kai] Yeah sure we can just use the 2 parameters constructor for this type. You're good at finding this as the example. We relied on the enum order value mostly but this one and possible many slipped out.
Second, why don't we write things like :
new ExplicitField(TicketField.TKT_VNO, 0, new Asn1Integer()),
instead of passing a class ? Bottom line, the instance will be created the same way. I don't think it will make any difference in term of performances, and as all the object *must* extends the Asn1Object (or implement the Asn1Type), this should be good enough.
[Kai] Well, let the framework determine when to create the field instances would be most flexible. We may want to be lazy on creating them and create them on demand; we may need to avoid creating them in decoding because they're nulls; in the framework codes there're some places that uses (value == null) to check some conditions. For Any, the actual value type can only be set by applications in specific contexts.
wdyt ?
RE: [kerby] ASN1 Quick review
Posted by "Zheng, Kai" <ka...@intel.com>.
Resend after formatted.
Thanks Emmanuel for the review and great comments! The questions are hard but fortunately I'm still kept in the loop so my pleasure to address them. My comments are embedded and marked by [Kai].
-----Original Message-----
From: Emmanuel Lécharny [mailto:elecharny@gmail.com]
Sent: Wednesday, December 23, 2015 8:54 PM
To: Apache Directory Developers List <de...@directory.apache.org>
Subject: [kerby] ASN1 Quick review
Hi band,
I'm having a quick look at the kerby-asn1 code, as I wanted to paly around the idea of porting the LDAP codec to use this piece of nice code. AFAIU, when you want to declare an object that can be encoded or decoded, you have to extend the correct Asn1Object child. Looking at the Ticket object, here is what I see :
[Kai] Glad you want a try. This is also something I wish to help with as discussed before, but am not able to do it immediately because of bandwidth. Sorry for that. I certainly would and will also be able to try to provide any help if needed.
...
import static org.apache.kerby.kerberos.kerb.type.ticket.Ticket.MyEnum.*;
...
public class Ticket extends KrbAppSequenceType {
public static final int TKT_KVNO = KrbConstant.KRB_V5;
public static final int TAG = 1;
protected enum MyEnum implements EnumType {
TKT_VNO,
REALM,
SNAME,
ENC_PART;
@Override
public int getValue() {
return ordinal();
}
@Override
public String getName() {
return name();
}
}
static Asn1FieldInfo[] fieldInfos = new Asn1FieldInfo[] {
new ExplicitField(TKT_VNO, 0, Asn1Integer.class),
new ExplicitField(REALM, 1, KerberosString.class),
new ExplicitField(SNAME, 2, PrincipalName.class),
new ExplicitField(ENC_PART, 3, EncryptedData.class)
};
I like the idea of defining the fields this way, except that I would suggest some slight modifications :
- get read of the import ...Ticket.MyEnum.*;
[Kai] Hmmm, I guess you mean "get rid of", good point. It's like this because, initially we didn't use enum, but int constants. And some weeks ago when we want to dump user defined type objects like this with meaningful field info, we switched to use enum because it can give a friendly name. We were in a hurry at that time and wanted to do it as less effort as possible, thus it's like this now: the enum constant is rather like int constant and used as before.
- make the enum private (there is no reason we wuld like to expose it anyway)
[Kai] Well, actually there are some reasons to make it protected and disciplined exposed. Such user defined types can be extended and these fields may be accessed there. Ref. child classes of ContentInfo (also some other examples I remembered when defining protected int constants).
- name it something more meaningful, like TicketField inseatd if MyEnum
[Kai] Right agree. Again it was like this because it's out as a simple pattern in a whole project scope replacement.
- use it with the enum name, like in new ExplicitField(TicketField.TKT_VNO, 0, Asn1Integer.class)
[Kai] Agree.
<side note>
I find the "import static xxx.*;" atrocious, most of the time. It makes it barely readable : you have to go to the top of your file to actually
*know* where the constant comes from... That may seems of for small files, but otherwise... I accept it for Asserts, because it's really clear, in a test context - pretty much like annotations.
</side note>
[Kai] I thought you may be right, but ...
Here is what I come with :
public class Ticket extends KrbAppSequenceType {
public static final int TKT_KVNO = KrbConstant.KRB_V5;
public static final int TAG = 1;
private enum TicketField implements EnumType {
TKT_VNO,
REALM,
SNAME,
ENC_PART;
@Override
public int getValue() {
return ordinal();
}
@Override
public String getName() {
return name();
}
}
static Asn1FieldInfo[] fieldInfos = new Asn1FieldInfo[] {
new ExplicitField(TicketField.TKT_VNO, 0, Asn1Integer.class),
new ExplicitField(TicketField.REALM, 1, KerberosString.class),
new ExplicitField(TicketField.SNAME, 2, PrincipalName.class),
new ExplicitField(TicketField.ENC_PART, 3, EncryptedData.class)
};
(note that it's just a suggestion at this point...)
[Kai] They're very good suggestions and should become true, though a tiresome work because there are so many user defined types now. I'm wondering if any contributor would help with such.
Now, let's foduc on the fields declarations. Here, we create ExplicitFields, passing a Xxx.class as the third parameter, and a number as the second parameter.
First of all, as the enum being used has an implicit ordering, can't we simply avoid passing those numbers ? There is already an ExplicitField constructor that takes only 2 parameters, deducing the number from teh enum...
[Kai] Yeah sure we can just use the 2 parameters constructor for this type. You're good at finding this as the example. We relied on the enum order value mostly but this one and possible many slipped out.
Second, why don't we write things like :
new ExplicitField(TicketField.TKT_VNO, 0, new Asn1Integer()),
instead of passing a class ? Bottom line, the instance will be created the same way. I don't think it will make any difference in term of performances, and as all the object *must* extends the Asn1Object (or implement the Asn1Type), this should be good enough.
[Kai] Well, let the framework determine when to create the field instances would be most flexible. We may want to be lazy on creating them and create them on demand; we may need to avoid creating them in decoding because they're nulls; in the framework codes there're some places that uses (value == null) to check some conditions. For Any, the actual value type can only be set by applications in specific contexts.
wdyt ?
RE: [kerby] ASN1 Quick review
Posted by "Zheng, Kai" <ka...@intel.com>.
Thanks Yan for the hard taking! Note it means you would need to manually modify 200+ classes one by one since we'll need a friendly enum name.
Regards,
Kai
-----Original Message-----
From: Yan, Yan A [mailto:yan.a.yan@intel.com]
Sent: Thursday, December 24, 2015 9:13 AM
To: kerby@directory.apache.org; Apache Directory Developers List <de...@directory.apache.org>
Subject: RE: [kerby] ASN1 Quick review
Thanks for Emmanuel and Kai's discussion.
I'm wondering if any contributor would help with such.
I'd like to help with the issues, and by the way be familiar with the core part codes.
Best regards,
Yan
-----Original Message-----
From: Zheng, Kai [mailto:kai.zheng@intel.com]
Sent: Thursday, December 24, 2015 8:31 AM
To: Apache Directory Developers List <de...@directory.apache.org>; kerby@directory.apache.org
Subject: RE: [kerby] ASN1 Quick review
Thanks Emmanuel for the review and great comments! The questions are hard but fortunately I'm still kept in the loop so my pleasure to address them. My comments are embedded and marked by [Kai].
-----Original Message-----
From: Emmanuel Lécharny [mailto:elecharny@gmail.com]
Sent: Wednesday, December 23, 2015 8:54 PM
To: Apache Directory Developers List <de...@directory.apache.org>
Subject: [kerby] ASN1 Quick review
Hi band,
I'm having a quick look at the kerby-asn1 code, as I wanted to paly around the idea of porting the LDAP codec to use this piece of nice code. AFAIU, when you want to declare an object that can be encoded or decoded, you have to extend the correct Asn1Object child. Looking at the Ticket object, here is what I see :
[Kai] Glad you want a try. This is also something I wish to help with as discussed before, but am not able to do it immediately because of bandwidth. Sorry for that. I certainly would and will also be able to try to provide any help if needed.
...
import static org.apache.kerby.kerberos.kerb.type.ticket.Ticket.MyEnum.*;
...
public class Ticket extends KrbAppSequenceType {
public static final int TKT_KVNO = KrbConstant.KRB_V5;
public static final int TAG = 1;
protected enum MyEnum implements EnumType {
TKT_VNO,
REALM,
SNAME,
ENC_PART;
@Override
public int getValue() {
return ordinal();
}
@Override
public String getName() {
return name();
}
}
static Asn1FieldInfo[] fieldInfos = new Asn1FieldInfo[] {
new ExplicitField(TKT_VNO, 0, Asn1Integer.class),
new ExplicitField(REALM, 1, KerberosString.class),
new ExplicitField(SNAME, 2, PrincipalName.class),
new ExplicitField(ENC_PART, 3, EncryptedData.class)
};
I like the idea of defining the fields this way, except that I would suggest some slight modifications :
- get read of the import ...Ticket.MyEnum.*; [Kai] Hmmm, I guess you mean "get rid of", good point. It's like this because, initially we didn't use enum, but int constants. And some weeks ago when we want to dump user defined type objects like this with meaningful field info, we switched to use enum because it can give a friendly name. We were in a hurry at that time and wanted to do it as less effort as possible, thus it's like this now: the enum constant is rather like int constant and used as before.
- make the enum private (there is no reason we wuld like to expose it anyway) [Kai] Well, actually there are some reasons to make it protected and disciplined exposed. Such user defined types can be extended and these fields may be accessed there. Ref. child classes of ContentInfo (also some other examples I remembered when defining protected int constants).
- name it something more meaningful, like TicketField inseatd if MyEnum [Kai] Right agree. Again it was like this because it's out as a simple pattern in a whole project scope replacement.
- use it with the enum name, like in new ExplicitField(TicketField.TKT_VNO, 0, Asn1Integer.class) [Kai] Agree.
<side note>
I find the "import static xxx.*;" atrocious, most of the time. It makes it barely readable : you have to go to the top of your file to actually
*know* where the constant comes from... That may seems of for small files, but otherwise... I accept it for Asserts, because it's really clear, in a test context - pretty much like annotations.
</side note>
[Kai] I thought you may be right, but ...
Here is what I come with :
public class Ticket extends KrbAppSequenceType {
public static final int TKT_KVNO = KrbConstant.KRB_V5;
public static final int TAG = 1;
private enum TicketField implements EnumType {
TKT_VNO,
REALM,
SNAME,
ENC_PART;
@Override
public int getValue() {
return ordinal();
}
@Override
public String getName() {
return name();
}
}
static Asn1FieldInfo[] fieldInfos = new Asn1FieldInfo[] {
new ExplicitField(TicketField.TKT_VNO, 0, Asn1Integer.class),
new ExplicitField(TicketField.REALM, 1, KerberosString.class),
new ExplicitField(TicketField.SNAME, 2, PrincipalName.class),
new ExplicitField(TicketField.ENC_PART, 3, EncryptedData.class)
};
(note that it's just a suggestion at this point...) [Kai] They're very good suggestions and should become true, though a tiresome work because there are so many user defined types now. I'm wondering if any contributor would help with such.
Now, let's foduc on the fields declarations. Here, we create ExplicitFields, passing a Xxx.class as the third parameter, and a number as the second parameter.
First of all, as the enum being used has an implicit ordering, can't we simply avoid passing those numbers ? There is already an ExplicitField constructor that takes only 2 parameters, deducing the number from teh enum...
[Kai] Yeah sure we can just use the 2 parameters constructor for this type. You're good at finding this as the example. We relied on the enum order value mostly but this one and possible many slipped out.
Second, why don't we write things like :
new ExplicitField(TicketField.TKT_VNO, 0, new Asn1Integer()),
instead of passing a class ? Bottom line, the instance will be created the same way. I don't think it will make any difference in term of performances, and as all the object *must* extends the Asn1Object (or implement the Asn1Type), this should be good enough.
[Kai] Well, let the framework determine when to create the field instances would be most flexible. We may want to be lazy on creating them and create them on demand; we may need to avoid creating them in decoding because they're nulls; in the framework codes there're some places that uses (value == null) to check some conditions. For Any, the actual value type can only be set by applications in specific contexts.
wdyt ?
RE: [kerby] ASN1 Quick review
Posted by "Zheng, Kai" <ka...@intel.com>.
Thanks Yan for the hard taking! Note it means you would need to manually modify 200+ classes one by one since we'll need a friendly enum name.
Regards,
Kai
-----Original Message-----
From: Yan, Yan A [mailto:yan.a.yan@intel.com]
Sent: Thursday, December 24, 2015 9:13 AM
To: kerby@directory.apache.org; Apache Directory Developers List <de...@directory.apache.org>
Subject: RE: [kerby] ASN1 Quick review
Thanks for Emmanuel and Kai's discussion.
I'm wondering if any contributor would help with such.
I'd like to help with the issues, and by the way be familiar with the core part codes.
Best regards,
Yan
-----Original Message-----
From: Zheng, Kai [mailto:kai.zheng@intel.com]
Sent: Thursday, December 24, 2015 8:31 AM
To: Apache Directory Developers List <de...@directory.apache.org>; kerby@directory.apache.org
Subject: RE: [kerby] ASN1 Quick review
Thanks Emmanuel for the review and great comments! The questions are hard but fortunately I'm still kept in the loop so my pleasure to address them. My comments are embedded and marked by [Kai].
-----Original Message-----
From: Emmanuel Lécharny [mailto:elecharny@gmail.com]
Sent: Wednesday, December 23, 2015 8:54 PM
To: Apache Directory Developers List <de...@directory.apache.org>
Subject: [kerby] ASN1 Quick review
Hi band,
I'm having a quick look at the kerby-asn1 code, as I wanted to paly around the idea of porting the LDAP codec to use this piece of nice code. AFAIU, when you want to declare an object that can be encoded or decoded, you have to extend the correct Asn1Object child. Looking at the Ticket object, here is what I see :
[Kai] Glad you want a try. This is also something I wish to help with as discussed before, but am not able to do it immediately because of bandwidth. Sorry for that. I certainly would and will also be able to try to provide any help if needed.
...
import static org.apache.kerby.kerberos.kerb.type.ticket.Ticket.MyEnum.*;
...
public class Ticket extends KrbAppSequenceType {
public static final int TKT_KVNO = KrbConstant.KRB_V5;
public static final int TAG = 1;
protected enum MyEnum implements EnumType {
TKT_VNO,
REALM,
SNAME,
ENC_PART;
@Override
public int getValue() {
return ordinal();
}
@Override
public String getName() {
return name();
}
}
static Asn1FieldInfo[] fieldInfos = new Asn1FieldInfo[] {
new ExplicitField(TKT_VNO, 0, Asn1Integer.class),
new ExplicitField(REALM, 1, KerberosString.class),
new ExplicitField(SNAME, 2, PrincipalName.class),
new ExplicitField(ENC_PART, 3, EncryptedData.class)
};
I like the idea of defining the fields this way, except that I would suggest some slight modifications :
- get read of the import ...Ticket.MyEnum.*; [Kai] Hmmm, I guess you mean "get rid of", good point. It's like this because, initially we didn't use enum, but int constants. And some weeks ago when we want to dump user defined type objects like this with meaningful field info, we switched to use enum because it can give a friendly name. We were in a hurry at that time and wanted to do it as less effort as possible, thus it's like this now: the enum constant is rather like int constant and used as before.
- make the enum private (there is no reason we wuld like to expose it anyway) [Kai] Well, actually there are some reasons to make it protected and disciplined exposed. Such user defined types can be extended and these fields may be accessed there. Ref. child classes of ContentInfo (also some other examples I remembered when defining protected int constants).
- name it something more meaningful, like TicketField inseatd if MyEnum [Kai] Right agree. Again it was like this because it's out as a simple pattern in a whole project scope replacement.
- use it with the enum name, like in new ExplicitField(TicketField.TKT_VNO, 0, Asn1Integer.class) [Kai] Agree.
<side note>
I find the "import static xxx.*;" atrocious, most of the time. It makes it barely readable : you have to go to the top of your file to actually
*know* where the constant comes from... That may seems of for small files, but otherwise... I accept it for Asserts, because it's really clear, in a test context - pretty much like annotations.
</side note>
[Kai] I thought you may be right, but ...
Here is what I come with :
public class Ticket extends KrbAppSequenceType {
public static final int TKT_KVNO = KrbConstant.KRB_V5;
public static final int TAG = 1;
private enum TicketField implements EnumType {
TKT_VNO,
REALM,
SNAME,
ENC_PART;
@Override
public int getValue() {
return ordinal();
}
@Override
public String getName() {
return name();
}
}
static Asn1FieldInfo[] fieldInfos = new Asn1FieldInfo[] {
new ExplicitField(TicketField.TKT_VNO, 0, Asn1Integer.class),
new ExplicitField(TicketField.REALM, 1, KerberosString.class),
new ExplicitField(TicketField.SNAME, 2, PrincipalName.class),
new ExplicitField(TicketField.ENC_PART, 3, EncryptedData.class)
};
(note that it's just a suggestion at this point...) [Kai] They're very good suggestions and should become true, though a tiresome work because there are so many user defined types now. I'm wondering if any contributor would help with such.
Now, let's foduc on the fields declarations. Here, we create ExplicitFields, passing a Xxx.class as the third parameter, and a number as the second parameter.
First of all, as the enum being used has an implicit ordering, can't we simply avoid passing those numbers ? There is already an ExplicitField constructor that takes only 2 parameters, deducing the number from teh enum...
[Kai] Yeah sure we can just use the 2 parameters constructor for this type. You're good at finding this as the example. We relied on the enum order value mostly but this one and possible many slipped out.
Second, why don't we write things like :
new ExplicitField(TicketField.TKT_VNO, 0, new Asn1Integer()),
instead of passing a class ? Bottom line, the instance will be created the same way. I don't think it will make any difference in term of performances, and as all the object *must* extends the Asn1Object (or implement the Asn1Type), this should be good enough.
[Kai] Well, let the framework determine when to create the field instances would be most flexible. We may want to be lazy on creating them and create them on demand; we may need to avoid creating them in decoding because they're nulls; in the framework codes there're some places that uses (value == null) to check some conditions. For Any, the actual value type can only be set by applications in specific contexts.
wdyt ?
RE: [kerby] ASN1 Quick review
Posted by "Yan, Yan A" <ya...@intel.com>.
Thanks for Emmanuel and Kai's discussion.
I'm wondering if any contributor would help with such.
I'd like to help with the issues, and by the way be familiar with the core part codes.
Best regards,
Yan
-----Original Message-----
From: Zheng, Kai [mailto:kai.zheng@intel.com]
Sent: Thursday, December 24, 2015 8:31 AM
To: Apache Directory Developers List <de...@directory.apache.org>; kerby@directory.apache.org
Subject: RE: [kerby] ASN1 Quick review
Thanks Emmanuel for the review and great comments! The questions are hard but fortunately I'm still kept in the loop so my pleasure to address them. My comments are embedded and marked by [Kai].
-----Original Message-----
From: Emmanuel Lécharny [mailto:elecharny@gmail.com]
Sent: Wednesday, December 23, 2015 8:54 PM
To: Apache Directory Developers List <de...@directory.apache.org>
Subject: [kerby] ASN1 Quick review
Hi band,
I'm having a quick look at the kerby-asn1 code, as I wanted to paly around the idea of porting the LDAP codec to use this piece of nice code. AFAIU, when you want to declare an object that can be encoded or decoded, you have to extend the correct Asn1Object child. Looking at the Ticket object, here is what I see :
[Kai] Glad you want a try. This is also something I wish to help with as discussed before, but am not able to do it immediately because of bandwidth. Sorry for that. I certainly would and will also be able to try to provide any help if needed.
...
import static org.apache.kerby.kerberos.kerb.type.ticket.Ticket.MyEnum.*;
...
public class Ticket extends KrbAppSequenceType {
public static final int TKT_KVNO = KrbConstant.KRB_V5;
public static final int TAG = 1;
protected enum MyEnum implements EnumType {
TKT_VNO,
REALM,
SNAME,
ENC_PART;
@Override
public int getValue() {
return ordinal();
}
@Override
public String getName() {
return name();
}
}
static Asn1FieldInfo[] fieldInfos = new Asn1FieldInfo[] {
new ExplicitField(TKT_VNO, 0, Asn1Integer.class),
new ExplicitField(REALM, 1, KerberosString.class),
new ExplicitField(SNAME, 2, PrincipalName.class),
new ExplicitField(ENC_PART, 3, EncryptedData.class)
};
I like the idea of defining the fields this way, except that I would suggest some slight modifications :
- get read of the import ...Ticket.MyEnum.*; [Kai] Hmmm, I guess you mean "get rid of", good point. It's like this because, initially we didn't use enum, but int constants. And some weeks ago when we want to dump user defined type objects like this with meaningful field info, we switched to use enum because it can give a friendly name. We were in a hurry at that time and wanted to do it as less effort as possible, thus it's like this now: the enum constant is rather like int constant and used as before.
- make the enum private (there is no reason we wuld like to expose it anyway) [Kai] Well, actually there are some reasons to make it protected and disciplined exposed. Such user defined types can be extended and these fields may be accessed there. Ref. child classes of ContentInfo (also some other examples I remembered when defining protected int constants).
- name it something more meaningful, like TicketField inseatd if MyEnum [Kai] Right agree. Again it was like this because it's out as a simple pattern in a whole project scope replacement.
- use it with the enum name, like in new ExplicitField(TicketField.TKT_VNO, 0, Asn1Integer.class) [Kai] Agree.
<side note>
I find the "import static xxx.*;" atrocious, most of the time. It makes it barely readable : you have to go to the top of your file to actually
*know* where the constant comes from... That may seems of for small files, but otherwise... I accept it for Asserts, because it's really clear, in a test context - pretty much like annotations.
</side note>
[Kai] I thought you may be right, but ...
Here is what I come with :
public class Ticket extends KrbAppSequenceType {
public static final int TKT_KVNO = KrbConstant.KRB_V5;
public static final int TAG = 1;
private enum TicketField implements EnumType {
TKT_VNO,
REALM,
SNAME,
ENC_PART;
@Override
public int getValue() {
return ordinal();
}
@Override
public String getName() {
return name();
}
}
static Asn1FieldInfo[] fieldInfos = new Asn1FieldInfo[] {
new ExplicitField(TicketField.TKT_VNO, 0, Asn1Integer.class),
new ExplicitField(TicketField.REALM, 1, KerberosString.class),
new ExplicitField(TicketField.SNAME, 2, PrincipalName.class),
new ExplicitField(TicketField.ENC_PART, 3, EncryptedData.class)
};
(note that it's just a suggestion at this point...) [Kai] They're very good suggestions and should become true, though a tiresome work because there are so many user defined types now. I'm wondering if any contributor would help with such.
Now, let's foduc on the fields declarations. Here, we create ExplicitFields, passing a Xxx.class as the third parameter, and a number as the second parameter.
First of all, as the enum being used has an implicit ordering, can't we simply avoid passing those numbers ? There is already an ExplicitField constructor that takes only 2 parameters, deducing the number from teh enum...
[Kai] Yeah sure we can just use the 2 parameters constructor for this type. You're good at finding this as the example. We relied on the enum order value mostly but this one and possible many slipped out.
Second, why don't we write things like :
new ExplicitField(TicketField.TKT_VNO, 0, new Asn1Integer()),
instead of passing a class ? Bottom line, the instance will be created the same way. I don't think it will make any difference in term of performances, and as all the object *must* extends the Asn1Object (or implement the Asn1Type), this should be good enough.
[Kai] Well, let the framework determine when to create the field instances would be most flexible. We may want to be lazy on creating them and create them on demand; we may need to avoid creating them in decoding because they're nulls; in the framework codes there're some places that uses (value == null) to check some conditions. For Any, the actual value type can only be set by applications in specific contexts.
wdyt ?
RE: [kerby] ASN1 Quick review
Posted by "Yan, Yan A" <ya...@intel.com>.
Thanks for Emmanuel and Kai's discussion.
I'm wondering if any contributor would help with such.
I'd like to help with the issues, and by the way be familiar with the core part codes.
Best regards,
Yan
-----Original Message-----
From: Zheng, Kai [mailto:kai.zheng@intel.com]
Sent: Thursday, December 24, 2015 8:31 AM
To: Apache Directory Developers List <de...@directory.apache.org>; kerby@directory.apache.org
Subject: RE: [kerby] ASN1 Quick review
Thanks Emmanuel for the review and great comments! The questions are hard but fortunately I'm still kept in the loop so my pleasure to address them. My comments are embedded and marked by [Kai].
-----Original Message-----
From: Emmanuel Lécharny [mailto:elecharny@gmail.com]
Sent: Wednesday, December 23, 2015 8:54 PM
To: Apache Directory Developers List <de...@directory.apache.org>
Subject: [kerby] ASN1 Quick review
Hi band,
I'm having a quick look at the kerby-asn1 code, as I wanted to paly around the idea of porting the LDAP codec to use this piece of nice code. AFAIU, when you want to declare an object that can be encoded or decoded, you have to extend the correct Asn1Object child. Looking at the Ticket object, here is what I see :
[Kai] Glad you want a try. This is also something I wish to help with as discussed before, but am not able to do it immediately because of bandwidth. Sorry for that. I certainly would and will also be able to try to provide any help if needed.
...
import static org.apache.kerby.kerberos.kerb.type.ticket.Ticket.MyEnum.*;
...
public class Ticket extends KrbAppSequenceType {
public static final int TKT_KVNO = KrbConstant.KRB_V5;
public static final int TAG = 1;
protected enum MyEnum implements EnumType {
TKT_VNO,
REALM,
SNAME,
ENC_PART;
@Override
public int getValue() {
return ordinal();
}
@Override
public String getName() {
return name();
}
}
static Asn1FieldInfo[] fieldInfos = new Asn1FieldInfo[] {
new ExplicitField(TKT_VNO, 0, Asn1Integer.class),
new ExplicitField(REALM, 1, KerberosString.class),
new ExplicitField(SNAME, 2, PrincipalName.class),
new ExplicitField(ENC_PART, 3, EncryptedData.class)
};
I like the idea of defining the fields this way, except that I would suggest some slight modifications :
- get read of the import ...Ticket.MyEnum.*; [Kai] Hmmm, I guess you mean "get rid of", good point. It's like this because, initially we didn't use enum, but int constants. And some weeks ago when we want to dump user defined type objects like this with meaningful field info, we switched to use enum because it can give a friendly name. We were in a hurry at that time and wanted to do it as less effort as possible, thus it's like this now: the enum constant is rather like int constant and used as before.
- make the enum private (there is no reason we wuld like to expose it anyway) [Kai] Well, actually there are some reasons to make it protected and disciplined exposed. Such user defined types can be extended and these fields may be accessed there. Ref. child classes of ContentInfo (also some other examples I remembered when defining protected int constants).
- name it something more meaningful, like TicketField inseatd if MyEnum [Kai] Right agree. Again it was like this because it's out as a simple pattern in a whole project scope replacement.
- use it with the enum name, like in new ExplicitField(TicketField.TKT_VNO, 0, Asn1Integer.class) [Kai] Agree.
<side note>
I find the "import static xxx.*;" atrocious, most of the time. It makes it barely readable : you have to go to the top of your file to actually
*know* where the constant comes from... That may seems of for small files, but otherwise... I accept it for Asserts, because it's really clear, in a test context - pretty much like annotations.
</side note>
[Kai] I thought you may be right, but ...
Here is what I come with :
public class Ticket extends KrbAppSequenceType {
public static final int TKT_KVNO = KrbConstant.KRB_V5;
public static final int TAG = 1;
private enum TicketField implements EnumType {
TKT_VNO,
REALM,
SNAME,
ENC_PART;
@Override
public int getValue() {
return ordinal();
}
@Override
public String getName() {
return name();
}
}
static Asn1FieldInfo[] fieldInfos = new Asn1FieldInfo[] {
new ExplicitField(TicketField.TKT_VNO, 0, Asn1Integer.class),
new ExplicitField(TicketField.REALM, 1, KerberosString.class),
new ExplicitField(TicketField.SNAME, 2, PrincipalName.class),
new ExplicitField(TicketField.ENC_PART, 3, EncryptedData.class)
};
(note that it's just a suggestion at this point...) [Kai] They're very good suggestions and should become true, though a tiresome work because there are so many user defined types now. I'm wondering if any contributor would help with such.
Now, let's foduc on the fields declarations. Here, we create ExplicitFields, passing a Xxx.class as the third parameter, and a number as the second parameter.
First of all, as the enum being used has an implicit ordering, can't we simply avoid passing those numbers ? There is already an ExplicitField constructor that takes only 2 parameters, deducing the number from teh enum...
[Kai] Yeah sure we can just use the 2 parameters constructor for this type. You're good at finding this as the example. We relied on the enum order value mostly but this one and possible many slipped out.
Second, why don't we write things like :
new ExplicitField(TicketField.TKT_VNO, 0, new Asn1Integer()),
instead of passing a class ? Bottom line, the instance will be created the same way. I don't think it will make any difference in term of performances, and as all the object *must* extends the Asn1Object (or implement the Asn1Type), this should be good enough.
[Kai] Well, let the framework determine when to create the field instances would be most flexible. We may want to be lazy on creating them and create them on demand; we may need to avoid creating them in decoding because they're nulls; in the framework codes there're some places that uses (value == null) to check some conditions. For Any, the actual value type can only be set by applications in specific contexts.
wdyt ?
RE: [kerby] ASN1 Quick review
Posted by "Zheng, Kai" <ka...@intel.com>.
Yeah, I can see that it's a good try. Sometime when I work on the compiler or more accurately, code generator, I can learn about the ASN1 parser and borrow some idea. Thanks!
-----Original Message-----
From: Emmanuel Lécharny [mailto:elecharny@gmail.com]
Sent: Thursday, December 24, 2015 9:37 PM
To: kerby@directory.apache.org
Subject: Re: [kerby] ASN1 Quick review
Le 24/12/15 14:14, Zheng, Kai a écrit :
> Ah it happens so fast and thank you so much. I wish we could have a compiler that accepts the ASN1 module definition files and then help generate/update these classes. It's one thing I want to develop but had better focus on our Kerberos things first. Thanks again.
I have started that years ago. Never had time to go very far :
http://svn.apache.org/repos/asf/labs/dungeon/
>
> I noted our new contributor Yan said she would help with the boring task. Maybe the left can be for her and you take a good rest? She needs to be familiar with such codes anyway.
Your call !
i'll be out anyway for the day, so feel free to task her ;-)
Re: [kerby] ASN1 Quick review
Posted by Emmanuel Lécharny <el...@gmail.com>.
Le 24/12/15 14:14, Zheng, Kai a écrit :
> Ah it happens so fast and thank you so much. I wish we could have a compiler that accepts the ASN1 module definition files and then help generate/update these classes. It's one thing I want to develop but had better focus on our Kerberos things first. Thanks again.
I have started that years ago. Never had time to go very far :
http://svn.apache.org/repos/asf/labs/dungeon/
>
> I noted our new contributor Yan said she would help with the boring task. Maybe the left can be for her and you take a good rest? She needs to be familiar with such codes anyway.
Your call !
i'll be out anyway for the day, so feel free to task her ;-)
RE: [kerby] ASN1 Quick review
Posted by "Zheng, Kai" <ka...@intel.com>.
Ah it happens so fast and thank you so much. I wish we could have a compiler that accepts the ASN1 module definition files and then help generate/update these classes. It's one thing I want to develop but had better focus on our Kerberos things first. Thanks again.
I noted our new contributor Yan said she would help with the boring task. Maybe the left can be for her and you take a good rest? She needs to be familiar with such codes anyway.
Regards,
Kai
-----Original Message-----
From: Emmanuel Lécharny [mailto:elecharny@gmail.com]
Sent: Thursday, December 24, 2015 8:52 PM
To: kerby@directory.apache.org
Subject: Re: [kerby] ASN1 Quick review
Le 24/12/15 10:10, Emmanuel Lécharny a écrit :
> Le 24/12/15 01:31, Zheng, Kai a écrit :
> [Kai] They're very good suggestions and should become true, though a tiresome work because there are so many user defined types now. I'm wondering if any contributor would help with such.
> I can do that. This is typically the kind of (boring) task that I like
> to do when I have little brain cycles (like when it's late, or when I
> don't have energy to focus on serious code : doing boring things make
> me feel I'm still working and contributing to the community).
Just committed a change for almost half of the classes. Piece of cake, took me less then an hour...
The other half is all in the pkix project.
Re: [kerby] ASN1 Quick review
Posted by Emmanuel Lécharny <el...@gmail.com>.
Le 24/12/15 10:10, Emmanuel Lécharny a écrit :
> Le 24/12/15 01:31, Zheng, Kai a écrit :
> [Kai] They're very good suggestions and should become true, though a tiresome work because there are so many user defined types now. I'm wondering if any contributor would help with such.
> I can do that. This is typically the kind of (boring) task that I like
> to do when I have little brain cycles (like when it's late, or when I
> don't have energy to focus on serious code : doing boring things make me
> feel I'm still working and contributing to the community).
Just committed a change for almost half of the classes. Piece of cake,
took me less then an hour...
The other half is all in the pkix project.
Re: [kerby] ASN1 Quick review
Posted by Emmanuel Lécharny <el...@gmail.com>.
Le 24/12/15 01:31, Zheng, Kai a écrit :
> Thanks Emmanuel for the review and great comments! The questions are hard but fortunately I'm still kept in the loop so my pleasure to address them. My comments are embedded and marked by [Kai].
>
> -----Original Message-----
> From: Emmanuel Lécharny [mailto:elecharny@gmail.com]
> Sent: Wednesday, December 23, 2015 8:54 PM
> To: Apache Directory Developers List <de...@directory.apache.org>
> Subject: [kerby] ASN1 Quick review
>
> Hi band,
>
> I'm having a quick look at the kerby-asn1 code, as I wanted to paly around the idea of porting the LDAP codec to use this piece of nice code. AFAIU, when you want to declare an object that can be encoded or decoded, you have to extend the correct Asn1Object child. Looking at the Ticket object, here is what I see :
>
> [Kai] Glad you want a try. This is also something I wish to help with as discussed before, but am not able to do it immediately because of bandwidth. Sorry for that. I certainly would and will also be able to try to provide any help if needed.
>
> ...
> import static org.apache.kerby.kerberos.kerb.type.ticket.Ticket.MyEnum.*;
> ...
> public class Ticket extends KrbAppSequenceType {
> public static final int TKT_KVNO = KrbConstant.KRB_V5;
> public static final int TAG = 1;
>
> protected enum MyEnum implements EnumType {
> TKT_VNO,
> REALM,
> SNAME,
> ENC_PART;
>
> @Override
> public int getValue() {
> return ordinal();
> }
>
> @Override
> public String getName() {
> return name();
> }
> }
>
> static Asn1FieldInfo[] fieldInfos = new Asn1FieldInfo[] {
> new ExplicitField(TKT_VNO, 0, Asn1Integer.class),
> new ExplicitField(REALM, 1, KerberosString.class),
> new ExplicitField(SNAME, 2, PrincipalName.class),
> new ExplicitField(ENC_PART, 3, EncryptedData.class)
> };
>
> I like the idea of defining the fields this way, except that I would suggest some slight modifications :
>
> - get read of the import ...Ticket.MyEnum.*;
> [Kai] Hmmm, I guess you mean "get rid of", good point. It's like this because, initially we didn't use enum, but int constants. And some weeks ago when we want to dump user defined type objects like this with meaningful field info, we switched to use enum because it can give a friendly name. We were in a hurry at that time and wanted to do it as less effort as possible, thus it's like this now: the enum constant is rather like int constant and used as before.
Okie, so I guess you don't have any provision against the suggested
modification.
>
> - make the enum private (there is no reason we wuld like to expose it anyway)
> [Kai] Well, actually there are some reasons to make it protected and disciplined exposed. Such user defined types can be extended and these fields may be accessed there. Ref. child classes of ContentInfo (also some other examples I remembered when defining protected int constants).
Okie for 'protected'.
>
> - name it something more meaningful, like TicketField inseatd if MyEnum
> [Kai] Right agree. Again it was like this because it's out as a simple pattern in a whole project scope replacement.
Okie. A rename is then possible.
>
> - use it with the enum name, like in new ExplicitField(TicketField.TKT_VNO, 0, Asn1Integer.class)
> [Kai] Agree.
>
> <side note>
> I find the "import static xxx.*;" atrocious, most of the time. It makes it barely readable : you have to go to the top of your file to actually
> *know* where the constant comes from... That may seems of for small files, but otherwise... I accept it for Asserts, because it's really clear, in a test context - pretty much like annotations.
> </side note>
> [Kai] I thought you may be right, but ...
>
> Here is what I come with :
>
> public class Ticket extends KrbAppSequenceType {
> public static final int TKT_KVNO = KrbConstant.KRB_V5;
> public static final int TAG = 1;
>
> private enum TicketField implements EnumType {
> TKT_VNO,
> REALM,
> SNAME,
> ENC_PART;
>
> @Override
> public int getValue() {
> return ordinal();
> }
>
> @Override
> public String getName() {
> return name();
> }
> }
>
> static Asn1FieldInfo[] fieldInfos = new Asn1FieldInfo[] {
> new ExplicitField(TicketField.TKT_VNO, 0, Asn1Integer.class),
> new ExplicitField(TicketField.REALM, 1, KerberosString.class),
> new ExplicitField(TicketField.SNAME, 2, PrincipalName.class),
> new ExplicitField(TicketField.ENC_PART, 3, EncryptedData.class)
> };
>
> (note that it's just a suggestion at this point...)
> [Kai] They're very good suggestions and should become true, though a tiresome work because there are so many user defined types now. I'm wondering if any contributor would help with such.
I can do that. This is typically the kind of (boring) task that I like
to do when I have little brain cycles (like when it's late, or when I
don't have energy to focus on serious code : doing boring things make me
feel I'm still working and contributing to the community).
>
> Now, let's foduc on the fields declarations. Here, we create ExplicitFields, passing a Xxx.class as the third parameter, and a number as the second parameter.
>
> First of all, as the enum being used has an implicit ordering, can't we simply avoid passing those numbers ? There is already an ExplicitField constructor that takes only 2 parameters, deducing the number from teh enum...
> [Kai] Yeah sure we can just use the 2 parameters constructor for this type. You're good at finding this as the example. We relied on the enum order value mostly but this one and possible many slipped out.
Okie. Another boring task ;-)
>
> Second, why don't we write things like :
>
>
> new ExplicitField(TicketField.TKT_VNO, 0, new Asn1Integer()),
>
> instead of passing a class ? Bottom line, the instance will be created the same way. I don't think it will make any difference in term of performances, and as all the object *must* extends the Asn1Object (or implement the Asn1Type), this should be good enough.
> [Kai] Well, let the framework determine when to create the field instances would be most flexible. We may want to be lazy on creating them and create them on demand; we may need to avoid creating them in decoding because they're nulls; in the framework codes there're some places that uses (value == null) to check some conditions. For Any, the actual value type can only be set by applications in specific contexts.
Makes perfect sense.
Side note : I wonder if calling newInstance(class) is really slower that
doing new Class at this point... It's not anymore the case, I think (it
was before Java 6)
RE: [kerby] ASN1 Quick review
Posted by "Zheng, Kai" <ka...@intel.com>.
Resend after formatted.
Thanks Emmanuel for the review and great comments! The questions are hard but fortunately I'm still kept in the loop so my pleasure to address them. My comments are embedded and marked by [Kai].
-----Original Message-----
From: Emmanuel Lécharny [mailto:elecharny@gmail.com]
Sent: Wednesday, December 23, 2015 8:54 PM
To: Apache Directory Developers List <de...@directory.apache.org>
Subject: [kerby] ASN1 Quick review
Hi band,
I'm having a quick look at the kerby-asn1 code, as I wanted to paly around the idea of porting the LDAP codec to use this piece of nice code. AFAIU, when you want to declare an object that can be encoded or decoded, you have to extend the correct Asn1Object child. Looking at the Ticket object, here is what I see :
[Kai] Glad you want a try. This is also something I wish to help with as discussed before, but am not able to do it immediately because of bandwidth. Sorry for that. I certainly would and will also be able to try to provide any help if needed.
...
import static org.apache.kerby.kerberos.kerb.type.ticket.Ticket.MyEnum.*;
...
public class Ticket extends KrbAppSequenceType {
public static final int TKT_KVNO = KrbConstant.KRB_V5;
public static final int TAG = 1;
protected enum MyEnum implements EnumType {
TKT_VNO,
REALM,
SNAME,
ENC_PART;
@Override
public int getValue() {
return ordinal();
}
@Override
public String getName() {
return name();
}
}
static Asn1FieldInfo[] fieldInfos = new Asn1FieldInfo[] {
new ExplicitField(TKT_VNO, 0, Asn1Integer.class),
new ExplicitField(REALM, 1, KerberosString.class),
new ExplicitField(SNAME, 2, PrincipalName.class),
new ExplicitField(ENC_PART, 3, EncryptedData.class)
};
I like the idea of defining the fields this way, except that I would suggest some slight modifications :
- get read of the import ...Ticket.MyEnum.*;
[Kai] Hmmm, I guess you mean "get rid of", good point. It's like this because, initially we didn't use enum, but int constants. And some weeks ago when we want to dump user defined type objects like this with meaningful field info, we switched to use enum because it can give a friendly name. We were in a hurry at that time and wanted to do it as less effort as possible, thus it's like this now: the enum constant is rather like int constant and used as before.
- make the enum private (there is no reason we wuld like to expose it anyway)
[Kai] Well, actually there are some reasons to make it protected and disciplined exposed. Such user defined types can be extended and these fields may be accessed there. Ref. child classes of ContentInfo (also some other examples I remembered when defining protected int constants).
- name it something more meaningful, like TicketField inseatd if MyEnum
[Kai] Right agree. Again it was like this because it's out as a simple pattern in a whole project scope replacement.
- use it with the enum name, like in new ExplicitField(TicketField.TKT_VNO, 0, Asn1Integer.class)
[Kai] Agree.
<side note>
I find the "import static xxx.*;" atrocious, most of the time. It makes it barely readable : you have to go to the top of your file to actually
*know* where the constant comes from... That may seems of for small files, but otherwise... I accept it for Asserts, because it's really clear, in a test context - pretty much like annotations.
</side note>
[Kai] I thought you may be right, but ...
Here is what I come with :
public class Ticket extends KrbAppSequenceType {
public static final int TKT_KVNO = KrbConstant.KRB_V5;
public static final int TAG = 1;
private enum TicketField implements EnumType {
TKT_VNO,
REALM,
SNAME,
ENC_PART;
@Override
public int getValue() {
return ordinal();
}
@Override
public String getName() {
return name();
}
}
static Asn1FieldInfo[] fieldInfos = new Asn1FieldInfo[] {
new ExplicitField(TicketField.TKT_VNO, 0, Asn1Integer.class),
new ExplicitField(TicketField.REALM, 1, KerberosString.class),
new ExplicitField(TicketField.SNAME, 2, PrincipalName.class),
new ExplicitField(TicketField.ENC_PART, 3, EncryptedData.class)
};
(note that it's just a suggestion at this point...)
[Kai] They're very good suggestions and should become true, though a tiresome work because there are so many user defined types now. I'm wondering if any contributor would help with such.
Now, let's foduc on the fields declarations. Here, we create ExplicitFields, passing a Xxx.class as the third parameter, and a number as the second parameter.
First of all, as the enum being used has an implicit ordering, can't we simply avoid passing those numbers ? There is already an ExplicitField constructor that takes only 2 parameters, deducing the number from teh enum...
[Kai] Yeah sure we can just use the 2 parameters constructor for this type. You're good at finding this as the example. We relied on the enum order value mostly but this one and possible many slipped out.
Second, why don't we write things like :
new ExplicitField(TicketField.TKT_VNO, 0, new Asn1Integer()),
instead of passing a class ? Bottom line, the instance will be created the same way. I don't think it will make any difference in term of performances, and as all the object *must* extends the Asn1Object (or implement the Asn1Type), this should be good enough.
[Kai] Well, let the framework determine when to create the field instances would be most flexible. We may want to be lazy on creating them and create them on demand; we may need to avoid creating them in decoding because they're nulls; in the framework codes there're some places that uses (value == null) to check some conditions. For Any, the actual value type can only be set by applications in specific contexts.
wdyt ?