From f154151016abe4da31026adf6b0a28bfdc24ef4a Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Fri, 1 May 2020 11:07:06 +0200 Subject: [PATCH] Jetty 10.0.x 4825 pushbuilder (#4829) * Issue #4825 Implement spec requirements for Request.newPushBuilder Signed-off-by: Jan Bartel --- .../eclipse/jetty/server/PushBuilderImpl.java | 21 ++- .../org/eclipse/jetty/server/Request.java | 47 ++++++- .../org/eclipse/jetty/server/RequestTest.java | 130 ++++++++++++++++++ 3 files changed, 190 insertions(+), 8 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/PushBuilderImpl.java b/jetty-server/src/main/java/org/eclipse/jetty/server/PushBuilderImpl.java index ab745646d884..61095d646c07 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/PushBuilderImpl.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/PushBuilderImpl.java @@ -18,6 +18,8 @@ package org.eclipse.jetty.server; +import java.util.EnumSet; +import java.util.Objects; import java.util.Set; import javax.servlet.http.PushBuilder; @@ -27,6 +29,7 @@ import org.eclipse.jetty.http.HttpMethod; import org.eclipse.jetty.http.HttpURI; import org.eclipse.jetty.http.MetaData; +import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.URIUtil; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -38,7 +41,14 @@ public class PushBuilderImpl implements PushBuilder { private static final Logger LOG = LoggerFactory.getLogger(PushBuilderImpl.class); - private static final HttpField JettyPush = new HttpField("x-http2-push", "PushBuilder"); + private static final HttpField JETTY_PUSH = new HttpField("x-http2-push", "PushBuilder"); + private static EnumSet UNSAFE_METHODS = EnumSet.of( + HttpMethod.POST, + HttpMethod.PUT, + HttpMethod.DELETE, + HttpMethod.CONNECT, + HttpMethod.OPTIONS, + HttpMethod.TRACE); private final Request _request; private final HttpFields.Mutable _fields; @@ -56,7 +66,7 @@ public PushBuilderImpl(Request request, HttpFields fields, String method, String _method = method; _queryString = queryString; _sessionId = sessionId; - _fields.add(JettyPush); + _fields.add(JETTY_PUSH); if (LOG.isDebugEnabled()) LOG.debug("PushBuilder({} {}?{} s={} c={})", _method, _request.getRequestURI(), _queryString, _sessionId); } @@ -70,6 +80,10 @@ public String getMethod() @Override public PushBuilder method(String method) { + Objects.requireNonNull(method); + + if (StringUtil.isBlank(method) || UNSAFE_METHODS.contains(HttpMethod.fromString(method))) + throw new IllegalArgumentException("Method not allowed for push: " + method); _method = method; return this; } @@ -149,9 +163,6 @@ public PushBuilder path(String path) @Override public void push() { - if (HttpMethod.POST.is(_method) || HttpMethod.PUT.is(_method)) - throw new IllegalStateException("Bad Method " + _method); - if (_path == null || _path.length() == 0) throw new IllegalStateException("Bad Path " + _path); diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java b/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java index 1a8d0be7b8e0..617816dcf378 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java @@ -71,6 +71,7 @@ import org.eclipse.jetty.http.ComplianceViolation; import org.eclipse.jetty.http.HostPortHttpField; import org.eclipse.jetty.http.HttpCookie; +import org.eclipse.jetty.http.HttpCookie.SetCookieHttpField; import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.HttpHeader; @@ -393,6 +394,11 @@ public PushBuilder newPushBuilder() HttpFields.Mutable fields = HttpFields.build(getHttpFields(), NOT_PUSHED_HEADERS); + HttpField authField = getHttpFields().getField(HttpHeader.AUTHORIZATION); + //TODO check what to do for digest etc etc + if (getUserPrincipal() != null && authField.getValue().startsWith("Basic")) + fields.add(authField); + String id; try { @@ -410,12 +416,47 @@ public PushBuilder newPushBuilder() id = getRequestedSessionId(); } + Map cookies = new HashMap<>(); + Cookie[] existingCookies = getCookies(); + if (existingCookies != null) + { + for (Cookie c: getCookies()) + { + cookies.put(c.getName(), c.getValue()); + } + } + + //Any Set-Cookies that were set on the response must be set as Cookies on the + //PushBuilder, unless the max-age of the cookie is <= 0 + HttpFields responseFields = getResponse().getHttpFields(); + for (HttpField field : responseFields) + { + HttpHeader header = field.getHeader(); + if (header == HttpHeader.SET_COOKIE) + { + HttpCookie cookie = ((SetCookieHttpField)field).getHttpCookie(); + if (cookie.getMaxAge() > 0) + cookies.put(cookie.getName(), cookie.getValue()); + else + cookies.remove(cookie.getName()); + } + } + + if (!cookies.isEmpty()) + { + StringBuilder buff = new StringBuilder(); + for (Map.Entry entry : cookies.entrySet()) + { + if (buff.length() > 0) + buff.append("; "); + buff.append(entry.getKey()).append('=').append(entry.getValue()); + } + fields.add(new HttpField(HttpHeader.COOKIE, buff.toString())); + } + PushBuilder builder = new PushBuilderImpl(this, fields, getMethod(), getQueryString(), id); builder.addHeader("referer", getRequestURL().toString()); - // TODO process any set cookies - // TODO process any user_identity - return builder; } diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java index e6c0a0ce660b..a23c4c8aef48 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java @@ -27,6 +27,7 @@ import java.io.Reader; import java.io.UnsupportedEncodingException; import java.nio.charset.StandardCharsets; +import java.security.Principal; import java.time.Duration; import java.util.ArrayList; import java.util.Arrays; @@ -45,10 +46,19 @@ import javax.servlet.http.HttpServletMapping; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import javax.servlet.http.HttpSession; import javax.servlet.http.Part; +import javax.servlet.http.PushBuilder; import org.eclipse.jetty.http.BadMessageException; +import org.eclipse.jetty.http.CookieCompliance; import org.eclipse.jetty.http.HttpCompliance; +import org.eclipse.jetty.http.HttpCookie; +import org.eclipse.jetty.http.HttpFields; +import org.eclipse.jetty.http.HttpHeader; +import org.eclipse.jetty.http.HttpURI; +import org.eclipse.jetty.http.HttpVersion; +import org.eclipse.jetty.http.MetaData; import org.eclipse.jetty.http.MimeTypes; import org.eclipse.jetty.http.pathmap.ServletPathSpec; import org.eclipse.jetty.http.tools.HttpTester; @@ -57,6 +67,9 @@ import org.eclipse.jetty.server.handler.AbstractHandler; import org.eclipse.jetty.server.handler.ContextHandler; import org.eclipse.jetty.server.handler.ErrorHandler; +import org.eclipse.jetty.server.session.Session; +import org.eclipse.jetty.server.session.SessionData; +import org.eclipse.jetty.server.session.SessionHandler; import org.eclipse.jetty.toolchain.test.FS; import org.eclipse.jetty.toolchain.test.MavenTestingUtils; import org.eclipse.jetty.util.BufferUtil; @@ -1860,11 +1873,128 @@ public void testGetterSafeFromNullPointerException() assertNotNull(request.getParameterMap()); assertEquals(0, request.getParameterMap().size()); } + + @Test + public void testPushBuilder() throws Exception + { + String uri = "/foo/something"; + Request request = new TestRequest(null, null); + request.getResponse().getHttpFields().add(new HttpCookie.SetCookieHttpField(new HttpCookie("good","thumbsup", 100), CookieCompliance.RFC6265)); + request.getResponse().getHttpFields().add(new HttpCookie.SetCookieHttpField(new HttpCookie("bonza","bewdy", 1), CookieCompliance.RFC6265)); + request.getResponse().getHttpFields().add(new HttpCookie.SetCookieHttpField(new HttpCookie("bad", "thumbsdown", 0), CookieCompliance.RFC6265)); + HttpFields.Mutable fields = HttpFields.build(); + fields.add(HttpHeader.AUTHORIZATION, "Basic foo"); + request.setMetaData(new MetaData.Request("GET", HttpURI.from(uri), HttpVersion.HTTP_1_0, fields)); + assertTrue(request.isPushSupported()); + PushBuilder builder = request.newPushBuilder(); + assertNotNull(builder); + assertEquals("GET", builder.getMethod()); + assertThrows(NullPointerException.class, () -> + { + builder.method(null); + }); + assertThrows(IllegalArgumentException.class, () -> + { + builder.method(""); + }); + assertThrows(IllegalArgumentException.class, () -> + { + builder.method(" "); + }); + assertThrows(IllegalArgumentException.class, () -> + { + builder.method("POST"); + }); + assertThrows(IllegalArgumentException.class, () -> + { + builder.method("PUT"); + }); + assertThrows(IllegalArgumentException.class, () -> + { + builder.method("DELETE"); + }); + assertThrows(IllegalArgumentException.class, () -> + { + builder.method("CONNECT"); + }); + assertThrows(IllegalArgumentException.class, () -> + { + builder.method("OPTIONS"); + }); + assertThrows(IllegalArgumentException.class, () -> + { + builder.method("TRACE"); + }); + assertEquals(TestRequest.TEST_SESSION_ID, builder.getSessionId()); + builder.path("/foo/something-else.txt"); + assertEquals("/foo/something-else.txt", builder.getPath()); + assertEquals("Basic foo", builder.getHeader("Authorization")); + assertThat(builder.getHeader("Cookie"), containsString("bonza")); + assertThat(builder.getHeader("Cookie"), containsString("good")); + assertThat(builder.getHeader("Cookie"), containsString("maxpos")); + assertThat(builder.getHeader("Cookie"), not(containsString("bad"))); + } interface RequestTester { boolean check(HttpServletRequest request, HttpServletResponse response) throws IOException; } + + private class TestRequest extends Request + { + public static final String TEST_SESSION_ID = "abc123"; + Response _response = new Response(null, null); + Cookie c1; + Cookie c2; + + public TestRequest(HttpChannel channel, HttpInput input) + { + super(channel, input); + c1 = new Cookie("maxpos", "xxx"); + c1.setMaxAge(1); + c2 = new Cookie("maxneg", "yyy"); + c2.setMaxAge(-1); + } + + @Override + public boolean isPushSupported() + { + return true; + } + + @Override + public HttpSession getSession() + { + return new Session(new SessionHandler(), new SessionData(TEST_SESSION_ID, "", "0.0.0.0", 0, 0, 0, 300)); + } + + @Override + public Principal getUserPrincipal() + { + return new Principal() + { + + @Override + public String getName() + { + return "user"; + } + + }; + } + + @Override + public Response getResponse() + { + return _response; + } + + @Override + public Cookie[] getCookies() + { + return new Cookie[] {c1,c2}; + } + } private class RequestHandler extends AbstractHandler {