From fe1a4a80707b39fd8f13d12557fe41958849cfc7 Mon Sep 17 00:00:00 2001
From: Dan Lutker <dlutker@openjdk.org>
Date: Thu, 25 Jul 2024 20:53:57 +0000
Subject: [PATCH] 8276660: Scalability bottleneck in
java.security.Provider.getService() 8292739: Invalid legacy entries may be
returned by Provider.getServices() call 8279222: Incorrect legacyMap.get in
java.security.Provider after JDK-8276660
Reviewed-by: coffeys
Backport-of: 9b747491de01fd011b09668a67113e80c2b7c708
---
.../share/classes/java/security/Provider.java | 502 ++++++++++--------
.../Provider/CaseSensitiveServices.java | 33 +-
2 files changed, 303 insertions(+), 232 deletions(-)
diff --git a/src/java.base/share/classes/java/security/Provider.java b/src/java.base/share/classes/java/security/Provider.java
index 7b1ac0e8019..72ddb412578 100644
--- a/src/java.base/share/classes/java/security/Provider.java
+++ b/src/java.base/share/classes/java/security/Provider.java
@@ -195,6 +195,8 @@ public abstract class Provider extends Properties {
this.versionStr = Double.toString(version);
this.info = info;
this.serviceMap = new ConcurrentHashMap<>();
+ this.legacyMap = new ConcurrentHashMap<>();
+ this.prngAlgos = new LinkedHashSet<String>(6);
putId();
initialized = true;
}
@@ -233,6 +235,8 @@ public abstract class Provider extends Properties {
this.version = parseVersionStr(versionStr);
this.info = info;
this.serviceMap = new ConcurrentHashMap<>();
+ this.legacyMap = new ConcurrentHashMap<>();
+ this.prngAlgos = new LinkedHashSet<String>(6);
putId();
initialized = true;
}
@@ -576,7 +580,6 @@ public abstract class Provider extends Properties {
public synchronized boolean replace(Object key, Object oldValue,
Object newValue) {
check("putProviderProperty." + name);
-
if (debug != null) {
debug.println("Replace " + name + " provider property " + key);
}
@@ -602,7 +605,6 @@ public abstract class Provider extends Properties {
@Override
public synchronized Object replace(Object key, Object value) {
check("putProviderProperty." + name);
-
if (debug != null) {
debug.println("Replace " + name + " provider property " + key);
}
@@ -631,7 +633,6 @@ public abstract class Provider extends Properties {
public synchronized void replaceAll(BiFunction<? super Object,
? super Object, ? extends Object> function) {
check("putProviderProperty." + name);
-
if (debug != null) {
debug.println("ReplaceAll " + name + " provider property ");
}
@@ -661,7 +662,6 @@ public abstract class Provider extends Properties {
? super Object, ? extends Object> remappingFunction) {
check("putProviderProperty." + name);
check("removeProviderProperty." + name);
-
if (debug != null) {
debug.println("Compute " + name + " provider property " + key);
}
@@ -688,11 +688,10 @@ public abstract class Provider extends Properties {
* @since 1.8
*/
@Override
- public synchronized Object computeIfAbsent(Object key, Function<? super Object,
- ? extends Object> mappingFunction) {
+ public synchronized Object computeIfAbsent(Object key,
+ Function<? super Object, ? extends Object> mappingFunction) {
check("putProviderProperty." + name);
check("removeProviderProperty." + name);
-
if (debug != null) {
debug.println("ComputeIfAbsent " + name + " provider property " +
key);
@@ -718,11 +717,11 @@ public abstract class Provider extends Properties {
* @since 1.8
*/
@Override
- public synchronized Object computeIfPresent(Object key, BiFunction<? super Object,
- ? super Object, ? extends Object> remappingFunction) {
+ public synchronized Object computeIfPresent(Object key,
+ BiFunction<? super Object, ? super Object, ? extends Object>
+ remappingFunction) {
check("putProviderProperty." + name);
check("removeProviderProperty." + name);
-
if (debug != null) {
debug.println("ComputeIfPresent " + name + " provider property " +
key);
@@ -751,11 +750,11 @@ public abstract class Provider extends Properties {
* @since 1.8
*/
@Override
- public synchronized Object merge(Object key, Object value, BiFunction<? super Object,
- ? super Object, ? extends Object> remappingFunction) {
+ public synchronized Object merge(Object key, Object value,
+ BiFunction<? super Object, ? super Object, ? extends Object>
+ remappingFunction) {
check("putProviderProperty." + name);
check("removeProviderProperty." + name);
-
if (debug != null) {
debug.println("Merge " + name + " provider property " + key);
}
@@ -781,7 +780,8 @@ public abstract class Provider extends Properties {
* @since 1.8
*/
@Override
- public synchronized void forEach(BiConsumer<? super Object, ? super Object> action) {
+ public synchronized void forEach(BiConsumer<? super Object, ? super Object>
+ action) {
checkInitialized();
super.forEach(action);
}
@@ -821,14 +821,11 @@ public abstract class Provider extends Properties {
}
}
- // legacy properties changed since last call to any services method?
- private transient boolean legacyChanged;
+ // legacyMap changed since last call to getServices()
+ private transient volatile boolean legacyChanged;
// serviceMap changed since last call to getServices()
private volatile transient boolean servicesChanged;
- // Map<String,String> used to keep track of legacy registration
- private transient Map<String,String> legacyStrings;
-
// Map<ServiceKey,Service>
// used for services added via putService(), initialized on demand
private transient Map<ServiceKey,Service> serviceMap;
@@ -836,6 +833,9 @@ public abstract class Provider extends Properties {
// For backward compatibility, the registration ordering of
// SecureRandom (RNG) algorithms needs to be preserved for
// "new SecureRandom()" calls when this provider is used
+ // NOTE: may need extra mechanism for providers to indicate their
+ // preferred ordering of SecureRandom algorithms since registration
+ // ordering info is lost once serialized
private transient Set<String> prngAlgos;
// Map<ServiceKey,Service>
@@ -844,7 +844,7 @@ public abstract class Provider extends Properties {
// Set<Service>
// Unmodifiable set of all services. Initialized on demand.
- private transient Set<Service> serviceSet;
+ private transient volatile Set<Service> serviceSet;
// register the id attributes for this provider
// this is to ensure that equals() and hashCode() do not incorrectly
@@ -876,6 +876,7 @@ public abstract class Provider extends Properties {
for (Map.Entry<Object,Object> entry : super.entrySet()) {
copy.put(entry.getKey(), entry.getValue());
}
+
defaults = null;
in.defaultReadObject();
if (this.versionStr == null) {
@@ -886,23 +887,22 @@ public abstract class Provider extends Properties {
this.version = parseVersionStr(this.versionStr);
}
this.serviceMap = new ConcurrentHashMap<>();
+ this.legacyMap = new ConcurrentHashMap<>();
+ this.prngAlgos = new LinkedHashSet<String>(6);
implClear();
initialized = true;
putAll(copy);
}
- // check whether to update 'legacyString' with the specified key
- private boolean checkLegacy(Object key) {
- String keyString = (String)key;
- if (keyString.startsWith("Provider.")) {
+ // returns false if no update necessary, i.e. key isn't String or
+ // is String but it's provider-related (name/version/info/className)
+ private static boolean checkLegacy(Object key) {
+ if (key instanceof String && ((String)key).startsWith("Provider.")) {
+ // ignore provider related updates
return false;
+ } else {
+ return true;
}
-
- legacyChanged = true;
- if (legacyStrings == null) {
- legacyStrings = new LinkedHashMap<>();
- }
- return true;
}
/**
@@ -917,149 +917,161 @@ public abstract class Provider extends Properties {
}
private Object implRemove(Object key) {
- if (key instanceof String) {
- if (!checkLegacy(key)) {
- return null;
- }
- legacyStrings.remove((String)key);
+ if (!checkLegacy(key)) return null;
+
+ Object o = super.remove(key);
+ if (o instanceof String so && key instanceof String sk) {
+ parseLegacy(sk, so, OPType.REMOVE);
}
- return super.remove(key);
+ return o;
}
private boolean implRemove(Object key, Object value) {
- if (key instanceof String && value instanceof String) {
- if (!checkLegacy(key)) {
- return false;
- }
- legacyStrings.remove((String)key, (String)value);
+ if (!checkLegacy(key)) return false;
+
+ boolean result = super.remove(key, value);
+ if (result && key instanceof String sk && value instanceof String sv) {
+ parseLegacy(sk, sv, OPType.REMOVE);
}
- return super.remove(key, value);
+ return result;
}
private boolean implReplace(Object key, Object oldValue, Object newValue) {
- if ((key instanceof String) && (oldValue instanceof String) &&
- (newValue instanceof String)) {
- if (!checkLegacy(key)) {
- return false;
+ if (!checkLegacy(key)) return false;
+
+ boolean result = super.replace(key, oldValue, newValue);
+ if (result && key instanceof String sk) {
+ if (newValue instanceof String sv) {
+ parseLegacy(sk, sv, OPType.ADD);
+ } else if (oldValue instanceof String sv) {
+ parseLegacy(sk, sv, OPType.REMOVE);
}
- legacyStrings.replace((String)key, (String)oldValue,
- (String)newValue);
}
- return super.replace(key, oldValue, newValue);
+ return result;
}
private Object implReplace(Object key, Object value) {
- if ((key instanceof String) && (value instanceof String)) {
- if (!checkLegacy(key)) {
- return null;
+ if (!checkLegacy(key)) return null;
+
+ Object o = super.replace(key, value);
+ if (key instanceof String sk) {
+ if (o instanceof String so) {
+ if (value instanceof String sv) {
+ parseLegacy(sk, sv, OPType.ADD);
+ } else {
+ parseLegacy(sk, so, OPType.REMOVE);
+ }
}
- legacyStrings.replace((String)key, (String)value);
}
- return super.replace(key, value);
+ return o;
}
@SuppressWarnings("unchecked") // Function must actually operate over strings
private void implReplaceAll(BiFunction<? super Object, ? super Object,
? extends Object> function) {
+
+ super.replaceAll(function);
+ // clear out all existing mappings and start fresh
+ legacyMap.clear();
legacyChanged = true;
- if (legacyStrings == null) {
- legacyStrings = new LinkedHashMap<>();
- } else {
- legacyStrings.replaceAll((BiFunction<? super String, ? super String,
- ? extends String>) function);
+ for (Map.Entry<Object, Object> entry : super.entrySet()) {
+ Object key = entry.getKey();
+ Object value = entry.getValue();
+ if ((key instanceof String sk) && (value instanceof String sv)) {
+ if (!checkLegacy(sk)) {
+ continue;
+ }
+ parseLegacy(sk, sv, OPType.ADD);
+ }
}
- super.replaceAll(function);
}
@SuppressWarnings("unchecked") // Function must actually operate over strings
private Object implMerge(Object key, Object value,
BiFunction<? super Object, ? super Object, ? extends Object>
remappingFunction) {
- if ((key instanceof String) && (value instanceof String)) {
- if (!checkLegacy(key)) {
- return null;
+ if (!checkLegacy(key)) return null;
+
+ Object o = super.merge(key, value, remappingFunction);
+ if (key instanceof String sk) {
+ if (o == null) {
+ parseLegacy(sk, null, OPType.REMOVE);
+ } else if (o instanceof String so) {
+ parseLegacy(sk, so, OPType.ADD);
}
- legacyStrings.merge((String)key, (String)value,
- (BiFunction<? super String, ? super String,
- ? extends String>) remappingFunction);
}
- return super.merge(key, value, remappingFunction);
+ return o;
}
@SuppressWarnings("unchecked") // Function must actually operate over strings
private Object implCompute(Object key, BiFunction<? super Object,
? super Object, ? extends Object> remappingFunction) {
- if (key instanceof String) {
- if (!checkLegacy(key)) {
- return null;
+
+ if (!checkLegacy(key)) return null;
+
+ Object o = super.compute(key, remappingFunction);
+ if (key instanceof String sk) {
+ if (o == null) {
+ parseLegacy(sk, null, OPType.REMOVE);
+ } else if (o instanceof String so) {
+ parseLegacy(sk, so, OPType.ADD);
}
- legacyStrings.compute((String) key,
- (BiFunction<? super String,? super String,
- ? extends String>) remappingFunction);
}
- return super.compute(key, remappingFunction);
+ return o;
}
@SuppressWarnings("unchecked") // Function must actually operate over strings
private Object implComputeIfAbsent(Object key, Function<? super Object,
? extends Object> mappingFunction) {
- if (key instanceof String) {
- if (!checkLegacy(key)) {
- return null;
- }
- legacyStrings.computeIfAbsent((String) key,
- (Function<? super String, ? extends String>)
- mappingFunction);
+ if (!checkLegacy(key)) return null;
+
+ Object o = super.computeIfAbsent(key, mappingFunction);
+ if (o instanceof String so && key instanceof String sk) {
+ parseLegacy(sk, so, OPType.ADD);
}
- return super.computeIfAbsent(key, mappingFunction);
+ return o;
}
@SuppressWarnings("unchecked") // Function must actually operate over strings
private Object implComputeIfPresent(Object key, BiFunction<? super Object,
? super Object, ? extends Object> remappingFunction) {
- if (key instanceof String) {
- if (!checkLegacy(key)) {
- return null;
- }
- legacyStrings.computeIfPresent((String) key,
- (BiFunction<? super String, ? super String,
- ? extends String>) remappingFunction);
+ if (!checkLegacy(key)) return null;
+
+ Object o = super.computeIfPresent(key, remappingFunction);
+ if (o instanceof String so && key instanceof String sk) {
+ parseLegacy(sk, so, OPType.ADD);
}
- return super.computeIfPresent(key, remappingFunction);
+ return o;
}
private Object implPut(Object key, Object value) {
- if ((key instanceof String) && (value instanceof String)) {
- if (!checkLegacy(key)) {
- return null;
- }
- legacyStrings.put((String)key, (String)value);
+ if (!checkLegacy(key)) return null;
+
+ Object o = super.put(key, value);
+ if (key instanceof String sk && value instanceof String sv) {
+ parseLegacy(sk, sv, OPType.ADD);
}
- return super.put(key, value);
+ return o;
}
private Object implPutIfAbsent(Object key, Object value) {
- if ((key instanceof String) && (value instanceof String)) {
- if (!checkLegacy(key)) {
- return null;
- }
- legacyStrings.putIfAbsent((String)key, (String)value);
+ if (!checkLegacy(key)) return null;
+
+ Object o = super.putIfAbsent(key, value);
+ if (o == null && key instanceof String sk &&
+ value instanceof String sv) {
+ parseLegacy(sk, sv, OPType.ADD);
}
- return super.putIfAbsent(key, value);
+ return o;
}
private void implClear() {
- if (legacyStrings != null) {
- legacyStrings.clear();
- }
- if (legacyMap != null) {
- legacyMap.clear();
- }
+ legacyMap.clear();
serviceMap.clear();
legacyChanged = false;
servicesChanged = false;
serviceSet = null;
- prngAlgos = null;
+ prngAlgos.clear();
super.clear();
putId();
}
@@ -1089,40 +1101,8 @@ public abstract class Provider extends Properties {
boolean matches(String type, String algorithm) {
return (this.type == type) && (this.originalAlgorithm == algorithm);
}
- }
-
- /**
- * Ensure all the legacy String properties are fully parsed into
- * service objects.
- */
- private void ensureLegacyParsed() {
- if (legacyChanged == false || (legacyStrings == null)) {
- return;
- }
- serviceSet = null;
- if (legacyMap == null) {
- legacyMap = new ConcurrentHashMap<>();
- } else {
- legacyMap.clear();
- }
- for (Map.Entry<String,String> entry : legacyStrings.entrySet()) {
- parseLegacyPut(entry.getKey(), entry.getValue());
- }
- removeInvalidServices(legacyMap);
- legacyChanged = false;
- }
-
- /**
- * Remove all invalid services from the Map. Invalid services can only
- * occur if the legacy properties are inconsistent or incomplete.
- */
- private void removeInvalidServices(Map<ServiceKey,Service> map) {
- for (Iterator<Map.Entry<ServiceKey, Service>> t =
- map.entrySet().iterator(); t.hasNext(); ) {
- Service s = t.next().getValue();
- if (s.isValid() == false) {
- t.remove();
- }
+ public String toString() {
+ return type + "." + algorithm;
}
}
@@ -1140,71 +1120,136 @@ public abstract class Provider extends Properties {
return new String[] {type, alg};
}
+ // utility method for getting a String with service type and algorithm
+ private static String getKey(Service s) {
+ return s.getType() + "." + s.getAlgorithm();
+ }
+
private static final String ALIAS_PREFIX = "Alg.Alias.";
private static final String ALIAS_PREFIX_LOWER = "alg.alias.";
private static final int ALIAS_LENGTH = ALIAS_PREFIX.length();
- private void parseLegacyPut(String name, String value) {
+ private static enum OPType {
+ ADD, REMOVE;
+ }
+
+ private void parseLegacy(String name, String value, OPType opType) {
+ // alias
if (name.toLowerCase(ENGLISH).startsWith(ALIAS_PREFIX_LOWER)) {
// e.g. put("Alg.Alias.MessageDigest.SHA", "SHA-1");
// aliasKey ~ MessageDigest.SHA
- String stdAlg = value;
- String aliasKey = name.substring(ALIAS_LENGTH);
- String[] typeAndAlg = getTypeAndAlgorithm(aliasKey);
+ String aliasKeyStr = name.substring(ALIAS_LENGTH);
+ String[] typeAndAlg = getTypeAndAlgorithm(aliasKeyStr);
if (typeAndAlg == null) {
return;
}
+ legacyChanged = true;
+ Objects.requireNonNull(value, "alias value should map to an alg");
String type = getEngineName(typeAndAlg[0]);
String aliasAlg = typeAndAlg[1].intern();
- ServiceKey key = new ServiceKey(type, stdAlg, true);
- Service s = legacyMap.get(key);
- if (s == null) {
- s = new Service(this, type, stdAlg);
- legacyMap.put(key, s);
+ ServiceKey stdKey = new ServiceKey(type, value, true);
+ Service stdService = legacyMap.get(stdKey);
+ ServiceKey aliasKey = new ServiceKey(type, aliasAlg, true);
+ switch (opType) {
+ case ADD:
+ // clean up old alias if present
+ Service prevAliasService = legacyMap.get(aliasKey);
+ if (prevAliasService != null) {
+ prevAliasService.removeAlias(aliasAlg);
+ }
+ if (stdService == null) {
+ // add standard mapping in order to add alias
+ stdService = new Service(this, type, value);
+ legacyMap.put(stdKey, stdService);
+ }
+ stdService.addAlias(aliasAlg);
+ legacyMap.put(aliasKey, stdService);
+ break;
+ case REMOVE:
+ if (stdService != null) {
+ stdService.removeAlias(aliasAlg);
+ }
+ legacyMap.remove(aliasKey);
+ break;
+ default:
+ throw new AssertionError();
}
- legacyMap.put(new ServiceKey(type, aliasAlg, true), s);
- s.addAlias(aliasAlg);
} else {
String[] typeAndAlg = getTypeAndAlgorithm(name);
if (typeAndAlg == null) {
return;
}
+ legacyChanged = true;
int i = typeAndAlg[1].indexOf(' ');
+ // regular registration
if (i == -1) {
// e.g. put("MessageDigest.SHA-1", "sun.security.provider.SHA");
String type = getEngineName(typeAndAlg[0]);
String stdAlg = typeAndAlg[1].intern();
- String className = value;
- ServiceKey key = new ServiceKey(type, stdAlg, true);
- Service s = legacyMap.get(key);
- if (s == null) {
- s = new Service(this, type, stdAlg);
- legacyMap.put(key, s);
- }
- s.className = className;
-
- if (type.equals("SecureRandom")) {
- updateSecureRandomEntries(true, s.algorithm);
+ ServiceKey stdKey = new ServiceKey(type, stdAlg, true);
+ Service stdService = legacyMap.get(stdKey);
+ switch (opType) {
+ case ADD:
+ Objects.requireNonNull(value,
+ "className can't be null");
+ if (stdService == null) {
+ stdService = new Service(this, type, stdAlg);
+ legacyMap.put(stdKey, stdService);
+ }
+ stdService.className = value;
+ break;
+ case REMOVE:
+ // only remove if value also matches when non-null
+ if (stdService != null) {
+ if (value == null) {
+ legacyMap.remove(stdKey);
+ } else if (stdService.className.equals(value)) {
+ legacyMap.remove(stdKey, stdService);
+ }
+ // remove all corresponding alias mappings
+ for (String alias : stdService.getAliases()) {
+ legacyMap.remove(new ServiceKey(type, alias,
+ true), stdService);
+ }
+ }
+ break;
+ default:
+ throw new AssertionError();
}
+ checkAndUpdateSecureRandom(type, stdAlg,
+ (opType != OPType.REMOVE));
} else { // attribute
// e.g. put("MessageDigest.SHA-1 ImplementedIn", "Software");
- String attributeValue = value;
String type = getEngineName(typeAndAlg[0]);
- String attributeString = typeAndAlg[1];
- String stdAlg = attributeString.substring(0, i).intern();
- String attributeName = attributeString.substring(i + 1);
+ String attrString = typeAndAlg[1];
+ String stdAlg = attrString.substring(0, i).intern();
+ String attrName = attrString.substring(i + 1);
// kill additional spaces
- while (attributeName.startsWith(" ")) {
- attributeName = attributeName.substring(1);
+ while (attrName.startsWith(" ")) {
+ attrName = attrName.substring(1);
}
- attributeName = attributeName.intern();
- ServiceKey key = new ServiceKey(type, stdAlg, true);
- Service s = legacyMap.get(key);
- if (s == null) {
- s = new Service(this, type, stdAlg);
- legacyMap.put(key, s);
+ attrName = attrName.intern();
+ ServiceKey stdKey = new ServiceKey(type, stdAlg, true);
+ Service stdService = legacyMap.get(stdKey);
+ switch (opType) {
+ case ADD:
+ Objects.requireNonNull(value,
+ "attribute value should not be null");
+
+ if (stdService == null) {
+ stdService = new Service(this, type, stdAlg);
+ legacyMap.put(stdKey, stdService);
+ }
+ stdService.addAttribute(attrName, value);
+ break;
+ case REMOVE:
+ if (stdService != null) {
+ stdService.removeAttribute(attrName, value);
+ }
+ break;
+ default:
+ throw new AssertionError();
}
- s.addAttribute(attributeName, attributeValue);
}
}
}
@@ -1231,23 +1276,18 @@ public abstract class Provider extends Properties {
*/
public Service getService(String type, String algorithm) {
checkInitialized();
-
// avoid allocating a new ServiceKey object if possible
ServiceKey key = previousKey;
if (key.matches(type, algorithm) == false) {
key = new ServiceKey(type, algorithm, false);
previousKey = key;
}
- Service s = null;
- if (!serviceMap.isEmpty()) {
- s = serviceMap.get(key);
- }
+
+ Service s = serviceMap.get(key);
if (s == null) {
- synchronized (this) {
- ensureLegacyParsed();
- if (legacyMap != null && !legacyMap.isEmpty()) {
- s = legacyMap.get(key);
- }
+ s = legacyMap.get(key);
+ if (s != null && !s.isValid()) {
+ legacyMap.remove(key, s);
}
}
@@ -1280,22 +1320,25 @@ public abstract class Provider extends Properties {
*
* @since 1.5
*/
- public synchronized Set<Service> getServices() {
+ public Set<Service> getServices() {
checkInitialized();
- if (legacyChanged || servicesChanged) {
- serviceSet = null;
- }
- if (serviceSet == null) {
- ensureLegacyParsed();
+ if (serviceSet == null || legacyChanged || servicesChanged) {
Set<Service> set = new LinkedHashSet<>();
if (!serviceMap.isEmpty()) {
set.addAll(serviceMap.values());
}
- if (legacyMap != null && !legacyMap.isEmpty()) {
- set.addAll(legacyMap.values());
+ if (!legacyMap.isEmpty()) {
+ legacyMap.entrySet().forEach(entry -> {
+ if (!entry.getValue().isValid()) {
+ legacyMap.remove(entry.getKey(), entry.getValue());
+ } else {
+ set.add(entry.getValue());
+ }
+ });
}
serviceSet = Collections.unmodifiableSet(set);
servicesChanged = false;
+ legacyChanged = false;
}
return serviceSet;
}
@@ -1352,44 +1395,36 @@ public abstract class Provider extends Properties {
servicesChanged = true;
synchronized (this) {
putPropertyStrings(s);
- if (type.equals("SecureRandom")) {
- updateSecureRandomEntries(true, s.algorithm);
- }
+ checkAndUpdateSecureRandom(type, algorithm, true);
}
}
- // keep tracks of the registered secure random algos and store them in order
- private void updateSecureRandomEntries(boolean doAdd, String s) {
- Objects.requireNonNull(s);
- if (doAdd) {
- if (prngAlgos == null) {
- prngAlgos = new LinkedHashSet<String>();
+ private void checkAndUpdateSecureRandom(String type, String algo,
+ boolean doAdd) {
+ if (type.equalsIgnoreCase("SecureRandom")) {
+ if (doAdd) {
+ prngAlgos.add(algo);
+ } else {
+ prngAlgos.remove(algo);
+ }
+ if (debug != null) {
+ debug.println((doAdd? "Add":"Remove") +
+ " SecureRandom algo " + algo);
}
- prngAlgos.add(s);
- } else {
- prngAlgos.remove(s);
- }
-
- if (debug != null) {
- debug.println((doAdd? "Add":"Remove") + " SecureRandom algo " + s);
}
}
// used by new SecureRandom() to find out the default SecureRandom
// service for this provider
- synchronized Service getDefaultSecureRandomService() {
+ Service getDefaultSecureRandomService() {
checkInitialized();
- if (legacyChanged) {
- prngAlgos = null;
- ensureLegacyParsed();
- }
-
- if (prngAlgos != null && !prngAlgos.isEmpty()) {
+ if (!prngAlgos.isEmpty()) {
+ String algo = prngAlgos.iterator().next();
// IMPORTANT: use the Service obj returned by getService(...) call
// as providers may override putService(...)/getService(...) and
// return their own Service objects
- return getService("SecureRandom", prngAlgos.iterator().next());
+ return getService("SecureRandom", algo);
}
return null;
@@ -1486,12 +1521,9 @@ public abstract class Provider extends Properties {
for (String alias : s.getAliases()) {
serviceMap.remove(new ServiceKey(type, alias, false));
}
- synchronized (this) {
- removePropertyStrings(s);
- if (type.equals("SecureRandom")) {
- updateSecureRandomEntries(false, s.algorithm);
- }
- }
+
+ removePropertyStrings(s);
+ checkAndUpdateSecureRandom(type, algorithm, false);
}
// Wrapped String that behaves in a case insensitive way for equals/hashCode
@@ -1693,6 +1725,13 @@ public abstract class Provider extends Properties {
aliases.add(alias);
}
+ private void removeAlias(String alias) {
+ if (aliases.isEmpty()) {
+ return;
+ }
+ aliases.remove(alias);
+ }
+
void addAttribute(String type, String value) {
if (attributes.isEmpty()) {
attributes = new HashMap<>(8);
@@ -1700,6 +1739,17 @@ public abstract class Provider extends Properties {
attributes.put(new UString(type), value);
}
+ void removeAttribute(String type, String value) {
+ if (attributes.isEmpty()) {
+ return;
+ }
+ if (value == null) {
+ attributes.remove(new UString(type));
+ } else {
+ attributes.remove(new UString(type), value);
+ }
+ }
+
/**
* Construct a new service.
*
diff --git a/test/jdk/java/security/Provider/CaseSensitiveServices.java b/test/jdk/java/security/Provider/CaseSensitiveServices.java
index 2a6676b4f58..a5b39bde3af 100644
--- a/test/jdk/java/security/Provider/CaseSensitiveServices.java
+++ b/test/jdk/java/security/Provider/CaseSensitiveServices.java
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2004, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2004, 2022, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -23,7 +23,7 @@
/*
* @test
- * @bug 5097015 8130181
+ * @bug 5097015 8130181 8279222 8292739
* @summary make sure we correctly treat Provider string entries as case insensitive
* @author Andreas Sterbenz
*/
@@ -36,29 +36,50 @@ public class CaseSensitiveServices extends Provider {
super("Foo", "1.0", null);
put("MessageDigest.Foo", "com.Foo");
put("mESSAGEdIGEST.fOO xYz", "aBc");
- put("ALg.aliaS.MESSAGEdigest.Fu", "FoO");
+ // first assign the DEF alias to algorithm Foo
+ put("ALg.aliaS.MESSAGEdigest.DEF", "FoO");
put("messageDigest.Bar", "com.Bar");
put("MESSAGEDIGEST.BAZ", "com.Baz");
+ // reassign the DEF alias to algorithm Bar
+ put("ALg.aliaS.MESSAGEdigest.DEF", "Bar");
+ // invalid entry since it misses the corresponding impl class info
+ // e.g. put("MessageDigest.Invalid", "implClass");
+ put("MessageDigest.Invalid xYz", "aBc");
}
public static void main(String[] args) throws Exception {
Provider p = new CaseSensitiveServices();
- System.out.println(p.getServices());
+
+ System.out.println("Services: " + p.getServices());
+
if (p.getServices().size() != 3) {
throw new Exception("services.size() should be 3");
}
+
+ if (p.getService("MessageDigest", "Invalid") != null) {
+ throw new Exception("Invalid service returned");
+ }
Service s = testService(p, "MessageDigest", "fOO");
String val = s.getAttribute("Xyz");
if ("aBc".equals(val) == false) {
throw new Exception("Wrong value: " + val);
}
- testService(p, "MessageDigest", "fU");
+ if (s.toString().indexOf("DEF") != -1) {
+ throw new Exception("Old alias DEF should be removed");
+ }
+
+ // test Service alias DEF and its associated impl is Bar
+ s = testService(p, "MessageDigest", "DeF");
+ if (s.getAttribute("Xyz") != null) {
+ throw new Exception("DEF mapped to the wrong impl");
+ }
testService(p, "MessageDigest", "BAR");
testService(p, "MessageDigest", "baz");
System.out.println("OK");
}
- private static Service testService(Provider p, String type, String alg) throws Exception {
+ private static Service testService(Provider p, String type, String alg)
+ throws Exception {
System.out.println("Getting " + type + "." + alg + "...");
Service s = p.getService(type, alg);
System.out.println(s);
--
GitLab