Issue #9762 ee9 double parses cookies (#9764)

* Issue #9762 ee9 double parses cookies

* implemented the TODOs to cache servlet cookies

---------

Co-authored-by: gregw <gregw@webtide.com>
This commit is contained in:
Jan Bartel 2023-05-12 17:14:44 +02:00 committed by GitHub
parent 87a430d148
commit 2261ce9a50
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 206 additions and 53 deletions

View File

@ -144,4 +144,13 @@ public class CookieCache implements CookieParser.Handler
return _cookieList == null ? Collections.emptyList() : _cookieList;
}
/**
* Replace the cookie list with
* @param cookies The replacement cookie list, which must be equal to the existing list
*/
public void replaceCookieList(List<HttpCookie> cookies)
{
assert _cookieList.equals(cookies);
_cookieList = cookies;
}
}

View File

@ -22,6 +22,7 @@ import java.nio.charset.Charset;
import java.nio.charset.StandardCharsets;
import java.nio.charset.UnsupportedCharsetException;
import java.security.Principal;
import java.util.AbstractList;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
@ -56,6 +57,7 @@ import jakarta.servlet.http.HttpUpgradeHandler;
import jakarta.servlet.http.Part;
import jakarta.servlet.http.PushBuilder;
import org.eclipse.jetty.http.BadMessageException;
import org.eclipse.jetty.http.CookieCache;
import org.eclipse.jetty.http.CookieCompliance;
import org.eclipse.jetty.http.HttpCookie;
import org.eclipse.jetty.http.HttpException;
@ -237,21 +239,14 @@ public class ServletApiRequest implements HttpServletRequest
List<HttpCookie> httpCookies = Request.getCookies(getServletContextRequest());
if (httpCookies.isEmpty())
return null;
return httpCookies.stream()
.map(this::convertCookie)
.toArray(Cookie[]::new);
}
if (httpCookies instanceof ServletCookieList servletCookieList)
return servletCookieList.getServletCookies();
public Cookie convertCookie(HttpCookie cookie)
{
Cookie result = new Cookie(cookie.getName(), cookie.getValue());
//RFC2965 defines the cookie header as supporting path and domain but RFC6265 permits only name=value
if (CookieCompliance.RFC2965.equals(getServletContextRequest().getConnectionMetaData().getHttpConfiguration().getRequestCookieCompliance()))
{
result.setPath(cookie.getPath());
result.setDomain(cookie.getDomain());
}
return result;
ServletCookieList servletCookieList = new ServletCookieList(httpCookies, getServletContextRequest().getConnectionMetaData().getHttpConfiguration().getRequestCookieCompliance());
_request.setAttribute(Request.COOKIE_ATTRIBUTE, servletCookieList);
if (_request.getComponents().getCache().getAttribute(Request.CACHE_ATTRIBUTE) instanceof CookieCache cookieCache)
cookieCache.replaceCookieList(servletCookieList);
return servletCookieList.getServletCookies();
}
@Override
@ -1276,4 +1271,51 @@ public class ServletApiRequest implements HttpServletRequest
throw new HttpException.IllegalArgumentException(HttpStatus.BAD_REQUEST_400, "Ambiguous URI encoding");
}
}
/**
* Extended list of HttpCookies that converts and caches a servlet Cookie array.
*/
private static class ServletCookieList extends AbstractList<HttpCookie>
{
private final List<HttpCookie> _httpCookies;
private final Cookie[] _cookies;
ServletCookieList(List<HttpCookie> httpCookies, CookieCompliance compliance)
{
_httpCookies = httpCookies;
_cookies = new Cookie[_httpCookies.size()];
int i = 0;
for (HttpCookie httpCookie : _httpCookies)
_cookies[i++] = convertCookie(httpCookie, compliance);
}
@Override
public HttpCookie get(int index)
{
return _httpCookies.get(index);
}
public Cookie[] getServletCookies()
{
return _cookies;
}
@Override
public int size()
{
return _cookies.length;
}
private static Cookie convertCookie(HttpCookie cookie, CookieCompliance compliance)
{
Cookie result = new Cookie(cookie.getName(), cookie.getValue());
//RFC2965 defines the cookie header as supporting path and domain but RFC6265 permits only name=value
if (CookieCompliance.RFC2965.equals(compliance))
{
result.setPath(cookie.getPath());
result.setDomain(cookie.getDomain());
}
return result;
}
}
}

View File

@ -14,9 +14,13 @@
package org.eclipse.jetty.ee10.servlet;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicReference;
import jakarta.servlet.http.Cookie;
import jakarta.servlet.http.HttpServlet;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
@ -32,9 +36,12 @@ import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Test;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.sameInstance;
public class RequestTest
{
@ -222,4 +229,57 @@ public class RequestTest
assertThat("request.getRequestURI", resultRequestURI.get(), is("/test/path%20info/foo%2cbar"));
assertThat("request.getPathInfo", resultPathInfo.get(), is("/test/path info/foo,bar"));
}
@Test
public void testCachedServletCookies() throws Exception
{
final List<Cookie> cookieHistory = new ArrayList<>();
startServer(new HttpServlet()
{
@Override
protected void service(HttpServletRequest request, HttpServletResponse resp)
{
Cookie[] cookies = request.getCookies();
if (cookies != null)
cookieHistory.addAll(Arrays.asList(cookies));
}
});
try (LocalConnector.LocalEndPoint connection = _connector.connect())
{
connection.addInput("""
GET /one HTTP/1.1\r
Host: myhost\r
Cookie: name1=value1; name2=value2\r
\r
GET /two HTTP/1.1\r
Host: myhost\r
Cookie: name1=value1; name2=value2\r
\r
GET /three HTTP/1.1\r
Host: myhost\r
Cookie: name1=value1; name3=value3\r
Connection: close\r
\r
""");
assertThat(connection.getResponse(), containsString(" 200 OK"));
assertThat(connection.getResponse(), containsString(" 200 OK"));
assertThat(connection.getResponse(), containsString(" 200 OK"));
}
assertThat(cookieHistory.size(), is(6));
assertThat(cookieHistory.stream().map(c -> c.getName() + "=" + c.getValue()).toList(), contains(
"name1=value1",
"name2=value2",
"name1=value1",
"name2=value2",
"name1=value1",
"name3=value3"
));
assertThat(cookieHistory.get(0), sameInstance(cookieHistory.get(2)));
assertThat(cookieHistory.get(2), not(sameInstance(cookieHistory.get(4))));
}
}

View File

@ -27,7 +27,9 @@ import java.nio.charset.Charset;
import java.nio.charset.StandardCharsets;
import java.nio.charset.UnsupportedCharsetException;
import java.security.Principal;
import java.util.AbstractList;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.EnumSet;
@ -64,6 +66,8 @@ import jakarta.servlet.http.Part;
import jakarta.servlet.http.PushBuilder;
import org.eclipse.jetty.http.BadMessageException;
import org.eclipse.jetty.http.ComplianceViolation;
import org.eclipse.jetty.http.CookieCache;
import org.eclipse.jetty.http.CookieCompliance;
import org.eclipse.jetty.http.HttpCompliance;
import org.eclipse.jetty.http.HttpCookie;
import org.eclipse.jetty.http.HttpField;
@ -167,14 +171,12 @@ public class Request implements HttpServletRequest
private ServletPathMapping _servletPathMapping;
private Object _asyncNotSupportedSource = null;
private boolean _secure;
private boolean _cookiesExtracted = false;
private boolean _handled = false;
private boolean _contentParamsExtracted;
private Attributes _attributes;
private Authentication _authentication;
private String _contentType;
private String _characterEncoding;
private Cookies _cookies;
private DispatcherType _dispatcherType;
private int _inputState = INPUT_NONE;
private BufferedReader _reader;
@ -743,32 +745,22 @@ public class Request implements HttpServletRequest
@Override
public Cookie[] getCookies()
{
MetaData.Request metadata = _metaData;
if (metadata == null || _cookiesExtracted)
{
if (_cookies == null || _cookies.getCookies().length == 0)
return null;
return _cookies.getCookies();
}
_cookiesExtracted = true;
for (HttpField field : metadata.getHttpFields())
{
if (field.getHeader() == HttpHeader.COOKIE)
{
if (_cookies == null)
_cookies = new Cookies(getHttpChannel().getHttpConfiguration().getRequestCookieCompliance(), getComplianceViolationListener());
_cookies.addCookieField(field.getValue());
}
}
//Javadoc for Request.getCookies() stipulates null for no cookies
if (_cookies == null || _cookies.getCookies().length == 0)
ContextHandler.CoreContextRequest coreRequest = getCoreRequest();
if (coreRequest == null)
return null;
return _cookies.getCookies();
List<HttpCookie> httpCookies = org.eclipse.jetty.server.Request.getCookies(coreRequest);
if (httpCookies.isEmpty())
return null;
if (httpCookies instanceof ServletCookieList servletCookieList)
return servletCookieList.getServletCookies();
ServletCookieList servletCookieList = new ServletCookieList(httpCookies, coreRequest.getConnectionMetaData().getHttpConfiguration().getRequestCookieCompliance());
coreRequest.setAttribute(org.eclipse.jetty.server.Request.COOKIE_ATTRIBUTE, servletCookieList);
if (coreRequest.getComponents().getCache().getAttribute(org.eclipse.jetty.server.Request.CACHE_ATTRIBUTE) instanceof CookieCache cookieCache)
cookieCache.replaceCookieList(servletCookieList);
return servletCookieList.getServletCookies();
}
@Override
@ -1555,15 +1547,12 @@ public class Request implements HttpServletRequest
_servletPathMapping = null;
_asyncNotSupportedSource = null;
_secure = false;
_cookiesExtracted = false;
_handled = false;
_contentParamsExtracted = false;
_attributes = null;
setAuthentication(Authentication.NOT_CHECKED);
_contentType = null;
_characterEncoding = null;
if (_cookies != null)
_cookies.reset();
_dispatcherType = null;
_inputState = INPUT_NONE;
// _reader can be reused
@ -1770,16 +1759,6 @@ public class Request implements HttpServletRequest
_contentType = contentType;
}
/**
* @param cookies The cookies to set.
*/
public void setCookies(Cookie[] cookies)
{
if (_cookies == null)
_cookies = new Cookies(getHttpChannel().getHttpConfiguration().getRequestCookieCompliance(), getComplianceViolationListener());
_cookies.setCookies(cookies);
}
public void setDispatcherType(DispatcherType type)
{
_dispatcherType = type;
@ -2162,4 +2141,67 @@ public class Request implements HttpServletRequest
// which we recover from the IncludeAttributes wrapper.
return findServletPathMapping();
}
/**
* Extended list of HttpCookies that converts and caches a servlet Cookie array.
*/
private static class ServletCookieList extends AbstractList<HttpCookie>
{
private final List<HttpCookie> _httpCookies;
private final Cookie[] _cookies;
ServletCookieList(List<HttpCookie> httpCookies, CookieCompliance compliance)
{
_httpCookies = httpCookies;
Cookie[] cookies = new Cookie[_httpCookies.size()];
int i = 0;
for (HttpCookie httpCookie : _httpCookies)
{
Cookie cookie = convertCookie(httpCookie, compliance);
if (cookie == null)
cookies = Arrays.copyOf(cookies, cookies.length - 1);
else
cookies[i++] = cookie;
}
_cookies = cookies;
}
@Override
public HttpCookie get(int index)
{
return _httpCookies.get(index);
}
public Cookie[] getServletCookies()
{
return _cookies;
}
@Override
public int size()
{
return _cookies.length;
}
private static Cookie convertCookie(HttpCookie cookie, CookieCompliance compliance)
{
try
{
Cookie result = new Cookie(cookie.getName(), cookie.getValue());
//RFC2965 defines the cookie header as supporting path and domain but RFC6265 permits only name=value
if (CookieCompliance.RFC2965.equals(compliance))
{
result.setPath(cookie.getPath());
result.setDomain(cookie.getDomain());
}
return result;
}
catch (Exception ignore)
{
if (LOG.isDebugEnabled())
LOG.debug("Bad Cookie", ignore);
}
return null;
}
}
}