Issue #4247 use context default for samesite cookie attribute (#4512)

* Issue #4247 use context default for samesite cookie attribute

Signed-off-by: Jan Bartel <janb@webtide.com>
This commit is contained in:
Jan Bartel 2020-01-29 11:05:35 +01:00 committed by GitHub
parent 56754f4a29
commit 9850c29839
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 528 additions and 4 deletions

View File

@ -19,14 +19,21 @@
package org.eclipse.jetty.http;
import java.util.List;
import java.util.Locale;
import java.util.concurrent.TimeUnit;
import javax.servlet.ServletContext;
import org.eclipse.jetty.util.QuotedStringTokenizer;
import org.eclipse.jetty.util.StringUtil;
import org.eclipse.jetty.util.log.Log;
import org.eclipse.jetty.util.log.Logger;
// TODO consider replacing this with java.net.HttpCookie (once it supports RFC6265)
public class HttpCookie
{
private static final Logger LOG = Log.getLogger(HttpCookie.class);
private static final String __COOKIE_DELIM = "\",;\\ \t";
private static final String __01Jan1970_COOKIE = DateGenerator.formatCookieDate(0).trim();
@ -41,6 +48,11 @@ public class HttpCookie
public static final String SAME_SITE_NONE_COMMENT = SAME_SITE_COMMENT + "NONE__";
public static final String SAME_SITE_LAX_COMMENT = SAME_SITE_COMMENT + "LAX__";
public static final String SAME_SITE_STRICT_COMMENT = SAME_SITE_COMMENT + "STRICT__";
/**
* Name of context attribute with default SameSite cookie value
*/
public static final String SAME_SITE_DEFAULT_ATTRIBUTE = "org.eclipse.jetty.cookie.sameSiteDefault";
public enum SameSite
{
@ -70,7 +82,7 @@ public class HttpCookie
private final boolean _httpOnly;
private final long _expiration;
private final SameSite _sameSite;
public HttpCookie(String name, String value)
{
this(name, value, -1);
@ -448,6 +460,42 @@ public class HttpCookie
return null;
}
/**
* Get the default value for SameSite cookie attribute, if one
* has been set for the given context.
*
* @param context the context to check for default SameSite value
* @return the default SameSite value or null if one does not exist
* @throws IllegalStateException if the default value is not a permitted value
*/
public static SameSite getSameSiteDefault(ServletContext context)
{
if (context == null)
return null;
Object o = context.getAttribute(SAME_SITE_DEFAULT_ATTRIBUTE);
if (o == null)
{
if (LOG.isDebugEnabled())
LOG.debug("No default value for SameSite");
return null;
}
if (o instanceof SameSite)
return (SameSite)o;
try
{
SameSite samesite = Enum.valueOf(SameSite.class, o.toString().trim().toUpperCase(Locale.ENGLISH));
context.setAttribute(SAME_SITE_DEFAULT_ATTRIBUTE, samesite);
return samesite;
}
catch (Exception e)
{
LOG.warn("Bad default value {} for SameSite", o);
throw new IllegalStateException(e);
}
}
public static String getCommentWithoutAttributes(String comment)
{
if (comment == null)

View File

@ -18,8 +18,30 @@
package org.eclipse.jetty.http;
import java.io.InputStream;
import java.net.MalformedURLException;
import java.net.URL;
import java.util.Collections;
import java.util.Enumeration;
import java.util.EventListener;
import java.util.HashMap;
import java.util.Map;
import java.util.Set;
import java.util.stream.Stream;
import javax.servlet.Filter;
import javax.servlet.FilterRegistration;
import javax.servlet.RequestDispatcher;
import javax.servlet.Servlet;
import javax.servlet.ServletContext;
import javax.servlet.ServletException;
import javax.servlet.ServletRegistration;
import javax.servlet.ServletRegistration.Dynamic;
import javax.servlet.SessionCookieConfig;
import javax.servlet.SessionTrackingMode;
import javax.servlet.descriptor.JspConfigDescriptor;
import org.eclipse.jetty.http.HttpCookie.SameSite;
import org.hamcrest.Matchers;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
@ -40,7 +62,340 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
public class HttpCookieTest
{
public static class TestServletContext implements ServletContext
{
private Map<String, Object> _attributes = new HashMap<>();
@Override
public String getContextPath()
{
return null;
}
@Override
public ServletContext getContext(String uripath)
{
return null;
}
@Override
public int getMajorVersion()
{
return 0;
}
@Override
public int getMinorVersion()
{
return 0;
}
@Override
public int getEffectiveMajorVersion()
{
return 0;
}
@Override
public int getEffectiveMinorVersion()
{
return 0;
}
@Override
public String getMimeType(String file)
{
return null;
}
@Override
public Set<String> getResourcePaths(String path)
{
return null;
}
@Override
public URL getResource(String path) throws MalformedURLException
{
return null;
}
@Override
public InputStream getResourceAsStream(String path)
{
return null;
}
@Override
public RequestDispatcher getRequestDispatcher(String path)
{
return null;
}
@Override
public RequestDispatcher getNamedDispatcher(String name)
{
return null;
}
@Override
public Servlet getServlet(String name) throws ServletException
{
return null;
}
@Override
public Enumeration<Servlet> getServlets()
{
return null;
}
@Override
public Enumeration<String> getServletNames()
{
return null;
}
@Override
public void log(String msg)
{
}
@Override
public void log(Exception exception, String msg)
{
}
@Override
public void log(String message, Throwable throwable)
{
}
@Override
public String getRealPath(String path)
{
return null;
}
@Override
public String getServerInfo()
{
return null;
}
@Override
public String getInitParameter(String name)
{
return null;
}
@Override
public Enumeration<String> getInitParameterNames()
{
return null;
}
@Override
public boolean setInitParameter(String name, String value)
{
return false;
}
@Override
public Object getAttribute(String name)
{
return _attributes.get(name);
}
@Override
public Enumeration<String> getAttributeNames()
{
return Collections.enumeration(_attributes.keySet());
}
@Override
public void setAttribute(String name, Object object)
{
_attributes.put(name,object);
}
@Override
public void removeAttribute(String name)
{
_attributes.remove(name);
}
@Override
public String getServletContextName()
{
return null;
}
@Override
public Dynamic addServlet(String servletName, String className)
{
return null;
}
@Override
public Dynamic addServlet(String servletName, Servlet servlet)
{
return null;
}
@Override
public Dynamic addServlet(String servletName, Class<? extends Servlet> servletClass)
{
return null;
}
@Override
public <T extends Servlet> T createServlet(Class<T> clazz) throws ServletException
{
return null;
}
@Override
public ServletRegistration getServletRegistration(String servletName)
{
return null;
}
@Override
public Map<String, ? extends ServletRegistration> getServletRegistrations()
{
return null;
}
@Override
public javax.servlet.FilterRegistration.Dynamic addFilter(String filterName, String className)
{
return null;
}
@Override
public javax.servlet.FilterRegistration.Dynamic addFilter(String filterName, Filter filter)
{
return null;
}
@Override
public javax.servlet.FilterRegistration.Dynamic addFilter(String filterName, Class<? extends Filter> filterClass)
{
return null;
}
@Override
public <T extends Filter> T createFilter(Class<T> clazz) throws ServletException
{
return null;
}
@Override
public FilterRegistration getFilterRegistration(String filterName)
{
return null;
}
@Override
public Map<String, ? extends FilterRegistration> getFilterRegistrations()
{
return null;
}
@Override
public SessionCookieConfig getSessionCookieConfig()
{
return null;
}
@Override
public void setSessionTrackingModes(Set<SessionTrackingMode> sessionTrackingModes)
{
}
@Override
public Set<SessionTrackingMode> getDefaultSessionTrackingModes()
{
return null;
}
@Override
public Set<SessionTrackingMode> getEffectiveSessionTrackingModes()
{
return null;
}
@Override
public void addListener(String className)
{
}
@Override
public <T extends EventListener> void addListener(T t)
{
}
@Override
public void addListener(Class<? extends EventListener> listenerClass)
{
}
@Override
public <T extends EventListener> T createListener(Class<T> clazz) throws ServletException
{
return null;
}
@Override
public JspConfigDescriptor getJspConfigDescriptor()
{
return null;
}
@Override
public ClassLoader getClassLoader()
{
return null;
}
@Override
public void declareRoles(String... roleNames)
{
}
@Override
public String getVirtualServerName()
{
return null;
}
}
@Test
public void testDefaultSameSite()
{
TestServletContext context = new TestServletContext();
//test null value for default
assertNull(HttpCookie.getSameSiteDefault(context));
//test good value for default as SameSite enum
context.setAttribute(HttpCookie.SAME_SITE_DEFAULT_ATTRIBUTE, SameSite.LAX);
assertEquals(SameSite.LAX, HttpCookie.getSameSiteDefault(context));
//test good value for default as String
context.setAttribute(HttpCookie.SAME_SITE_DEFAULT_ATTRIBUTE, "NONE");
assertEquals(SameSite.NONE, HttpCookie.getSameSiteDefault(context));
//test case for default as String
context.setAttribute(HttpCookie.SAME_SITE_DEFAULT_ATTRIBUTE, "sTrIcT");
assertEquals(SameSite.STRICT, HttpCookie.getSameSiteDefault(context));
//test bad value for default as String
context.setAttribute(HttpCookie.SAME_SITE_DEFAULT_ATTRIBUTE, "fooBAR");
assertThrows(IllegalStateException.class,
() -> HttpCookie.getSameSiteDefault(context));
}
@Test
public void testConstructFromSetCookie()
{

View File

@ -188,13 +188,42 @@ public class Response implements HttpServletResponse
{
if (StringUtil.isBlank(cookie.getName()))
throw new IllegalArgumentException("Cookie.name cannot be blank/null");
// add the set cookie
_fields.add(new SetCookieHttpField(cookie, getHttpChannel().getHttpConfiguration().getResponseCookieCompliance()));
_fields.add(new SetCookieHttpField(checkSameSite(cookie), getHttpChannel().getHttpConfiguration().getResponseCookieCompliance()));
// Expire responses with set-cookie headers so they do not get cached.
_fields.put(__EXPIRES_01JAN1970);
}
/**
* Check that samesite is set on the cookie. If not, use a
* context default value, if one has been set.
*
* @param cookie the cookie to check
* @return either the original cookie, or a new one that has the samesit default set
*/
private HttpCookie checkSameSite(HttpCookie cookie)
{
if (cookie == null || cookie.getSameSite() != null)
return cookie;
//sameSite is not set, use the default configured for the context, if one exists
SameSite contextDefault = HttpCookie.getSameSiteDefault(_channel.getRequest().getServletContext());
if (contextDefault == null)
return cookie; //no default set
return new HttpCookie(cookie.getName(),
cookie.getValue(),
cookie.getDomain(),
cookie.getPath(),
cookie.getMaxAge(),
cookie.isHttpOnly(),
cookie.isSecure(),
cookie.getComment(),
cookie.getVersion(),
contextDefault);
}
/**
* Replace (or add) a cookie.
@ -237,7 +266,7 @@ public class Response implements HttpServletResponse
else if (!cookie.getPath().equals(oldCookie.getPath()))
continue;
i.set(new SetCookieHttpField(cookie, compliance));
i.set(new SetCookieHttpField(checkSameSite(cookie), compliance));
return;
}
}

View File

@ -32,10 +32,13 @@ import java.nio.ByteBuffer;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Enumeration;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.stream.Stream;
import javax.servlet.RequestDispatcher;
import javax.servlet.ServletException;
import javax.servlet.ServletOutputStream;
@ -963,6 +966,32 @@ public class ResponseTest
assertEquals("name=value; Path=/path; Domain=domain; Secure; HttpOnly", set);
}
@Test
public void testAddCookieSameSiteDefault() throws Exception
{
Response response = getResponse();
TestServletContextHandler context = new TestServletContextHandler();
_channel.getRequest().setContext(context.getServletContext());
context.setAttribute(HttpCookie.SAME_SITE_DEFAULT_ATTRIBUTE, HttpCookie.SameSite.STRICT);
Cookie cookie = new Cookie("name", "value");
cookie.setDomain("domain");
cookie.setPath("/path");
cookie.setSecure(true);
cookie.setComment("comment__HTTP_ONLY__");
response.addCookie(cookie);
String set = response.getHttpFields().get("Set-Cookie");
assertEquals("name=value; Path=/path; Domain=domain; Secure; HttpOnly; SameSite=Strict", set);
response.getHttpFields().remove("Set-Cookie");
//test bad default samesite value
context.setAttribute(HttpCookie.SAME_SITE_DEFAULT_ATTRIBUTE, "FooBar");
assertThrows(IllegalStateException.class,
() -> response.addCookie(cookie));
}
@Test
public void testAddCookieComplianceRFC2965() throws Exception
@ -1102,6 +1131,23 @@ public class ResponseTest
List<String> actual = Collections.list(response.getHttpFields().getValues("Set-Cookie"));
assertThat("HttpCookie order", actual, hasItems(expected));
}
@Test
public void testReplaceHttpCookieSameSite()
{
Response response = getResponse();
TestServletContextHandler context = new TestServletContextHandler();
context.setAttribute(HttpCookie.SAME_SITE_DEFAULT_ATTRIBUTE, "LAX");
_channel.getRequest().setContext(context.getServletContext());
//replace with no prior does an add
response.replaceCookie(new HttpCookie("Foo", "123456"));
String set = response.getHttpFields().get("Set-Cookie");
assertEquals("Foo=123456; SameSite=Lax", set);
//check replacement
response.replaceCookie(new HttpCookie("Foo", "other"));
set = response.getHttpFields().get("Set-Cookie");
assertEquals("Foo=other; SameSite=Lax", set);
}
@Test
public void testReplaceParsedHttpCookie()
@ -1127,6 +1173,20 @@ public class ResponseTest
actual = Collections.list(response.getHttpFields().getValues("Set-Cookie"));
assertThat(actual, hasItems(new String[]{"Foo=replaced; Path=/path; Domain=Bah"}));
}
@Test
public void testReplaceParsedHttpCookieSiteDefault()
{
Response response = getResponse();
TestServletContextHandler context = new TestServletContextHandler();
context.setAttribute(HttpCookie.SAME_SITE_DEFAULT_ATTRIBUTE, "LAX");
_channel.getRequest().setContext(context.getServletContext());
response.addHeader(HttpHeader.SET_COOKIE.asString(), "Foo=123456");
response.replaceCookie(new HttpCookie("Foo", "value"));
String set = response.getHttpFields().get("Set-Cookie");
assertEquals("Foo=value; SameSite=Lax", set);
}
@Test
public void testFlushAfterFullContent() throws Exception
@ -1155,4 +1215,36 @@ public class ResponseTest
super(handler, new SessionData(id, "", "0.0.0.0", 0, 0, 0, 300));
}
}
private static class TestServletContextHandler extends ContextHandler
{
private class Context extends ContextHandler.Context
{
private Map<String, Object> _attributes = new HashMap<>();
@Override
public Object getAttribute(String name)
{
return _attributes.get(name);
}
@Override
public Enumeration<String> getAttributeNames()
{
return Collections.enumeration(_attributes.keySet());
}
@Override
public void setAttribute(String name, Object object)
{
_attributes.put(name,object);
}
@Override
public void removeAttribute(String name)
{
_attributes.remove(name);
}
}
}
}