svn commit: r1815192 - /ofbiz/ofbiz-framework/trunk/applications/securityext/src/main/java/org/apache/ofbiz/securityext/login/LoginEvents.java

Author: jleroux
Date: Tue Nov 14 09:33:19 2017
New Revision: 1815192

URL: http://svn.apache.org/viewvc?rev=1815192&view=rev
Log:
Improved: Fixing defects reported by FindBugs, package
org.apache.ofbiz.securityext.login.
(OFBIZ-9637)

No functional change.

I prefer to use URLEncoder.encode(reqParam, "UTF-8") rather than ESAPI HTML
encoder for 3 reasons:
*  URLEncoder.encode() is sufficient to answer to HTTP response splitting using
  Percent-encoding (aka  URL encoding)
*  Consistent and simpler code using basic Java
*  Using "UTF-8" is (more than) recommended, see
        https://docs.oracle.com/javase/8/docs/api/java/net/URLEncoder.html
       
I will check what using ESAPI HTML encoder entails. As JavaDOc says "Not doing
so  may introduce incompatibilities." We have 30+ cases, they are maybe OK, but
we need to check...

Modified:
    ofbiz/ofbiz-framework/trunk/applications/securityext/src/main/java/org/apache/ofbiz/securityext/login/LoginEvents.java

Modified: ofbiz/ofbiz-framework/trunk/applications/securityext/src/main/java/org/apache/ofbiz/securityext/login/LoginEvents.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/applications/securityext/src/main/java/org/apache/ofbiz/securityext/login/LoginEvents.java?rev=1815192&r1=1815191&r2=1815192&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/applications/securityext/src/main/java/org/apache/ofbiz/securityext/login/LoginEvents.java (original)
+++ ofbiz/ofbiz-framework/trunk/applications/securityext/src/main/java/org/apache/ofbiz/securityext/login/LoginEvents.java Tue Nov 14 09:33:19 2017
@@ -35,7 +35,6 @@ import org.apache.commons.lang.RandomStr
 import org.apache.ofbiz.base.crypto.HashCrypt;
 import org.apache.ofbiz.base.util.Debug;
 import org.apache.ofbiz.base.util.GeneralException;
-import org.apache.ofbiz.base.util.UtilCodec;
 import org.apache.ofbiz.base.util.UtilFormatOut;
 import org.apache.ofbiz.base.util.UtilHttp;
 import org.apache.ofbiz.base.util.UtilMisc;
@@ -429,7 +428,7 @@ public class LoginEvents {
         return cookieUsername;
     }
 
-    public static void setUsername(HttpServletRequest request, HttpServletResponse response) {
+    public static void setUsername(HttpServletRequest request, HttpServletResponse response) throws UnsupportedEncodingException {
         HttpSession session = request.getSession();
         Delegator delegator = (Delegator) request.getAttribute("delegator");
         String domain = EntityUtilProperties.getPropertyValue("url", "cookie.domain", delegator);
@@ -437,7 +436,7 @@ public class LoginEvents {
         synchronized (session) {
             if (UtilValidate.isEmpty(getUsername(request))) {
                 // create the cookie and send it back
-                String usernameParam = UtilCodec.getEncoder("html").encode(request.getParameter("USERNAME"));
+                String usernameParam = URLEncoder.encode(request.getParameter("USERNAME"), "UTF-8");
                 Cookie cookie = new Cookie(usernameCookieName, usernameParam);
                 cookie.setMaxAge(60 * 60 * 24 * 365);
                 cookie.setPath("/");

[Jacques Le Roux]

Le 14/11/2017 à 10:33, [hidden email] a écrit :

> Author: jleroux
> Date: Tue Nov 14 09:33:19 2017
> New Revision: 1815192
>
> URL:http://svn.apache.org/viewvc?rev=1815192&view=rev
> Log:
> Improved: Fixing defects reported by FindBugs, package
> org.apache.ofbiz.securityext.login.
> (OFBIZ-9637)
>
> No functional change.
>
> I prefer to use URLEncoder.encode(reqParam, "UTF-8") rather than ESAPI HTML
> encoder for 3 reasons:
> *  URLEncoder.encode() is sufficient to answer to HTTP response splitting using
>     Percent-encoding (aka  URL encoding)
> *  Consistent and simpler code using basic Java
> *  Using "UTF-8" is (more than) recommended, see
> https://docs.oracle.com/javase/8/docs/api/java/net/URLEncoder.html
>
> I will check what using ESAPI HTML encoder entails. As JavaDOc says "Not doing
> so  may introduce incompatibilities." We have 30+ cases, they are maybe OK, but
> we need to check...

Among the 46 cases, I see no problems since it's only used in the context of widgets (mostly content wrappers) and only for HTML, not request parameters.

Jacques