search

digitalfondue / lavagna

Lavagna: issue tracker/project management tool
http://lavagna.io
GNU General Public License v3.0
636 stars 110 forks source link

Fix java.lang.IllegalArgumentException when setting the Remember Me cookie under Tomcat #81

Closed sebastianopilla closed 7 years ago

sebastianopilla commented 7 years ago

The current code in UserSession.java is using a comma (",") to separate the user id from the user token in the remember-me cookie: unfortunately the comma is not a valid character for a cookie value according to RFC 6265 and Tomcat rejects it with a 500 error and the following exception:

java.lang.IllegalArgumentException: An invalid character [44] was present in the Cookie value org.apache.tomcat.util.http.Rfc6265CookieProcessor.validateCookieValue(Rfc6265CookieProcessor.java:182) org.apache.tomcat.util.http.Rfc6265CookieProcessor.generateHeader(Rfc6265CookieProcessor.java:115) org.apache.catalina.connector.Response.generateCookieString(Response.java:999) org.apache.catalina.connector.Response.addCookie(Response.java:947) org.apache.catalina.connector.ResponseFacade.addCookie(ResponseFacade.java:386) io.lavagna.web.helper.UserSession.addRememberMeCookie(UserSession.java:160) io.lavagna.web.helper.UserSession.setUser(UserSession.java:135) io.lavagna.web.helper.UserSession.setUser(UserSession.java:146) io.lavagna.config.WebSecurityConfig$2.setUser(WebSecurityConfig.java:135) io.lavagna.web.security.login.PasswordLogin.doAction(PasswordLogin.java:63) io.lavagna.web.security.SecurityConfiguration$LoginUrlMatcher.doAction(SecurityConfiguration.java:256) io.lavagna.web.security.SecurityFilter.handleWith(SecurityFilter.java:85) io.lavagna.web.security.SecurityFilter.doFilterInternal(SecurityFilter.java:69) io.lavagna.web.security.AbstractBaseFilter.doFilter(AbstractBaseFilter.java:46) io.lavagna.web.security.AnonymousUserFilter.doFilterInternal(AnonymousUserFilter.java:59) io.lavagna.web.security.AbstractBaseFilter.doFilter(AbstractBaseFilter.java:46) io.lavagna.web.security.RememberMeFilter.doFilterInternal(RememberMeFilter.java:55) io.lavagna.web.security.AbstractBaseFilter.doFilter(AbstractBaseFilter.java:46) io.lavagna.web.security.CSFRFilter.doFilterInternal(CSFRFilter.java:62) io.lavagna.web.security.AbstractBaseFilter.doFilter(AbstractBaseFilter.java:46) io.lavagna.web.security.HSTSFilter.doFilterInternal(HSTSFilter.java:96) io.lavagna.web.security.AbstractBaseFilter.doFilter(AbstractBaseFilter.java:46)

This pull requests simply replaces the comma with a slash, which hopefully won't ever be found in a user id or user token.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.01%) to 69.684% when pulling 8fce777e2ced83c4cacf36e623370286b5b86f75 on sebastianopilla:master into 826d51dd5bbbb97b85d1b4798b006563d6ec2dfe on digitalfondue:master.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.01%) to 69.684% when pulling 8fce777e2ced83c4cacf36e623370286b5b86f75 on sebastianopilla:master into 826d51dd5bbbb97b85d1b4798b006563d6ec2dfe on digitalfondue:master.

syjer commented 7 years ago

Hi @sebastianopilla , thank you for the PR :+1: .

We will do a release as soon as possible.