WW-5422 Fixes support for trimable locale string in request

lukaszlenart · lukaszlenart · commit 627f27a040e3 · 2024-05-11T07:23:22.000+02:00
diff --git a/core/src/main/java/com/opensymphony/xwork2/ActionSupport.java b/core/src/main/java/com/opensymphony/xwork2/ActionSupport.java
@@ -90,6 +90,11 @@ public boolean isValidLocale(Locale locale) {
         return getLocaleProvider().isValidLocale(locale);
     }
 
+    @Override
+    public Locale toLocale(String localeStr) {
+        return getLocaleProvider().toLocale(localeStr);
+    }
+
     @Override
     public boolean hasKey(String key) {
         return getTextProvider().hasKey(key);
diff --git a/core/src/main/java/com/opensymphony/xwork2/DefaultLocaleProvider.java b/core/src/main/java/com/opensymphony/xwork2/DefaultLocaleProvider.java
@@ -48,7 +48,7 @@ public Locale getLocale() {
     public boolean isValidLocaleString(String localeStr) {
         Locale locale = null;
         try {
-            locale = LocaleUtils.toLocale(StringUtils.trimToNull(localeStr));
+            locale = LocaleUtils.toLocale(localeStr);
         } catch (IllegalArgumentException e) {
             LOG.warn(new ParameterizedMessage("Cannot convert [{}] to proper locale", localeStr), e);
         }
@@ -59,4 +59,15 @@ public boolean isValidLocaleString(String localeStr) {
     public boolean isValidLocale(Locale locale) {
         return LocaleUtils.isAvailableLocale(locale);
     }
+
+    @Override
+    public Locale toLocale(String localeStr) {
+        Locale locale = null;
+        try {
+            locale = LocaleUtils.toLocale(StringUtils.trimToNull(localeStr));
+        } catch (IllegalArgumentException e) {
+            LOG.warn(new ParameterizedMessage("Cannot convert [{}] to proper locale", localeStr), e);
+        }
+        return locale;
+    }
 }
diff --git a/core/src/main/java/com/opensymphony/xwork2/LocaleProvider.java b/core/src/main/java/com/opensymphony/xwork2/LocaleProvider.java
@@ -18,6 +18,9 @@
  */
 package com.opensymphony.xwork2;
 
+import org.apache.commons.lang3.LocaleUtils;
+import org.apache.commons.lang3.StringUtils;
+
 import java.util.Locale;
 
 
@@ -58,4 +61,17 @@ public interface LocaleProvider {
      */
     boolean isValidLocale(Locale locale);
 
+    /**
+     * Tries to convert provided locale string into {@link Locale} or returns null
+     * @param localeStr a String representing locale, e.g.: en_EN
+     * @return instance of {@link Locale} or null
+     * @since Struts 6.5.0
+     */
+    default Locale toLocale(String localeStr) {
+        try {
+            return LocaleUtils.toLocale(StringUtils.trimToNull(localeStr));
+        } catch (IllegalArgumentException e) {
+            return null;
+        }
+    }
 }
diff --git a/core/src/main/java/com/opensymphony/xwork2/validator/DelegatingValidatorContext.java b/core/src/main/java/com/opensymphony/xwork2/validator/DelegatingValidatorContext.java
@@ -122,10 +122,15 @@ public boolean isValidLocale(Locale locale) {
         return localeProvider.isValidLocale(locale);
     }
 
+    @Override
+    public Locale toLocale(String localeStr) {
+        return localeProvider.toLocale(localeStr);
+    }
+
     public boolean hasKey(String key) {
     	return textProvider.hasKey(key);
     }
-    
+
     public String getText(String aTextName) {
         return textProvider.getText(aTextName);
     }
@@ -280,6 +285,11 @@ public boolean isValidLocaleString(String localeStr) {
         public boolean isValidLocale(Locale locale) {
             return getLocaleProvider().isValidLocale(locale);
         }
+
+        @Override
+        public Locale toLocale(String localeStr) {
+            return getLocaleProvider().toLocale(localeStr);
+        }
     }
 
     /**
diff --git a/core/src/main/java/org/apache/struts2/interceptor/I18nInterceptor.java b/core/src/main/java/org/apache/struts2/interceptor/I18nInterceptor.java
@@ -24,7 +24,6 @@
 import com.opensymphony.xwork2.inject.Inject;
 import com.opensymphony.xwork2.interceptor.AbstractInterceptor;
 import com.opensymphony.xwork2.util.TextParseUtil;
-import org.apache.commons.lang3.LocaleUtils;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.apache.logging.log4j.message.ParameterizedMessage;
@@ -85,7 +84,7 @@ public void setRequestCookieParameterName(String requestCookieParameterName) {
     }
 
     public void setLocaleStorage(String storageName) {
-        if (storageName == null || "".equals(storageName)) {
+        if (storageName == null || storageName.isEmpty()) {
             this.storage = Storage.ACCEPT_LANGUAGE;
         } else {
             try {
@@ -169,27 +168,21 @@ protected LocaleHandler getLocaleHandler(ActionInvocation invocation) {
     }
 
     /**
-     * Creates a Locale object from the request param, which might
-     * be already a Local or a String
+     * Creates a Locale object from the request param
      *
      * @param requestedLocale the parameter from the request
-     * @return the Locale
+     * @return instance of {@link Locale} or null
      */
-    protected Locale getLocaleFromParam(Object requestedLocale) {
+    protected Locale getLocaleFromParam(String requestedLocale) {
         LocaleProvider localeProvider = localeProviderFactory.createLocaleProvider();
 
         Locale locale = null;
         if (requestedLocale != null) {
-            if (requestedLocale instanceof Locale) {
-                locale = (Locale) requestedLocale;
-            } else {
-                String localeStr = requestedLocale.toString();
-                if (localeProvider.isValidLocaleString(localeStr)) {
-                    locale = LocaleUtils.toLocale(localeStr);
-                } else {
-                    locale = localeProvider.getLocale();
-                }
+            locale = localeProvider.toLocale(requestedLocale);
+            if (locale == null) {
+                locale = localeProvider.getLocale();
             }
+
             if (locale != null) {
                 LOG.debug("Found locale: {}", locale);
             }
@@ -285,7 +278,7 @@ protected AcceptLanguageLocaleHandler(ActionInvocation invocation) {
         @Override
         @SuppressWarnings("rawtypes")
         public Locale find() {
-            if (supportedLocale.size() > 0) {
+            if (!supportedLocale.isEmpty()) {
                 Enumeration locales = actionInvocation.getInvocationContext().getServletRequest().getLocales();
                 while (locales.hasMoreElements()) {
                     Locale locale = (Locale) locales.nextElement();
diff --git a/core/src/test/java/org/apache/struts2/interceptor/I18nInterceptorTest.java b/core/src/test/java/org/apache/struts2/interceptor/I18nInterceptorTest.java
@@ -147,6 +147,16 @@ public void testNotExistingLocale() throws Exception {
         assertEquals(Locale.getDefault(), session.get(I18nInterceptor.DEFAULT_SESSION_ATTRIBUTE)); // should create a locale object
     }
 
+    public void testTrimableLocaleString() throws Exception {
+        prepare(I18nInterceptor.DEFAULT_PARAMETER, "de ");
+
+        interceptor.intercept(mai);
+
+        assertFalse(mai.getInvocationContext().getParameters().get(I18nInterceptor.DEFAULT_PARAMETER).isDefined()); // should have been removed
+        assertNotNull(session.get(I18nInterceptor.DEFAULT_SESSION_ATTRIBUTE)); // should be stored here
+        assertEquals(Locale.GERMAN, session.get(I18nInterceptor.DEFAULT_SESSION_ATTRIBUTE)); // should create a locale object
+    }
+
     public void testWithVariant() throws Exception {
         prepare(I18nInterceptor.DEFAULT_PARAMETER, "ja_JP_JP");
         interceptor.intercept(mai);

Original file line number	Diff line number	Diff line change
`@@ -122,10 +122,15 @@ public boolean isValidLocale(Locale locale) {`
`122`	`122`	`return localeProvider.isValidLocale(locale);`
`123`	`123`	`}`
`124`	`124`
	`125`	`+ @Override`
	`126`	`+ public Locale toLocale(String localeStr) {`
	`127`	`+ return localeProvider.toLocale(localeStr);`
	`128`	`+ }`
	`129`	`+`
`125`	`130`	`public boolean hasKey(String key) {`
`126`	`131`	`return textProvider.hasKey(key);`
`127`	`132`	`}`
`128`		`-`
	`133`	`+`
`129`	`134`	`public String getText(String aTextName) {`
`130`	`135`	`return textProvider.getText(aTextName);`
`131`	`136`	`}`
`@@ -280,6 +285,11 @@ public boolean isValidLocaleString(String localeStr) {`
`280`	`285`	`public boolean isValidLocale(Locale locale) {`
`281`	`286`	`return getLocaleProvider().isValidLocale(locale);`
`282`	`287`	`}`
	`288`	`+`
	`289`	`+ @Override`
	`290`	`+ public Locale toLocale(String localeStr) {`
	`291`	`+ return getLocaleProvider().toLocale(localeStr);`
	`292`	`+ }`
`283`	`293`	`}`
`284`	`294`
`285`	`295`	`/**`