Merge pull request #369 from sebastianfrey/SHIRO-887-do-not-trim-password-strings

[SHIRO-887] Do not trim passwords in FormAuthenticationFilter

Author: bdemers

lang/src/main/java/org/apache/shiro/util/StringUtils.java

@@ -130,7 +130,7 @@ public static boolean startsWithIgnoreCase(String str, String prefix) {
      * <p/>
      * <ol>
      * <li>If the specified <code>String</code> is <code>null</code>, return <code>null</code></li>
-     * <li>If not <code>null</code>, {@link String#trim() trim()} it.</li>
+     * <li>If not <code>null</code>, {@link String#trim() trim()} it, when the trim param is set to <code>true</code>.</li>
      * <li>If the trimmed string is equal to the empty String (i.e. &quot;&quot;), return <code>null</code></li>
      * <li>If the trimmed string is not the empty string, return the trimmed version</li>.
      * </ol>
@@ -139,13 +139,16 @@ public static boolean startsWithIgnoreCase(String str, String prefix) {
      * is returned.
      *
      * @param in the input String to clean.
+     * @param trim specifies whether the input String should be trimmed or not
      * @return a populated-but-trimmed String or <code>null</code> otherwise
      */
-    public static String clean(String in) {
+    public static String clean(String in, boolean trim) {
         String out = in;
 
         if (in != null) {
-            out = in.trim();
+            if (trim) {
+                out = in.trim();
+            }
             if (out.equals(EMPTY_STRING)) {
                 out = null;
             }
@@ -154,6 +157,26 @@ public static String clean(String in) {
         return out;
     }
 
+    /**
+     * Returns a 'cleaned' representation of the specified argument.  'Cleaned' is defined as the following:
+     * <p/>
+     * <ol>
+     * <li>If the specified <code>String</code> is <code>null</code>, return <code>null</code></li>
+     * <li>If not <code>null</code>, {@link String#trim() trim()} it.</li>
+     * <li>If the trimmed string is equal to the empty String (i.e. &quot;&quot;), return <code>null</code></li>
+     * <li>If the trimmed string is not the empty string, return the trimmed version</li>.
+     * </ol>
+     * <p/>
+     * Therefore this method always ensures that any given string has trimmed text, and if it doesn't, <code>null</code>
+     * is returned.
+     *
+     * @param in the input String to clean.
+     * @return a populated-but-trimmed String or <code>null</code> otherwise
+     */
+    public static String clean(String in) {
+        return clean(in, true);
+    }
+
     /**
      * Returns the specified array as a comma-delimited (',') string.
      *

lang/src/test/java/org/apache/shiro/lang/util/StringUtilsTest.java

@@ -0,0 +1,34 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.shiro.lang.util;
+
+import org.apache.shiro.util.StringUtils;
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+
+public class StringUtilsTest {
+
+  @Test
+  public void testClean() {
+    assertEquals(StringUtils.clean(" abc "), "abc");
+    assertEquals(StringUtils.clean(" abc ", true), "abc");
+    assertEquals(StringUtils.clean(" abc ", false), " abc ");
+  }
+}

web/src/main/java/org/apache/shiro/web/filter/authc/FormAuthenticationFilter.java

@@ -220,7 +220,7 @@ protected String getUsername(ServletRequest request) {
     }
 
     protected String getPassword(ServletRequest request) {
-        return WebUtils.getCleanParam(request, getPasswordParam());
+        return WebUtils.getCleanParam(request, getPasswordParam(), false);
     }
 
 

web/src/main/java/org/apache/shiro/web/util/WebUtils.java

@@ -616,7 +616,20 @@ public static boolean isTrue(ServletRequest request, String paramName) {
      * @return the clean param value, or null if the param does not exist or is empty.
      */
     public static String getCleanParam(ServletRequest request, String paramName) {
-        return StringUtils.clean(request.getParameter(paramName));
+        return getCleanParam(request, paramName, true);
+    }
+
+    /**
+     * Convenience method that returns a request parameter value, first running it through
+     * {@link StringUtils#clean(String)}.
+     *
+     * @param request   the servlet request.
+     * @param paramName the parameter name.
+     * @param trim specifies whether the parameter value should be trimmed or not
+     * @return the clean param value, or null if the param does not exist or is empty.
+     */
+    public static String getCleanParam(ServletRequest request, String paramName, boolean trim) {
+        return StringUtils.clean(request.getParameter(paramName), trim);
     }
 
     public static void saveRequest(ServletRequest request) {