fix some sonar issue

apache · Jun 29, 2018 · 9181823 · 9181823
1 parent 52e8d46
commit 9181823
Show file tree

Hide file tree

Showing 7 changed files with 232 additions and 77 deletions.
diff --git a/core/src/main/java/org/apache/struts2/StrutsConstants.java b/core/src/main/java/org/apache/struts2/StrutsConstants.java
@@ -270,6 +270,11 @@ public final class StrutsConstants {
 
     public static final String STRUTS_EXPRESSION_PARSER = "struts.expression.parser";
 
+    /** namespaces names' whitelist **/
+    public static final String STRUTS_ALLOWED_NAMESPACE_NAMES = "struts.allowed.namespace.names";
+    /** default namespace name to use when namespace didn't match the whitelist **/
+    public static final String STRUTS_DEFAULT_NAMESPACE_NAME = "struts.default.namespace.name";
+
     /** actions names' whitelist **/
     public static final String STRUTS_ALLOWED_ACTION_NAMES = "struts.allowed.action.names";
     /** default action name to use when action didn't match the whitelist **/

diff --git a/core/src/main/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapper.java b/core/src/main/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapper.java
@@ -120,6 +120,10 @@ public class DefaultActionMapper implements ActionMapper {
     protected boolean allowSlashesInActionNames = false;
     protected boolean alwaysSelectFullNamespace = false;
     protected PrefixTrie prefixTrie = null;
+
+    protected Pattern allowedNamespaceNames = Pattern.compile("[a-zA-Z0-9._/\\-]*");
+    protected String defaultNamespaceName = "/";
+
     protected Pattern allowedActionNames = Pattern.compile("[a-zA-Z0-9._!/\\-]*");
     protected String defaultActionName = "index";
 
@@ -166,8 +170,8 @@ public void execute(final String key, ActionMapping mapping) {
                                 }
                             }
                             if (!allowSlashesInActionNames && !allowActionCrossNamespaceAccess) {
-                                if (actionName.lastIndexOf("/") != -1) {
-                                    actionName = actionName.substring(actionName.lastIndexOf("/") + 1);
+                                if (actionName.lastIndexOf('/') != -1) {
+                                    actionName = actionName.substring(actionName.lastIndexOf('/') + 1);
                                 }
                             }
                             mapping.setName(actionName);
@@ -205,6 +209,16 @@ public void setAlwaysSelectFullNamespace(String val) {
         this.alwaysSelectFullNamespace = "true".equals(val);
     }
 
+    @Inject(value = StrutsConstants.STRUTS_ALLOWED_NAMESPACE_NAMES, required = false)
+    public void setAllowedNamespaceNames(String allowedNamespaceNames) {
+        this.allowedNamespaceNames = Pattern.compile(allowedNamespaceNames);
+    }
+
+    @Inject(value = StrutsConstants.STRUTS_DEFAULT_NAMESPACE_NAME, required = false)
+    public void setDefaultNamespaceName(String defaultNamespaceName) {
+        this.defaultNamespaceName = defaultNamespaceName;
+    }
+
     @Inject(value = StrutsConstants.STRUTS_ALLOWED_ACTION_NAMES, required = false)
     public void setAllowedActionNames(String allowedActionNames) {
         this.allowedActionNames = Pattern.compile(allowedActionNames);
@@ -274,7 +288,7 @@ public ActionMapping getMapping(HttpServletRequest request, ConfigurationManager
         ActionMapping mapping = new ActionMapping();
         String uri = RequestUtils.getUri(request);
 
-        int indexOfSemicolon = uri.indexOf(";");
+        int indexOfSemicolon = uri.indexOf(';');
         uri = (indexOfSemicolon > -1) ? uri.substring(0, indexOfSemicolon) : uri;
 
         uri = dropExtension(uri, mapping);
@@ -294,7 +308,7 @@ protected ActionMapping parseActionName(ActionMapping mapping) {
         if (allowDynamicMethodCalls) {
             // handle "name!method" convention.
             String name = mapping.getName();
-            int exclamation = name.lastIndexOf("!");
+            int exclamation = name.lastIndexOf('!');
             if (exclamation != -1) {
                 mapping.setName(name.substring(0, exclamation));
 
@@ -343,7 +357,7 @@ public void handleSpecialParameters(HttpServletRequest request, ActionMapping ma
      */
     protected void parseNameAndNamespace(String uri, ActionMapping mapping, ConfigurationManager configManager) {
         String namespace, name;
-        int lastSlash = uri.lastIndexOf("/");
+        int lastSlash = uri.lastIndexOf('/');
         if (lastSlash == -1) {
             namespace = "";
             name = uri;
@@ -391,10 +405,28 @@ protected void parseNameAndNamespace(String uri, ActionMapping mapping, Configur
             }
         }
 
-        mapping.setNamespace(namespace);
+        mapping.setNamespace(cleanupNamespaceName(namespace));
         mapping.setName(cleanupActionName(name));
     }
 
+    /**
+     * Checks namespace name against allowed pattern if not matched returns default namespace
+     *
+     * @param rawNamespace name extracted from URI
+     * @return safe namespace name
+     */
+    protected String cleanupNamespaceName(final String rawNamespace) {
+        if (allowedNamespaceNames.matcher(rawNamespace).matches()) {
+            return rawNamespace;
+        } else {
+            LOG.warn(
+                "{} did not match allowed namespace names {} - default namespace {} will be used!",
+                rawNamespace, allowedActionNames, defaultActionName
+            );
+            return defaultNamespaceName;
+        }
+    }
+
     /**
      * Checks action name against allowed pattern if not matched returns default action name
      *