This is an automated email from the ASF dual-hosted git repository. thiagohp pushed a commit to branch 5.9.x in repository https://gitbox.apache.org/repos/asf/tapestry-5.git
commit d1e275e603add833795416c32bb06b82c12c376b Author: Ben Weidig <[email protected]> AuthorDate: Wed Oct 7 08:21:08 2026 +0200 LocalizationSetter improvements --- .../internal/services/LocalizationSetterImpl.java | 82 ++++++++++++++- .../services/LocalizationSetterImplTest.java | 115 ++++++++++++++++++++- 2 files changed, 191 insertions(+), 6 deletions(-) diff --git a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/LocalizationSetterImpl.java b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/LocalizationSetterImpl.java index b2962ca57..e86069348 100644 --- a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/LocalizationSetterImpl.java +++ b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/LocalizationSetterImpl.java @@ -1,4 +1,4 @@ -// Copyright 2006, 2007, 2009, 2010, 2012 The Apache Software Foundation +// Copyright 2006, 2007, 2009, 2010, 2012, 2026 The Apache Software Foundation // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -39,6 +39,23 @@ import org.apache.tapestry5.services.PersistentLocale; */ public class LocalizationSetterImpl implements LocalizationSetter { + /** + * Upper bound on the number of entries retained in {@link #localeCache}. + * + * <p> + * The cache lives for the lifetime of this (singleton) service. + * The configured supported locales are pre-seeded and never evicted, so this budget only applies + * to request-derived names. + * </p> + */ + static final int MAX_LOCALE_CACHE_SIZE = 1000; + + /** + * Maximum length of a single underscore-separated term (language, country or variant) of a + * cacheable locale name. + */ + static final int MAX_TERM_LENGTH = 16; + private final Request request; private final ThreadLocale threadLocale; @@ -87,7 +104,13 @@ public class LocalizationSetterImpl implements LocalizationSetter for (String name : localeNames) { - list.add(toLocale(name)); + Locale locale = constructLocale(name); + + // Pre-seed the cache with the supported locales. + // They are trusted configuration and the hot resolution target. + localeCache.put(name, locale); + + list.add(locale); } return Collections.unmodifiableList(list); @@ -100,12 +123,63 @@ public class LocalizationSetterImpl implements LocalizationSetter if (result == null) { result = constructLocale(localeName); - localeCache.put(localeName, result); + + if (isCacheableLocaleName(localeName) && localeCache.size() < MAX_LOCALE_CACHE_SIZE) + { + localeCache.put(localeName, result); + } } return result; } + /** + * Whether a locale name is structurally plausible enough to be worth storing in the long-lived + * {@link #localeCache}. + * + * <p>This is a deliberately strict, cheap guard, instead of a full full locale validation: + * at most three underscore-separated terms, each ASCII-alphanumeric and bounded by + * {@link #MAX_TERM_LENGTH}. + */ + static boolean isCacheableLocaleName(String localeName) + { + final int length = localeName.length(); + + if (length == 0) + { + return false; + } + + int terms = 1; + int termLength = 0; + + for (int i = 0; i < length; i++) + { + final char c = localeName.charAt(i); + + if (c == '_') + { + // matches constructLocale()'s own limit of three terms + if (++terms > 3) + { + return false; + } + termLength = 0; + } + else if (!isAsciiLetterOrDigit(c) || ++termLength > MAX_TERM_LENGTH) + { + return false; + } + } + + return true; + } + + private static boolean isAsciiLetterOrDigit(char c) + { + return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9'); + } + private Locale constructLocale(String name) { String[] terms = name.split("_"); @@ -119,11 +193,9 @@ public class LocalizationSetterImpl implements LocalizationSetter return new Locale(terms[0], terms[1]); case 3: - return new Locale(terms[0], terms[1], terms[2]); default: - throw new IllegalArgumentException(); } } diff --git a/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/LocalizationSetterImplTest.java b/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/LocalizationSetterImplTest.java index 8aa409b6d..0c1e69138 100644 --- a/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/LocalizationSetterImplTest.java +++ b/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/LocalizationSetterImplTest.java @@ -1,4 +1,4 @@ -// Copyright 2006, 2009, 2010, 2012 The Apache Software Foundation +// Copyright 2006, 2009, 2010, 2012, 2026 The Apache Software Foundation // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -18,6 +18,7 @@ import static org.apache.tapestry5.commons.util.CollectionFactory.newSet; import java.util.List; import java.util.Locale; +import java.util.Map; import org.apache.tapestry5.OptionModel; import org.apache.tapestry5.SelectModel; @@ -71,6 +72,118 @@ public class LocalizationSetterImplTest extends InternalBaseTestCase checkLocale(setter.toLocale("klingon_Gach_snuff"), "klingon", "GACH", "snuff"); } + @SuppressWarnings("unchecked") + private static Map<String, Locale> localeCache(LocalizationSetterImpl setter) + { + return (Map<String, Locale>) TestBase.get(setter, "localeCache"); + } + + @Test + public void is_cacheable_locale_name() + { + // well-formed names are cacheable + assertTrue(LocalizationSetterImpl.isCacheableLocaleName("en")); + assertTrue(LocalizationSetterImpl.isCacheableLocaleName("en_US")); + assertTrue(LocalizationSetterImpl.isCacheableLocaleName("en_US_POSIX")); + assertTrue(LocalizationSetterImpl.isCacheableLocaleName("es_419")); // numeric UN M.49 region + assertTrue(LocalizationSetterImpl.isCacheableLocaleName("klingon_Gach_snuff")); + + // malformed / hostile names are not + assertFalse(LocalizationSetterImpl.isCacheableLocaleName("")); + assertFalse(LocalizationSetterImpl.isCacheableLocaleName("en_US_en_US")); // more than three terms + assertFalse(LocalizationSetterImpl.isCacheableLocaleName("en-US")); // non-alphanumeric separator + assertFalse(LocalizationSetterImpl.isCacheableLocaleName("en/../secret")); // path-ish junk + + // a single over-long term, bounded by MAX_TERM_LENGTH... + assertFalse(LocalizationSetterImpl.isCacheableLocaleName( + repeat('a', LocalizationSetterImpl.MAX_TERM_LENGTH + 1))); + + assertFalse(LocalizationSetterImpl.isCacheableLocaleName( + "en_US_" + repeat('a', LocalizationSetterImpl.MAX_TERM_LENGTH + 1))); + + // a genuinely huge string is rejected without being fully scanned + assertFalse(LocalizationSetterImpl.isCacheableLocaleName(repeat('a', 8192))); + } + + @Test + public void supported_locales_are_seeded_into_the_cache() + { + LocalizationSetterImpl setter = new LocalizationSetterImpl(null, null, null, "en,fr"); + + Map<String, Locale> cache = localeCache(setter); + + // pre-seeded at construction, before any request can be processed + assertTrue(cache.containsKey("en")); + assertTrue(cache.containsKey("fr")); + } + + @Test + public void locale_cache_is_size_bounded() + { + LocalizationSetterImpl setter = new LocalizationSetterImpl(null, null, null, "en"); + + // Fill the cache with distinct, well-formed short names. + // The cache must stop growing at its cap rather than expanding without bound. + for (int i = 0; i < LocalizationSetterImpl.MAX_LOCALE_CACHE_SIZE + 500; i++) + { + setter.toLocale("zz" + i); + } + + assertTrue(localeCache(setter).size() <= LocalizationSetterImpl.MAX_LOCALE_CACHE_SIZE); + } + + @Test + public void overly_long_locale_names_are_not_cached() + { + LocalizationSetterImpl setter = new LocalizationSetterImpl(null, null, null, "en"); + + // A well-formed short name is still cached (same instance returned on the second call)... + assertSame(setter.toLocale("de"), setter.toLocale("de")); + + // ...large name is resolved without being retained, so it is re-parsed and + // a fresh (non-cached) instance comes back each time. + String longName = repeat('a', 8192); + + assertNotNull(setter.toLocale(longName)); + assertNotSame(setter.toLocale(longName), setter.toLocale(longName)); + assertFalse(localeCache(setter).containsKey(longName)); + } + + @Test + public void attacker_supplied_locale_name_is_not_retained() + { + PersistentLocale pl = mockPersistentLocale(); + ThreadLocale tl = mockThreadLocale(); + Request request = mockRequest(); + + // an unresolvable, over-long name narrows to the default (first supported) locale + tl.setLocale(Locale.ENGLISH); + + replay(); + + LocalizationSetterImpl setter = new LocalizationSetterImpl(request, pl, tl, "en,fr"); + + String attackerName = repeat('x', 8192); + + setter.setNonPersistentLocaleFromLocaleName(attackerName); + + verify(); + + assertFalse(localeCache(setter).containsKey(attackerName)); + } + + private static String repeat(char c, int count) + { + StringBuilder builder = new StringBuilder(count); + + for (int i = 0; i < count; i++) + { + builder.append(c); + } + + return builder.toString(); + } + @Test public void known_locale() {
