Hmm, I took a shot at keytool/Main.java and used
KnownOIDs.findMatch(...) to construct the oid.
They will be included in webrev.02.
Thanks,
Valerie
On 5/1/2020 3:29 PM, Valerie Peng wrote:
These two BASE ones are simply used to get rid of the hardcoded oid
string code in keytool/Main.java.
I can remove them (in webrev.02) and maybe you can update
keytool/Main.java later to use the right KnownOIDs enum for oid
construction? There are a few places in keytool/Main.java which can be
updated to use the enum KnownOIDs, but that's a bit far from the main
purpose of this RFE.
Thanks,
Valerie
On 5/1/2020 6:16 AM, Weijun Wang wrote:
One more thing:
In KnownOIDs.java, I found these 2 lines:
PKIX_KP_BASE("1.3.6.1.5.5.7.3."),
PKIX_OCSP_BASE("1.3.6.1.5.5.7.48."),
IMHO, they should not belong here, at least, we should remove the dot
at the end and make them real OIDs.
I was testing the ObjectIdentifier generation and notice these two.
Thanks,
Max
On May 1, 2020, at 5:45 PM, Weijun Wang <weijun.w...@oracle.com> wrote:
ObjectIdentifier.java
---------------------
Have you thought about storing the ObjectIdentifier object
somewhere? ObjectIdentifier.of() creates a new object each time and
the conversion of string to byte[] might be a performance issue. We
used to have a lot of ObjectIdentifier objects in AlgorithmId but
now we only have KnownOIDs.
I had a talk with Stuart and he has a suggestion that we can stuff
all pre-calculated OID DER encodings in a long byte array in a
resource file, and in KnownOIDs each element has an offset/length
pair that point to its DER encoding. Also, whatever cache mechanism
we use in the future, I suggest making "new
ObjectIdentifier(String)" private and keep "ObjectIdentifier
of(String)".
SecurityProviderConstants.java
------------------------------
public static List<String> alias(String ... aliases) {
return Arrays.asList(aliases);
}
Probably not necessary, you can simply call List.of(...) everywhere.
SunMSCAPI.java
--------------
Why not call getAliases() inside "new ProviderService" like in the
other providers? Same in UCrypto.
AlgorithmId.java
----------------
algOID(String). You don't check "if (name.indexOf('.') != -1)" at
the beginning anymore. Is there an algorithm name containing "."?
It's a pity we have to collect OIDs from other providers. Maybe it
should only be necessary when we use that provider, for example,
when encoding a signature, we should ask the provider about the OID.
I wish there were a Signature::getAlgorithmId, but if not, maybe we
can rename Algorithm::alfOID(name) to Algorithm::alfOID(name,
provider).
Do you know a bad case if we don't collect those OIDs? It must be
some algorithm that is not in the Standard Names.
Overall I think the change looks great, and we have a single place
to store all OIDs. The mapping among the OID string, KnownOIDs, and
ObjectIdentifier could be enhanced. Do you have a benchmark?
Thanks,
Max
On Apr 30, 2020, at 6:59 AM, Valerie Peng <valerie.p...@oracle.com>
wrote:
Here is the updated webrev
http://cr.openjdk.java.net/~valeriep/8242151/webrev.01/
