Yes, it will be extensive, but since we already make all keys OID, we should always call those set/get() with OIDs for performance. In case we might miss some, inside the implementation of get/set(), we can still accept names.
Also, it seems there is no need for a nameCache everywhere, you can probably maintain a global one inside OIDMap. As for the IOException, I think it's because CRLExtensions does not have getExtension() that returns null. Now that you create another inconsistence that X509CRLImpl.getExtension() is starting to throw IOE while X509CertImpl.getExtension() does not. --Max > On Mar 19, 2015, at 07:40, Jason Uh <jason...@oracle.com> wrote: > > Hi Max, > > On 03/18/2015 04:09 PM, Wang Weijun wrote: >> Hi Jason >> >> I was thinking about changing all set/get calls to using OID so inside >> CertificateExtensions and CRLExtensions you won't need to care about name >> conversions. Is that possible? > > It might be, but it'd call for more extensive changes. Currently, when > certificates are parsed, the extensions are added using the name, and when > they are retrieved, they are also specified by name -- for example, > X509CertImpl.getBasicConstraints() calls a get() method with the > BasicConstraints alias as the lookup parameter. > > It's been a while, but the last we spoke about this, I think you wanted to > keep the changes for this contained within CertificateExtensions and > CRLExtensions. If that's still the case, we'll have to do this name > conversion. If not, I could try changing things outside of these classes. > >> Also I see you adding some IOException throwing and catching. Is there any >> case you actually see them? I would think if that really happen there might >> be some programming error. > > I added the IOExceptions mainly to keep CRLExtensions consistent with > CertificateExtensions. I thought it was a little odd that they weren't being > thrown, but they may not need to be there for CRLExtensions. I can take them > out. > >> --Max >> >>> On Mar 18, 2015, at 09:34, Jason Uh <jason...@oracle.com> wrote: >>> >>> Please review this fix, which changes the internal map in >>> sun.security.x509.CertificateExtensions and CRLExtensions to always use the >>> OID as the key. >>> >>> As stated in the JBS issue: >>>> The sun.security.x509.CertificateExtensions class maintains a >>>> Map<String,Extension> map field to store all the extensions it manages. >>>> The key of map uses the name (say, "BasicConstraints") of the extension, >>>> or the OID when the type of the extension is unknown. >>> >>> With this change, the map will consistently use the OID as the key. >>> >>> webrev: http://cr.openjdk.java.net/~juh/7145757/00/ >>> bug: https://bugs.openjdk.java.net/browse/JDK-7145757 >>> >>> Thanks, >>> Jason >>
