This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit a012d122e7941434281e29bd76113a9755e1cc63
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Tue Jan 24 10:16:41 2023 +0000
Review all uses of LinkedHashMap#removeEldestEntry
Ensure FIFO or LRU is used as appropriate.
Add/expand existing comments to make intentions clearer.
Don't override default load factor to retain size/performance balance.
---
java/org/apache/catalina/authenticator/DigestAuthenticator.java | 4 ++++
java/org/apache/catalina/filters/CsrfPreventionFilter.java | 6 ++++++
java/org/apache/catalina/realm/LockOutRealm.java | 7 +++++--
java/org/apache/catalina/tribes/util/StringManager.java | 9 ++++-----
java/org/apache/juli/OneLineFormatter.java | 4 ++++
java/org/apache/tomcat/util/res/StringManager.java | 9 ++++-----
6 files changed, 27 insertions(+), 12 deletions(-)
diff --git a/java/org/apache/catalina/authenticator/DigestAuthenticator.java
b/java/org/apache/catalina/authenticator/DigestAuthenticator.java
index 9775705447..9ef3dd8b40 100644
--- a/java/org/apache/catalina/authenticator/DigestAuthenticator.java
+++ b/java/org/apache/catalina/authenticator/DigestAuthenticator.java
@@ -376,6 +376,10 @@ public class DigestAuthenticator extends AuthenticatorBase
{
setOpaque(sessionIdGenerator.generateSessionId());
}
+ /*
+ * This is a FIFO cache as using an older nonce should not delay its
removal from the cache in favour of more
+ * recent values.
+ */
nonces = new LinkedHashMap<>() {
private static final long serialVersionUID = 1L;
diff --git a/java/org/apache/catalina/filters/CsrfPreventionFilter.java
b/java/org/apache/catalina/filters/CsrfPreventionFilter.java
index e018d62989..349e755c14 100644
--- a/java/org/apache/catalina/filters/CsrfPreventionFilter.java
+++ b/java/org/apache/catalina/filters/CsrfPreventionFilter.java
@@ -331,6 +331,12 @@ public class CsrfPreventionFilter extends
CsrfPreventionFilterBase {
}
+ /**
+ * Despite its name, this is a FIFO cache not an LRU cache. Using an older
nonce should not delay its removal from
+ * the cache in favour of more recent values.
+ *
+ * @param <T> The type held by this cache.
+ */
protected static class LruCache<T> implements NonceCache<T> {
private static final long serialVersionUID = 1L;
diff --git a/java/org/apache/catalina/realm/LockOutRealm.java
b/java/org/apache/catalina/realm/LockOutRealm.java
index ab09da9811..fc79bee747 100644
--- a/java/org/apache/catalina/realm/LockOutRealm.java
+++ b/java/org/apache/catalina/realm/LockOutRealm.java
@@ -81,8 +81,11 @@ public class LockOutRealm extends CombinedRealm {
*/
@Override
protected synchronized void startInternal() throws LifecycleException {
- // Configure the list of failed users to delete the oldest entry once
it
- // exceeds the specified size
+ /*
+ * Configure the list of failed users to delete the oldest entry once
it exceeds the specified size. This is an
+ * LRU cache so if the cache size is exceeded the users who most
recently failed authentication will be
+ * retained.
+ */
failedUsers = new LinkedHashMap<>(cacheSize, 0.75f, true) {
private static final long serialVersionUID = 1L;
diff --git a/java/org/apache/catalina/tribes/util/StringManager.java
b/java/org/apache/catalina/tribes/util/StringManager.java
index 32a69e6e39..c3096263f7 100644
--- a/java/org/apache/catalina/tribes/util/StringManager.java
+++ b/java/org/apache/catalina/tribes/util/StringManager.java
@@ -213,12 +213,11 @@ public class StringManager {
Map<Locale, StringManager> map = managers.get(packageName);
if (map == null) {
/*
- * Don't want the HashMap to be expanded beyond LOCALE_CACHE_SIZE.
Expansion occurs when size() exceeds
- * capacity. Therefore keep size at or below capacity.
removeEldestEntry() executes after insertion
- * therefore the test for removal needs to use one less than the
maximum desired size
- *
+ * Don't want the HashMap size to exceed LOCALE_CACHE_SIZE.
Expansion occurs when size() exceeds capacity.
+ * Therefore keep size at or below capacity. removeEldestEntry()
executes after insertion therefore the test
+ * for removal needs to use one less than the maximum desired
size. Note this is an LRU cache.
*/
- map = new LinkedHashMap<>(LOCALE_CACHE_SIZE, 1, true) {
+ map = new LinkedHashMap<>(LOCALE_CACHE_SIZE, 0.75f, true) {
private static final long serialVersionUID = 1L;
@Override
diff --git a/java/org/apache/juli/OneLineFormatter.java
b/java/org/apache/juli/OneLineFormatter.java
index 85004ffe5e..00e8ffb82f 100644
--- a/java/org/apache/juli/OneLineFormatter.java
+++ b/java/org/apache/juli/OneLineFormatter.java
@@ -238,6 +238,9 @@ public class OneLineFormatter extends Formatter {
}
+ /*
+ * This is an LRU cache.
+ */
private static class ThreadNameCache extends LinkedHashMap<Long, String> {
private static final long serialVersionUID = 1L;
@@ -245,6 +248,7 @@ public class OneLineFormatter extends Formatter {
private final int cacheSize;
public ThreadNameCache(int cacheSize) {
+ super(cacheSize, 0.75f, true);
this.cacheSize = cacheSize;
}
diff --git a/java/org/apache/tomcat/util/res/StringManager.java
b/java/org/apache/tomcat/util/res/StringManager.java
index 8abff4dc71..78d05f9745 100644
--- a/java/org/apache/tomcat/util/res/StringManager.java
+++ b/java/org/apache/tomcat/util/res/StringManager.java
@@ -220,12 +220,11 @@ public class StringManager {
Map<Locale, StringManager> map = managers.get(packageName);
if (map == null) {
/*
- * Don't want the HashMap to be expanded beyond LOCALE_CACHE_SIZE.
Expansion occurs when size() exceeds
- * capacity. Therefore keep size at or below capacity.
removeEldestEntry() executes after insertion
- * therefore the test for removal needs to use one less than the
maximum desired size
- *
+ * Don't want the HashMap size to exceed LOCALE_CACHE_SIZE.
Expansion occurs when size() exceeds capacity.
+ * Therefore keep size at or below capacity. removeEldestEntry()
executes after insertion therefore the test
+ * for removal needs to use one less than the maximum desired
size. Note this is an LRU cache.
*/
- map = new LinkedHashMap<>(LOCALE_CACHE_SIZE, 1, true) {
+ map = new LinkedHashMap<>(LOCALE_CACHE_SIZE, 0.75f, true) {
private static final long serialVersionUID = 1L;
@Override
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
