CRLExtensions.java
==================
116 Class<?> extClass =
ExtensionsMap.getClass(ext.getExtensionId());
117 if (extClass == null) { // Unsupported extension
118 if (ext.isCritical())
119 unsupportedCritExt = true;
120 if (map.put(ext.getId(), ext) != null)
121 throw new CRLException("Duplicate extensions not
allowed");
122 return;
123 }
124 Constructor<?> cons = extClass.getConstructor(PARAMS);
125 Object[] passed = new Object[]
{Boolean.valueOf(ext.isCritical()),
126 ext.getExtensionValue()};
127 CertAttrSet<?> crlExt =
(CertAttrSet<?>)cons.newInstance(passed);
128 if (map.put(ext.getId(), (Extension)crlExt) != null) {
129 throw new CRLException("Duplicate extensions not allowed");
130 }
I see no reason to convert cons.newInstance(passed) first to one type then
another. Also, the 2 throw statements can be combined to one.
195 public void set(String oid, Object obj) {
196 map.put(oid, (Extension) obj);
197 }
How about simply set(Object obj) or set(Extension ext)? You can always find oid
from ext.getId(). This prevents unnoticed codes still using the name as keys.
I'd like to add a comment to the map field, something like
For known extensions listed in ExtensionMap, the value must be of a child
type of Extension (defined in ExtensionMap).
This is quite critical for the getExtension() methods in X509CRLImpl to work
correctly.
CertificateExtensions.java
==========================
Same as above, but in the comment of map, there is an extra case
Known extensions which is unparseable and not critical will be stored in an
unparseableExtensions map.
Since we don't use names as keys, CertificateExtensions is not a typical
CertAttrSet now. Remove the implements clause.
OtherName.java
==============
ExtensionMap will not support OtherName anymore. getGNI() should always return
null. File a new RFE on how to implement OtherName in post-jdk9 era.
certAttributes.html
===================
229 Extensions can be added by implementing the
230 <code>sun.security.x509.CertAttrSet</code> interface and
231 subclassing <code>sun.security.x509.Extension</code> class.
232 Register the new extension using the ExtensionsMap class.
After jdk9, these interface/class will not be available outside jdk. File an
enhancement on how to do this later.
ExtensionsMap.java
==================
195 public static void addAttribute(String name, String oid, Class<?>
clazz)
196 throws CertificateException {
After jdk9, this method will not be available outside jdk. Mention this in the
enhancement filed above.
Thanks
Max
> On Mar 28, 2015, at 22:45, Wang Weijun <[email protected]> wrote:
>
>>
>> On Mar 28, 2015, at 05:19, Jason Uh <[email protected]> wrote:
>>
>>
>>
>> On 03/27/2015 03:53 AM, Wang Weijun wrote:
>>>
>>>> On Mar 27, 2015, at 06:37, Jason Uh <[email protected]> wrote:
>>>>
>>>> Please review this revision:
>>>> http://cr.openjdk.java.net/~juh/7145757/01/
>>>>
>>>> * a global nameCache is maintained in OIDMap as suggested
>>>
>>> Can you just use the existing OIDMap.getId() method? It looks like your
>>> getCachedOid(name) is the same as getId("x509.info.extensions." + name).
>>>
>>> In fact, since the OIDMap only contains mapping of extensions, I'd suggest
>>> renaming it to ExtensionMap and change the name inside to short names
>>> (without the "x509.info.extensions." prefix).
>>
>> OK, thanks for that suggestion. I thought there was some significance to
>> using the "full" name in OIDMap,
>
> I think it was designed to be more powerful (see how OtherName uses it) but
> unfortunately it hasn't been so. Now that with jdk9/module it is no more
> accessible from outside the JDK, we can simplify it.
>
>> but if that's not necessary, it makes things more flexible. Here is the
>> latest revision that uses only the existing OIDMap (now called
>> ExtensionsMap).
>>
>> http://cr.openjdk.java.net/~juh/7145757/02/
>
> Will read it.
>
> Thanks
> Max
>
>>
>> Thanks,
>> Jason
>>
>>> --Max
