From f5d63e86260bf1596090b4351025d8577d56f81a Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Tue, 5 Jun 2012 12:32:07 +0200 Subject: [PATCH 1/5] Moved version() method to base test class, and using HTTPSPDYHeader instead of hardcoded strings. --- .../jetty/spdy/http/AbstractHTTPSPDYTest.java | 5 +++++ .../jetty/spdy/http/ConcurrentStreamsTest.java | 16 ++++++++-------- .../spdy/http/PushStrategyBenchmarkTest.java | 6 ------ .../jetty/spdy/http/ServerHTTPSPDYv2Test.java | 6 ------ 4 files changed, 13 insertions(+), 20 deletions(-) diff --git a/jetty-spdy/spdy-jetty-http/src/test/java/org/eclipse/jetty/spdy/http/AbstractHTTPSPDYTest.java b/jetty-spdy/spdy-jetty-http/src/test/java/org/eclipse/jetty/spdy/http/AbstractHTTPSPDYTest.java index b875379f3ba..f6f41776d24 100644 --- a/jetty-spdy/spdy-jetty-http/src/test/java/org/eclipse/jetty/spdy/http/AbstractHTTPSPDYTest.java +++ b/jetty-spdy/spdy-jetty-http/src/test/java/org/eclipse/jetty/spdy/http/AbstractHTTPSPDYTest.java @@ -113,4 +113,9 @@ public abstract class AbstractHTTPSPDYTest server.join(); } } + + protected short version() + { + return SPDY.V2; + } } diff --git a/jetty-spdy/spdy-jetty-http/src/test/java/org/eclipse/jetty/spdy/http/ConcurrentStreamsTest.java b/jetty-spdy/spdy-jetty-http/src/test/java/org/eclipse/jetty/spdy/http/ConcurrentStreamsTest.java index a26555004b9..0bbf0e68ff3 100644 --- a/jetty-spdy/spdy-jetty-http/src/test/java/org/eclipse/jetty/spdy/http/ConcurrentStreamsTest.java +++ b/jetty-spdy/spdy-jetty-http/src/test/java/org/eclipse/jetty/spdy/http/ConcurrentStreamsTest.java @@ -73,10 +73,10 @@ public class ConcurrentStreamsTest extends AbstractHTTPSPDYTest // Perform slow request. This will wait on server side until the fast request wakes it up Headers headers = new Headers(); - headers.put("method", "GET"); - headers.put("url", "/slow"); - headers.put("version", "HTTP/1.1"); - headers.put("host", "localhost:" + connector.getLocalPort()); + headers.put(HTTPSPDYHeader.METHOD.name(version()), "GET"); + headers.put(HTTPSPDYHeader.URI.name(version()), "/slow"); + headers.put(HTTPSPDYHeader.VERSION.name(version()), "HTTP/1.1"); + headers.put(HTTPSPDYHeader.HOST.name(version()), "localhost:" + connector.getLocalPort()); final CountDownLatch slowClientLatch = new CountDownLatch(1); session.syn(new SynInfo(headers, true), new StreamFrameListener.Adapter() { @@ -91,10 +91,10 @@ public class ConcurrentStreamsTest extends AbstractHTTPSPDYTest // Perform the fast request. This will wake up the slow request headers.clear(); - headers.put("method", "GET"); - headers.put("url", "/fast"); - headers.put("version", "HTTP/1.1"); - headers.put("host", "localhost:" + connector.getLocalPort()); + headers.put(HTTPSPDYHeader.METHOD.name(version()), "GET"); + headers.put(HTTPSPDYHeader.URI.name(version()), "/fast"); + headers.put(HTTPSPDYHeader.VERSION.name(version()), "HTTP/1.1"); + headers.put(HTTPSPDYHeader.HOST.name(version()), "localhost:" + connector.getLocalPort()); final CountDownLatch fastClientLatch = new CountDownLatch(1); session.syn(new SynInfo(headers, true), new StreamFrameListener.Adapter() { diff --git a/jetty-spdy/spdy-jetty-http/src/test/java/org/eclipse/jetty/spdy/http/PushStrategyBenchmarkTest.java b/jetty-spdy/spdy-jetty-http/src/test/java/org/eclipse/jetty/spdy/http/PushStrategyBenchmarkTest.java index 0ff4893bd09..29b5952d4e5 100644 --- a/jetty-spdy/spdy-jetty-http/src/test/java/org/eclipse/jetty/spdy/http/PushStrategyBenchmarkTest.java +++ b/jetty-spdy/spdy-jetty-http/src/test/java/org/eclipse/jetty/spdy/http/PushStrategyBenchmarkTest.java @@ -38,7 +38,6 @@ import org.eclipse.jetty.server.handler.AbstractHandler; import org.eclipse.jetty.spdy.AsyncConnectionFactory; import org.eclipse.jetty.spdy.api.DataInfo; import org.eclipse.jetty.spdy.api.Headers; -import org.eclipse.jetty.spdy.api.SPDY; import org.eclipse.jetty.spdy.api.Session; import org.eclipse.jetty.spdy.api.SessionFrameListener; import org.eclipse.jetty.spdy.api.Stream; @@ -62,11 +61,6 @@ public class PushStrategyBenchmarkTest extends AbstractHTTPSPDYTest private final long roundtrip = 100; private final int runs = 10; - protected short version() - { - return SPDY.V2; - } - @Test public void benchmarkPushStrategy() throws Exception { diff --git a/jetty-spdy/spdy-jetty-http/src/test/java/org/eclipse/jetty/spdy/http/ServerHTTPSPDYv2Test.java b/jetty-spdy/spdy-jetty-http/src/test/java/org/eclipse/jetty/spdy/http/ServerHTTPSPDYv2Test.java index 3363d124a50..61ebfd127d9 100644 --- a/jetty-spdy/spdy-jetty-http/src/test/java/org/eclipse/jetty/spdy/http/ServerHTTPSPDYv2Test.java +++ b/jetty-spdy/spdy-jetty-http/src/test/java/org/eclipse/jetty/spdy/http/ServerHTTPSPDYv2Test.java @@ -41,7 +41,6 @@ import org.eclipse.jetty.spdy.api.BytesDataInfo; import org.eclipse.jetty.spdy.api.DataInfo; import org.eclipse.jetty.spdy.api.Headers; import org.eclipse.jetty.spdy.api.ReplyInfo; -import org.eclipse.jetty.spdy.api.SPDY; import org.eclipse.jetty.spdy.api.Session; import org.eclipse.jetty.spdy.api.Stream; import org.eclipse.jetty.spdy.api.StreamFrameListener; @@ -52,11 +51,6 @@ import org.junit.Test; public class ServerHTTPSPDYv2Test extends AbstractHTTPSPDYTest { - protected short version() - { - return SPDY.V2; - } - @Test public void testSimpleGET() throws Exception { From c7d09af3f255b1fd96632e36e6235e1d7731cfe4 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Tue, 5 Jun 2012 12:33:10 +0200 Subject: [PATCH 2/5] Improved ReferrerPushStrategy to check also for content-type of pushed resources. --- .../jetty/spdy/http/ReferrerPushStrategy.java | 49 +++- ...t.java => ReferrerPushStrategyV2Test.java} | 268 ++++++++++++++---- .../spdy/http/ReferrerPushStrategyV3Test.java | 28 ++ 3 files changed, 272 insertions(+), 73 deletions(-) rename jetty-spdy/spdy-jetty-http/src/test/java/org/eclipse/jetty/spdy/http/{ReferrerPushStrategyTest.java => ReferrerPushStrategyV2Test.java} (57%) create mode 100644 jetty-spdy/spdy-jetty-http/src/test/java/org/eclipse/jetty/spdy/http/ReferrerPushStrategyV3Test.java diff --git a/jetty-spdy/spdy-jetty-http/src/main/java/org/eclipse/jetty/spdy/http/ReferrerPushStrategy.java b/jetty-spdy/spdy-jetty-http/src/main/java/org/eclipse/jetty/spdy/http/ReferrerPushStrategy.java index cf3fe06f1b6..bc6e26e00b2 100644 --- a/jetty-spdy/spdy-jetty-http/src/main/java/org/eclipse/jetty/spdy/http/ReferrerPushStrategy.java +++ b/jetty-spdy/spdy-jetty-http/src/main/java/org/eclipse/jetty/spdy/http/ReferrerPushStrategy.java @@ -18,7 +18,7 @@ package org.eclipse.jetty.spdy.http; import java.util.Arrays; import java.util.Collections; -import java.util.LinkedHashSet; +import java.util.HashSet; import java.util.List; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; @@ -50,32 +50,41 @@ import org.eclipse.jetty.util.log.Logger; * TODO: although these entries will be limited by the number of application pages. * TODO: however, there is no ConcurrentLinkedHashMap yet in JDK (there is in Guava though) * TODO: so we cannot use the built-in LRU features of LinkedHashMap - * - * TODO: Wikipedia maps URLs like http://en.wikipedia.org/wiki/File:PNG-Gradient_hex.png - * TODO: to text/html, so perhaps we need to improve isPushResource() by looking at the - * TODO: response Content-Type header, and not only at the URL extension */ public class ReferrerPushStrategy implements PushStrategy { private static final Logger logger = Log.getLogger(ReferrerPushStrategy.class); private final ConcurrentMap> resources = new ConcurrentHashMap<>(); - private final Set pushRegexps = new LinkedHashSet<>(); - private final Set allowedPushOrigins = new LinkedHashSet<>(); + private final Set pushRegexps = new HashSet<>(); + private final Set pushContentTypes = new HashSet<>(); + private final Set allowedPushOrigins = new HashSet<>(); public ReferrerPushStrategy() { - this(Arrays.asList(".*\\.css", ".*\\.js", ".*\\.png", ".*\\.jpg", ".*\\.gif")); + this(Arrays.asList(".*\\.css", ".*\\.js", ".*\\.png", ".*\\.jpeg", ".*\\.jpg", ".*\\.gif", ".*\\.ico")); } public ReferrerPushStrategy(List pushRegexps) { - this(pushRegexps, Collections.emptyList()); + this(pushRegexps, Arrays.asList( + "text/css", + "text/javascript", "application/javascript", "application/x-javascript", + "image/png", "image/x-png", + "image/jpeg", + "image/gif", + "image/x-icon", "image/vnd.microsoft.icon")); } - public ReferrerPushStrategy(List pushRegexps, List allowedPushOrigins) + public ReferrerPushStrategy(List pushRegexps, List pushContentTypes) + { + this(pushRegexps, pushContentTypes, Collections.emptyList()); + } + + public ReferrerPushStrategy(List pushRegexps, List pushContentTypes, List allowedPushOrigins) { for (String pushRegexp : pushRegexps) this.pushRegexps.add(Pattern.compile(pushRegexp)); + this.pushContentTypes.addAll(pushContentTypes); for (String allowedPushOrigin : allowedPushOrigins) this.allowedPushOrigins.add(Pattern.compile(allowedPushOrigin.replace(".", "\\.").replace("*", ".*"))); } @@ -84,13 +93,14 @@ public class ReferrerPushStrategy implements PushStrategy public Set apply(Stream stream, Headers requestHeaders, Headers responseHeaders) { Set result = Collections.emptySet(); - String scheme = requestHeaders.get("scheme").value(); - String host = requestHeaders.get("host").value(); + short version = stream.getSession().getVersion(); + String scheme = requestHeaders.get(HTTPSPDYHeader.SCHEME.name(version)).value(); + String host = requestHeaders.get(HTTPSPDYHeader.HOST.name(version)).value(); String origin = new StringBuilder(scheme).append("://").append(host).toString(); - String url = requestHeaders.get("url").value(); + String url = requestHeaders.get(HTTPSPDYHeader.URI.name(version)).value(); String absoluteURL = new StringBuilder(origin).append(url).toString(); logger.debug("Applying push strategy for {}", absoluteURL); - if (isValidMethod(requestHeaders.get("method").value())) + if (isValidMethod(requestHeaders.get(HTTPSPDYHeader.METHOD.name(version)).value())) { if (isMainResource(url, responseHeaders)) { @@ -129,7 +139,16 @@ public class ReferrerPushStrategy implements PushStrategy for (Pattern pushRegexp : pushRegexps) { if (pushRegexp.matcher(url).matches()) - return true; + { + Headers.Header header = responseHeaders.get("content-type"); + if (header == null) + return true; + + String contentType = header.value().toLowerCase(); + for (String pushContentType : pushContentTypes) + if (contentType.startsWith(pushContentType)) + return true; + } } return false; } diff --git a/jetty-spdy/spdy-jetty-http/src/test/java/org/eclipse/jetty/spdy/http/ReferrerPushStrategyTest.java b/jetty-spdy/spdy-jetty-http/src/test/java/org/eclipse/jetty/spdy/http/ReferrerPushStrategyV2Test.java similarity index 57% rename from jetty-spdy/spdy-jetty-http/src/test/java/org/eclipse/jetty/spdy/http/ReferrerPushStrategyTest.java rename to jetty-spdy/spdy-jetty-http/src/test/java/org/eclipse/jetty/spdy/http/ReferrerPushStrategyV2Test.java index 4215118d5a1..b9b73d30138 100644 --- a/jetty-spdy/spdy-jetty-http/src/test/java/org/eclipse/jetty/spdy/http/ReferrerPushStrategyTest.java +++ b/jetty-spdy/spdy-jetty-http/src/test/java/org/eclipse/jetty/spdy/http/ReferrerPushStrategyV2Test.java @@ -1,3 +1,19 @@ +/* + * Copyright (c) 2012 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + package org.eclipse.jetty.spdy.http; import java.io.IOException; @@ -24,7 +40,7 @@ import org.eclipse.jetty.spdy.api.SynInfo; import org.junit.Assert; import org.junit.Test; -public class ReferrerPushStrategyTest extends AbstractHTTPSPDYTest +public class ReferrerPushStrategyV2Test extends AbstractHTTPSPDYTest { @Override protected SPDYServerConnector newHTTPSPDYServerConnector(short version) @@ -38,7 +54,7 @@ public class ReferrerPushStrategyTest extends AbstractHTTPSPDYTest @Test public void testAssociatedResourceIsPushed() throws Exception { - InetSocketAddress address = startHTTPServer(new AbstractHandler() + InetSocketAddress address = startHTTPServer(version(), new AbstractHandler() { @Override public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException @@ -52,16 +68,16 @@ public class ReferrerPushStrategyTest extends AbstractHTTPSPDYTest baseRequest.setHandled(true); } }); - Session session1 = startClient(address, null); + Session session1 = startClient(version(), address, null); final CountDownLatch mainResourceLatch = new CountDownLatch(1); Headers mainRequestHeaders = new Headers(); - mainRequestHeaders.put("method", "GET"); + mainRequestHeaders.put(HTTPSPDYHeader.METHOD.name(version()), "GET"); String mainResource = "/index.html"; - mainRequestHeaders.put("url", mainResource); - mainRequestHeaders.put("version", "HTTP/1.1"); - mainRequestHeaders.put("scheme", "http"); - mainRequestHeaders.put("host", "localhost:" + connector.getLocalPort()); + mainRequestHeaders.put(HTTPSPDYHeader.URI.name(version()), mainResource); + mainRequestHeaders.put(HTTPSPDYHeader.VERSION.name(version()), "HTTP/1.1"); + mainRequestHeaders.put(HTTPSPDYHeader.SCHEME.name(version()), "http"); + mainRequestHeaders.put(HTTPSPDYHeader.HOST.name(version()), "localhost:" + connector.getLocalPort()); session1.syn(new SynInfo(mainRequestHeaders, true), new StreamFrameListener.Adapter() { @Override @@ -76,11 +92,11 @@ public class ReferrerPushStrategyTest extends AbstractHTTPSPDYTest final CountDownLatch associatedResourceLatch = new CountDownLatch(1); Headers associatedRequestHeaders = new Headers(); - associatedRequestHeaders.put("method", "GET"); - associatedRequestHeaders.put("url", "/style.css"); - associatedRequestHeaders.put("version", "HTTP/1.1"); - associatedRequestHeaders.put("scheme", "http"); - associatedRequestHeaders.put("host", "localhost:" + connector.getLocalPort()); + associatedRequestHeaders.put(HTTPSPDYHeader.METHOD.name(version()), "GET"); + associatedRequestHeaders.put(HTTPSPDYHeader.URI.name(version()), "/style.css"); + associatedRequestHeaders.put(HTTPSPDYHeader.VERSION.name(version()), "HTTP/1.1"); + associatedRequestHeaders.put(HTTPSPDYHeader.SCHEME.name(version()), "http"); + associatedRequestHeaders.put(HTTPSPDYHeader.HOST.name(version()), "localhost:" + connector.getLocalPort()); associatedRequestHeaders.put("referer", "http://localhost:" + connector.getLocalPort() + mainResource); session1.syn(new SynInfo(associatedRequestHeaders, true), new StreamFrameListener.Adapter() { @@ -98,7 +114,7 @@ public class ReferrerPushStrategyTest extends AbstractHTTPSPDYTest final CountDownLatch mainStreamLatch = new CountDownLatch(2); final CountDownLatch pushDataLatch = new CountDownLatch(1); - Session session2 = startClient(address, new SessionFrameListener.Adapter() + Session session2 = startClient(version(), address, new SessionFrameListener.Adapter() { @Override public StreamFrameListener onSyn(Stream stream, SynInfo synInfo) @@ -138,10 +154,146 @@ public class ReferrerPushStrategyTest extends AbstractHTTPSPDYTest Assert.assertTrue(pushDataLatch.await(5, TimeUnit.SECONDS)); } + @Test + public void testAssociatedResourceWithWrongContentTypeIsNotPushed() throws Exception + { + InetSocketAddress address = startHTTPServer(version(), new AbstractHandler() + { + @Override + public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException + { + String url = request.getRequestURI(); + PrintWriter output = response.getWriter(); + if (url.endsWith(".html")) + { + response.setContentType("text/html"); + output.print("HELLO"); + } + else if (url.equals("/fake.png")) + { + response.setContentType("text/html"); + output.print("IMAGE"); + } + else if (url.endsWith(".css")) + { + response.setContentType("text/css"); + output.print("body { background: #FFF; }"); + } + baseRequest.setHandled(true); + } + }); + Session session1 = startClient(version(), address, null); + + final CountDownLatch mainResourceLatch = new CountDownLatch(1); + Headers mainRequestHeaders = new Headers(); + mainRequestHeaders.put(HTTPSPDYHeader.METHOD.name(version()), "GET"); + String mainResource = "/index.html"; + mainRequestHeaders.put(HTTPSPDYHeader.URI.name(version()), mainResource); + mainRequestHeaders.put(HTTPSPDYHeader.VERSION.name(version()), "HTTP/1.1"); + mainRequestHeaders.put(HTTPSPDYHeader.SCHEME.name(version()), "http"); + mainRequestHeaders.put(HTTPSPDYHeader.HOST.name(version()), "localhost:" + connector.getLocalPort()); + session1.syn(new SynInfo(mainRequestHeaders, true), new StreamFrameListener.Adapter() + { + @Override + public void onData(Stream stream, DataInfo dataInfo) + { + dataInfo.consume(dataInfo.length()); + if (dataInfo.isClose()) + mainResourceLatch.countDown(); + } + }); + Assert.assertTrue(mainResourceLatch.await(5, TimeUnit.SECONDS)); + + final CountDownLatch associatedResourceLatch = new CountDownLatch(1); + Headers associatedRequestHeaders = new Headers(); + associatedRequestHeaders.put(HTTPSPDYHeader.METHOD.name(version()), "GET"); + associatedRequestHeaders.put(HTTPSPDYHeader.URI.name(version()), "/stylesheet.css"); + associatedRequestHeaders.put(HTTPSPDYHeader.VERSION.name(version()), "HTTP/1.1"); + associatedRequestHeaders.put(HTTPSPDYHeader.SCHEME.name(version()), "http"); + associatedRequestHeaders.put(HTTPSPDYHeader.HOST.name(version()), "localhost:" + connector.getLocalPort()); + associatedRequestHeaders.put("referer", "http://localhost:" + connector.getLocalPort() + mainResource); + session1.syn(new SynInfo(associatedRequestHeaders, true), new StreamFrameListener.Adapter() + { + @Override + public void onData(Stream stream, DataInfo dataInfo) + { + dataInfo.consume(dataInfo.length()); + if (dataInfo.isClose()) + associatedResourceLatch.countDown(); + } + }); + Assert.assertTrue(associatedResourceLatch.await(5, TimeUnit.SECONDS)); + + final CountDownLatch fakeAssociatedResourceLatch = new CountDownLatch(1); + Headers fakeAssociatedRequestHeaders = new Headers(); + fakeAssociatedRequestHeaders.put(HTTPSPDYHeader.METHOD.name(version()), "GET"); + fakeAssociatedRequestHeaders.put(HTTPSPDYHeader.URI.name(version()), "/fake.png"); + fakeAssociatedRequestHeaders.put(HTTPSPDYHeader.VERSION.name(version()), "HTTP/1.1"); + fakeAssociatedRequestHeaders.put(HTTPSPDYHeader.SCHEME.name(version()), "http"); + fakeAssociatedRequestHeaders.put(HTTPSPDYHeader.HOST.name(version()), "localhost:" + connector.getLocalPort()); + fakeAssociatedRequestHeaders.put("referer", "http://localhost:" + connector.getLocalPort() + mainResource); + session1.syn(new SynInfo(fakeAssociatedRequestHeaders, true), new StreamFrameListener.Adapter() + { + @Override + public void onData(Stream stream, DataInfo dataInfo) + { + dataInfo.consume(dataInfo.length()); + if (dataInfo.isClose()) + fakeAssociatedResourceLatch.countDown(); + } + }); + Assert.assertTrue(fakeAssociatedResourceLatch.await(5, TimeUnit.SECONDS)); + + // Create another client, and perform the same request for the main resource, + // we expect the css being pushed but not the fake PNG + + final CountDownLatch mainStreamLatch = new CountDownLatch(2); + final CountDownLatch pushDataLatch = new CountDownLatch(1); + Session session2 = startClient(version(), address, new SessionFrameListener.Adapter() + { + @Override + public StreamFrameListener onSyn(Stream stream, SynInfo synInfo) + { + Assert.assertTrue(stream.isUnidirectional()); + Assert.assertTrue(synInfo.getHeaders().get(HTTPSPDYHeader.URI.name(version())).value().endsWith(".css")); + return new StreamFrameListener.Adapter() + { + @Override + public void onData(Stream stream, DataInfo dataInfo) + { + dataInfo.consume(dataInfo.length()); + if (dataInfo.isClose()) + pushDataLatch.countDown(); + } + }; + } + }); + session2.syn(new SynInfo(mainRequestHeaders, true), new StreamFrameListener.Adapter() + { + @Override + public void onReply(Stream stream, ReplyInfo replyInfo) + { + Assert.assertFalse(replyInfo.isClose()); + mainStreamLatch.countDown(); + } + + @Override + public void onData(Stream stream, DataInfo dataInfo) + { + dataInfo.consume(dataInfo.length()); + if (dataInfo.isClose()) + mainStreamLatch.countDown(); + } + }); + + Assert.assertTrue(mainStreamLatch.await(5, TimeUnit.SECONDS)); + Assert.assertTrue(pushDataLatch.await(5, TimeUnit.SECONDS)); + } + @Test public void testNestedAssociatedResourceIsPushed() throws Exception { - InetSocketAddress address = startHTTPServer(new AbstractHandler() + InetSocketAddress address = startHTTPServer(version(), new AbstractHandler() { @Override public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException @@ -157,16 +309,16 @@ public class ReferrerPushStrategyTest extends AbstractHTTPSPDYTest baseRequest.setHandled(true); } }); - Session session1 = startClient(address, null); + Session session1 = startClient(version(), address, null); final CountDownLatch mainResourceLatch = new CountDownLatch(1); Headers mainRequestHeaders = new Headers(); - mainRequestHeaders.put("method", "GET"); + mainRequestHeaders.put(HTTPSPDYHeader.METHOD.name(version()), "GET"); String mainResource = "/index.html"; - mainRequestHeaders.put("url", mainResource); - mainRequestHeaders.put("version", "HTTP/1.1"); - mainRequestHeaders.put("scheme", "http"); - mainRequestHeaders.put("host", "localhost:" + connector.getLocalPort()); + mainRequestHeaders.put(HTTPSPDYHeader.URI.name(version()), mainResource); + mainRequestHeaders.put(HTTPSPDYHeader.VERSION.name(version()), "HTTP/1.1"); + mainRequestHeaders.put(HTTPSPDYHeader.SCHEME.name(version()), "http"); + mainRequestHeaders.put(HTTPSPDYHeader.HOST.name(version()), "localhost:" + connector.getLocalPort()); session1.syn(new SynInfo(mainRequestHeaders, true), new StreamFrameListener.Adapter() { @Override @@ -181,12 +333,12 @@ public class ReferrerPushStrategyTest extends AbstractHTTPSPDYTest final CountDownLatch associatedResourceLatch = new CountDownLatch(1); Headers associatedRequestHeaders = new Headers(); - associatedRequestHeaders.put("method", "GET"); + associatedRequestHeaders.put(HTTPSPDYHeader.METHOD.name(version()), "GET"); String associatedResource = "/style.css"; - associatedRequestHeaders.put("url", associatedResource); - associatedRequestHeaders.put("version", "HTTP/1.1"); - associatedRequestHeaders.put("scheme", "http"); - associatedRequestHeaders.put("host", "localhost:" + connector.getLocalPort()); + associatedRequestHeaders.put(HTTPSPDYHeader.URI.name(version()), associatedResource); + associatedRequestHeaders.put(HTTPSPDYHeader.VERSION.name(version()), "HTTP/1.1"); + associatedRequestHeaders.put(HTTPSPDYHeader.SCHEME.name(version()), "http"); + associatedRequestHeaders.put(HTTPSPDYHeader.HOST.name(version()), "localhost:" + connector.getLocalPort()); associatedRequestHeaders.put("referer", "http://localhost:" + connector.getLocalPort() + mainResource); session1.syn(new SynInfo(associatedRequestHeaders, true), new StreamFrameListener.Adapter() { @@ -202,11 +354,11 @@ public class ReferrerPushStrategyTest extends AbstractHTTPSPDYTest final CountDownLatch nestedResourceLatch = new CountDownLatch(1); Headers nestedRequestHeaders = new Headers(); - nestedRequestHeaders.put("method", "GET"); - nestedRequestHeaders.put("url", "/image.gif"); - nestedRequestHeaders.put("version", "HTTP/1.1"); - nestedRequestHeaders.put("scheme", "http"); - nestedRequestHeaders.put("host", "localhost:" + connector.getLocalPort()); + nestedRequestHeaders.put(HTTPSPDYHeader.METHOD.name(version()), "GET"); + nestedRequestHeaders.put(HTTPSPDYHeader.URI.name(version()), "/image.gif"); + nestedRequestHeaders.put(HTTPSPDYHeader.VERSION.name(version()), "HTTP/1.1"); + nestedRequestHeaders.put(HTTPSPDYHeader.SCHEME.name(version()), "http"); + nestedRequestHeaders.put(HTTPSPDYHeader.HOST.name(version()), "localhost:" + connector.getLocalPort()); nestedRequestHeaders.put("referer", "http://localhost:" + connector.getLocalPort() + associatedResource); session1.syn(new SynInfo(nestedRequestHeaders, true), new StreamFrameListener.Adapter() { @@ -224,7 +376,7 @@ public class ReferrerPushStrategyTest extends AbstractHTTPSPDYTest final CountDownLatch mainStreamLatch = new CountDownLatch(2); final CountDownLatch pushDataLatch = new CountDownLatch(2); - Session session2 = startClient(address, new SessionFrameListener.Adapter() + Session session2 = startClient(version(), address, new SessionFrameListener.Adapter() { @Override public StreamFrameListener onSyn(Stream stream, SynInfo synInfo) @@ -267,7 +419,7 @@ public class ReferrerPushStrategyTest extends AbstractHTTPSPDYTest @Test public void testMainResourceWithReferrerIsNotPushed() throws Exception { - InetSocketAddress address = startHTTPServer(new AbstractHandler() + InetSocketAddress address = startHTTPServer(version(), new AbstractHandler() { @Override public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException @@ -279,16 +431,16 @@ public class ReferrerPushStrategyTest extends AbstractHTTPSPDYTest baseRequest.setHandled(true); } }); - Session session1 = startClient(address, null); + Session session1 = startClient(version(), address, null); final CountDownLatch mainResourceLatch = new CountDownLatch(1); Headers mainRequestHeaders = new Headers(); - mainRequestHeaders.put("method", "GET"); + mainRequestHeaders.put(HTTPSPDYHeader.METHOD.name(version()), "GET"); String mainResource = "/index.html"; - mainRequestHeaders.put("url", mainResource); - mainRequestHeaders.put("version", "HTTP/1.1"); - mainRequestHeaders.put("scheme", "http"); - mainRequestHeaders.put("host", "localhost:" + connector.getLocalPort()); + mainRequestHeaders.put(HTTPSPDYHeader.URI.name(version()), mainResource); + mainRequestHeaders.put(HTTPSPDYHeader.VERSION.name(version()), "HTTP/1.1"); + mainRequestHeaders.put(HTTPSPDYHeader.SCHEME.name(version()), "http"); + mainRequestHeaders.put(HTTPSPDYHeader.HOST.name(version()), "localhost:" + connector.getLocalPort()); session1.syn(new SynInfo(mainRequestHeaders, true), new StreamFrameListener.Adapter() { @Override @@ -303,11 +455,11 @@ public class ReferrerPushStrategyTest extends AbstractHTTPSPDYTest final CountDownLatch associatedResourceLatch = new CountDownLatch(1); Headers associatedRequestHeaders = new Headers(); - associatedRequestHeaders.put("method", "GET"); - associatedRequestHeaders.put("url", "/home.html"); - associatedRequestHeaders.put("version", "HTTP/1.1"); - associatedRequestHeaders.put("scheme", "http"); - associatedRequestHeaders.put("host", "localhost:" + connector.getLocalPort()); + associatedRequestHeaders.put(HTTPSPDYHeader.METHOD.name(version()), "GET"); + associatedRequestHeaders.put(HTTPSPDYHeader.URI.name(version()), "/home.html"); + associatedRequestHeaders.put(HTTPSPDYHeader.VERSION.name(version()), "HTTP/1.1"); + associatedRequestHeaders.put(HTTPSPDYHeader.SCHEME.name(version()), "http"); + associatedRequestHeaders.put(HTTPSPDYHeader.HOST.name(version()), "localhost:" + connector.getLocalPort()); associatedRequestHeaders.put("referer", "http://localhost:" + connector.getLocalPort() + mainResource); session1.syn(new SynInfo(associatedRequestHeaders, true), new StreamFrameListener.Adapter() { @@ -325,7 +477,7 @@ public class ReferrerPushStrategyTest extends AbstractHTTPSPDYTest final CountDownLatch mainStreamLatch = new CountDownLatch(2); final CountDownLatch pushLatch = new CountDownLatch(1); - Session session2 = startClient(address, new SessionFrameListener.Adapter() + Session session2 = startClient(version(), address, new SessionFrameListener.Adapter() { @Override public StreamFrameListener onSyn(Stream stream, SynInfo synInfo) @@ -359,7 +511,7 @@ public class ReferrerPushStrategyTest extends AbstractHTTPSPDYTest @Test public void testRequestWithIfModifiedSinceHeaderPreventsPush() throws Exception { - InetSocketAddress address = startHTTPServer(new AbstractHandler() + InetSocketAddress address = startHTTPServer(version(), new AbstractHandler() { @Override public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException @@ -373,16 +525,16 @@ public class ReferrerPushStrategyTest extends AbstractHTTPSPDYTest baseRequest.setHandled(true); } }); - Session session1 = startClient(address, null); + Session session1 = startClient(version(), address, null); final CountDownLatch mainResourceLatch = new CountDownLatch(1); Headers mainRequestHeaders = new Headers(); - mainRequestHeaders.put("method", "GET"); + mainRequestHeaders.put(HTTPSPDYHeader.METHOD.name(version()), "GET"); String mainResource = "/index.html"; - mainRequestHeaders.put("url", mainResource); - mainRequestHeaders.put("version", "HTTP/1.1"); - mainRequestHeaders.put("scheme", "http"); - mainRequestHeaders.put("host", "localhost:" + connector.getLocalPort()); + mainRequestHeaders.put(HTTPSPDYHeader.URI.name(version()), mainResource); + mainRequestHeaders.put(HTTPSPDYHeader.VERSION.name(version()), "HTTP/1.1"); + mainRequestHeaders.put(HTTPSPDYHeader.SCHEME.name(version()), "http"); + mainRequestHeaders.put(HTTPSPDYHeader.HOST.name(version()), "localhost:" + connector.getLocalPort()); mainRequestHeaders.put("If-Modified-Since", "Tue, 27 Mar 2012 16:36:52 GMT"); session1.syn(new SynInfo(mainRequestHeaders, true), new StreamFrameListener.Adapter() { @@ -398,11 +550,11 @@ public class ReferrerPushStrategyTest extends AbstractHTTPSPDYTest final CountDownLatch associatedResourceLatch = new CountDownLatch(1); Headers associatedRequestHeaders = new Headers(); - associatedRequestHeaders.put("method", "GET"); - associatedRequestHeaders.put("url", "/style.css"); - associatedRequestHeaders.put("version", "HTTP/1.1"); - associatedRequestHeaders.put("scheme", "http"); - associatedRequestHeaders.put("host", "localhost:" + connector.getLocalPort()); + associatedRequestHeaders.put(HTTPSPDYHeader.METHOD.name(version()), "GET"); + associatedRequestHeaders.put(HTTPSPDYHeader.URI.name(version()), "/style.css"); + associatedRequestHeaders.put(HTTPSPDYHeader.VERSION.name(version()), "HTTP/1.1"); + associatedRequestHeaders.put(HTTPSPDYHeader.SCHEME.name(version()), "http"); + associatedRequestHeaders.put(HTTPSPDYHeader.HOST.name(version()), "localhost:" + connector.getLocalPort()); associatedRequestHeaders.put("referer", "http://localhost:" + connector.getLocalPort() + mainResource); session1.syn(new SynInfo(associatedRequestHeaders, true), new StreamFrameListener.Adapter() { @@ -421,7 +573,7 @@ public class ReferrerPushStrategyTest extends AbstractHTTPSPDYTest final CountDownLatch mainStreamLatch = new CountDownLatch(2); final CountDownLatch pushDataLatch = new CountDownLatch(1); - Session session2 = startClient(address, new SessionFrameListener.Adapter() + Session session2 = startClient(version(), address, new SessionFrameListener.Adapter() { @Override public StreamFrameListener onSyn(Stream stream, SynInfo synInfo) diff --git a/jetty-spdy/spdy-jetty-http/src/test/java/org/eclipse/jetty/spdy/http/ReferrerPushStrategyV3Test.java b/jetty-spdy/spdy-jetty-http/src/test/java/org/eclipse/jetty/spdy/http/ReferrerPushStrategyV3Test.java new file mode 100644 index 00000000000..a722fde9f46 --- /dev/null +++ b/jetty-spdy/spdy-jetty-http/src/test/java/org/eclipse/jetty/spdy/http/ReferrerPushStrategyV3Test.java @@ -0,0 +1,28 @@ +/* + * Copyright (c) 2012 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.eclipse.jetty.spdy.http; + +import org.eclipse.jetty.spdy.api.SPDY; + +public class ReferrerPushStrategyV3Test extends ReferrerPushStrategyV2Test +{ + @Override + protected short version() + { + return SPDY.V3; + } +} From cd05259be4eec537af1f908f89ca758d5114b645 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Tue, 5 Jun 2012 13:30:15 +0200 Subject: [PATCH 3/5] Updated ReferrerPushStrategy to limit the number of associated resources. --- .../jetty/spdy/http/ReferrerPushStrategy.java | 45 ++++-- .../spdy/http/ReferrerPushStrategyV2Test.java | 133 ++++++++++++++++++ 2 files changed, 167 insertions(+), 11 deletions(-) diff --git a/jetty-spdy/spdy-jetty-http/src/main/java/org/eclipse/jetty/spdy/http/ReferrerPushStrategy.java b/jetty-spdy/spdy-jetty-http/src/main/java/org/eclipse/jetty/spdy/http/ReferrerPushStrategy.java index bc6e26e00b2..52f1243d734 100644 --- a/jetty-spdy/spdy-jetty-http/src/main/java/org/eclipse/jetty/spdy/http/ReferrerPushStrategy.java +++ b/jetty-spdy/spdy-jetty-http/src/main/java/org/eclipse/jetty/spdy/http/ReferrerPushStrategy.java @@ -39,17 +39,18 @@ import org.eclipse.jetty.util.log.Logger; *

However, also following a hyperlink generates a HTTP request with a Referer * HTTP header that points to index.html; therefore main resources and associated * resources must be distinguishable.

- *

This class distinguishes associated resources by their URL path suffix. + *

This class distinguishes associated resources by their URL path suffix and content + * type. * CSS stylesheets, images and JavaScript files have recognizable URL path suffixes that * are classified as associated resources.

- *

Note however, that CSS stylesheets may refer to images, and the CSS image request - * will have the CSS stylesheet as referrer, so there is some degree of recursion that - * needs to be handled.

- * - * TODO: this class is kind-of leaking since the resources map is always adding entries - * TODO: although these entries will be limited by the number of application pages. - * TODO: however, there is no ConcurrentLinkedHashMap yet in JDK (there is in Guava though) - * TODO: so we cannot use the built-in LRU features of LinkedHashMap + *

When CSS stylesheets refer to images, the CSS image request will have the CSS + * stylesheet as referrer. This implementation will push also the CSS image.

+ *

The push metadata built by this implementation is limited by the number of pages + * of the application itself, and by the + * {@link #getMaxAssociatedResources() max associated resources} parameter. + * This parameter limits the number of associated resources per each main resource, so + * that if a main resource has hundreds of associated resources, only up to the number + * specified by this parameter will be pushed.

*/ public class ReferrerPushStrategy implements PushStrategy { @@ -58,6 +59,7 @@ public class ReferrerPushStrategy implements PushStrategy private final Set pushRegexps = new HashSet<>(); private final Set pushContentTypes = new HashSet<>(); private final Set allowedPushOrigins = new HashSet<>(); + private volatile int maxAssociatedResources = 32; public ReferrerPushStrategy() { @@ -89,6 +91,16 @@ public class ReferrerPushStrategy implements PushStrategy this.allowedPushOrigins.add(Pattern.compile(allowedPushOrigin.replace(".", "\\.").replace("*", ".*"))); } + public int getMaxAssociatedResources() + { + return maxAssociatedResources; + } + + public void setMaxAssociatedResources(int maxAssociatedResources) + { + this.maxAssociatedResources = maxAssociatedResources; + } + @Override public Set apply(Stream stream, Headers requestHeaders, Headers responseHeaders) { @@ -173,8 +185,19 @@ public class ReferrerPushStrategy implements PushStrategy if (existing != null) pushResources = existing; } - pushResources.add(url); - logger.debug("Built push metadata for {}: {}", referrer, pushResources); + // This check is not strictly concurrent-safe, but limiting + // the number of associated resources is achieved anyway + // although in rare cases few more resources will be stored + if (pushResources.size() < getMaxAssociatedResources()) + { + pushResources.add(url); + logger.debug("Stored push metadata for {}: {}", referrer, pushResources); + } + else + { + logger.debug("Skipped store of push metadata {} for {}: max associated resources ({}) reached", + url, referrer, maxAssociatedResources); + } } } diff --git a/jetty-spdy/spdy-jetty-http/src/test/java/org/eclipse/jetty/spdy/http/ReferrerPushStrategyV2Test.java b/jetty-spdy/spdy-jetty-http/src/test/java/org/eclipse/jetty/spdy/http/ReferrerPushStrategyV2Test.java index b9b73d30138..ce88e712c74 100644 --- a/jetty-spdy/spdy-jetty-http/src/test/java/org/eclipse/jetty/spdy/http/ReferrerPushStrategyV2Test.java +++ b/jetty-spdy/spdy-jetty-http/src/test/java/org/eclipse/jetty/spdy/http/ReferrerPushStrategyV2Test.java @@ -51,6 +51,139 @@ public class ReferrerPushStrategyV2Test extends AbstractHTTPSPDYTest return connector; } + @Test + public void testMaxAssociatedResources() throws Exception + { + InetSocketAddress address = startHTTPServer(version(), new AbstractHandler() + { + @Override + public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException + { + String url = request.getRequestURI(); + PrintWriter output = response.getWriter(); + if (url.endsWith(".html")) + output.print("HELLO"); + else if (url.endsWith(".css")) + output.print("body { background: #FFF; }"); + else if (url.endsWith(".js")) + output.print("function(){}();"); + baseRequest.setHandled(true); + } + }); + ReferrerPushStrategy pushStrategy = new ReferrerPushStrategy(); + pushStrategy.setMaxAssociatedResources(1); + AsyncConnectionFactory defaultFactory = new ServerHTTPSPDYAsyncConnectionFactory(version(), connector.getByteBufferPool(), connector.getExecutor(), connector.getScheduler(), connector, pushStrategy); + connector.setDefaultAsyncConnectionFactory(defaultFactory); + + Session session1 = startClient(version(), address, null); + + final CountDownLatch mainResourceLatch = new CountDownLatch(1); + Headers mainRequestHeaders = new Headers(); + mainRequestHeaders.put(HTTPSPDYHeader.METHOD.name(version()), "GET"); + String mainResource = "/index.html"; + mainRequestHeaders.put(HTTPSPDYHeader.URI.name(version()), mainResource); + mainRequestHeaders.put(HTTPSPDYHeader.VERSION.name(version()), "HTTP/1.1"); + mainRequestHeaders.put(HTTPSPDYHeader.SCHEME.name(version()), "http"); + mainRequestHeaders.put(HTTPSPDYHeader.HOST.name(version()), "localhost:" + connector.getLocalPort()); + session1.syn(new SynInfo(mainRequestHeaders, true), new StreamFrameListener.Adapter() + { + @Override + public void onData(Stream stream, DataInfo dataInfo) + { + dataInfo.consume(dataInfo.length()); + if (dataInfo.isClose()) + mainResourceLatch.countDown(); + } + }); + Assert.assertTrue(mainResourceLatch.await(5, TimeUnit.SECONDS)); + + final CountDownLatch associatedResourceLatch1 = new CountDownLatch(1); + Headers associatedRequestHeaders1 = new Headers(); + associatedRequestHeaders1.put(HTTPSPDYHeader.METHOD.name(version()), "GET"); + associatedRequestHeaders1.put(HTTPSPDYHeader.URI.name(version()), "/style.css"); + associatedRequestHeaders1.put(HTTPSPDYHeader.VERSION.name(version()), "HTTP/1.1"); + associatedRequestHeaders1.put(HTTPSPDYHeader.SCHEME.name(version()), "http"); + associatedRequestHeaders1.put(HTTPSPDYHeader.HOST.name(version()), "localhost:" + connector.getLocalPort()); + associatedRequestHeaders1.put("referer", "http://localhost:" + connector.getLocalPort() + mainResource); + session1.syn(new SynInfo(associatedRequestHeaders1, true), new StreamFrameListener.Adapter() + { + @Override + public void onData(Stream stream, DataInfo dataInfo) + { + dataInfo.consume(dataInfo.length()); + if (dataInfo.isClose()) + associatedResourceLatch1.countDown(); + } + }); + Assert.assertTrue(associatedResourceLatch1.await(5, TimeUnit.SECONDS)); + + final CountDownLatch associatedResourceLatch2 = new CountDownLatch(1); + Headers associatedRequestHeaders2 = new Headers(); + associatedRequestHeaders2.put(HTTPSPDYHeader.METHOD.name(version()), "GET"); + associatedRequestHeaders2.put(HTTPSPDYHeader.URI.name(version()), "/application.js"); + associatedRequestHeaders2.put(HTTPSPDYHeader.VERSION.name(version()), "HTTP/1.1"); + associatedRequestHeaders2.put(HTTPSPDYHeader.SCHEME.name(version()), "http"); + associatedRequestHeaders2.put(HTTPSPDYHeader.HOST.name(version()), "localhost:" + connector.getLocalPort()); + associatedRequestHeaders2.put("referer", "http://localhost:" + connector.getLocalPort() + mainResource); + session1.syn(new SynInfo(associatedRequestHeaders2, true), new StreamFrameListener.Adapter() + { + @Override + public void onData(Stream stream, DataInfo dataInfo) + { + dataInfo.consume(dataInfo.length()); + if (dataInfo.isClose()) + associatedResourceLatch2.countDown(); + } + }); + Assert.assertTrue(associatedResourceLatch2.await(5, TimeUnit.SECONDS)); + + // Create another client, and perform the same request for the main resource, + // we expect the css being pushed, but not the js + + final CountDownLatch mainStreamLatch = new CountDownLatch(2); + final CountDownLatch pushDataLatch = new CountDownLatch(1); + Session session2 = startClient(version(), address, new SessionFrameListener.Adapter() + { + @Override + public StreamFrameListener onSyn(Stream stream, SynInfo synInfo) + { + Assert.assertTrue(stream.isUnidirectional()); + Assert.assertTrue(synInfo.getHeaders().get(HTTPSPDYHeader.URI.name(version())).value().endsWith(".css")); + return new StreamFrameListener.Adapter() + { + @Override + public void onData(Stream stream, DataInfo dataInfo) + { + + dataInfo.consume(dataInfo.length()); + if (dataInfo.isClose()) + pushDataLatch.countDown(); + } + }; + } + }); + session2.syn(new SynInfo(mainRequestHeaders, true), new StreamFrameListener.Adapter() + { + @Override + public void onReply(Stream stream, ReplyInfo replyInfo) + { + Assert.assertFalse(replyInfo.isClose()); + mainStreamLatch.countDown(); + } + + @Override + public void onData(Stream stream, DataInfo dataInfo) + { + dataInfo.consume(dataInfo.length()); + if (dataInfo.isClose()) + mainStreamLatch.countDown(); + } + }); + + Assert.assertTrue(mainStreamLatch.await(5, TimeUnit.SECONDS)); + Assert.assertTrue(pushDataLatch.await(5, TimeUnit.SECONDS)); + } + @Test public void testAssociatedResourceIsPushed() throws Exception { From 5b3999a7f49fe0f7353cec67957c08351b1645a5 Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Tue, 5 Jun 2012 15:42:28 +0200 Subject: [PATCH 4/5] 381712 Anoint all JspServlets with correct environment --- .../jetty/webapp/StandardDescriptorProcessor.java | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/StandardDescriptorProcessor.java b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/StandardDescriptorProcessor.java index db5ec955afa..51800f4d7c6 100644 --- a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/StandardDescriptorProcessor.java +++ b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/StandardDescriptorProcessor.java @@ -239,23 +239,26 @@ public class StandardDescriptorProcessor extends IterativeDescriptorProcessor String servlet_class = node.getString("servlet-class", false, true); // Handle JSP - String jspServletName=null; String jspServletClass=null;; - boolean hasJSP=false; + + //Handle the default jsp servlet instance if (id != null && id.equals("jsp")) { - jspServletName = servlet_name; jspServletClass = servlet_class; try { Loader.loadClass(this.getClass(), servlet_class); - hasJSP = true; } catch (ClassNotFoundException e) { LOG.info("NO JSP Support for {}, did not find {}", context.getContextPath(), servlet_class); jspServletClass = servlet_class = "org.eclipse.jetty.servlet.NoJspServlet"; } + } + + //could be more than 1 declaration of the jsp servlet, so configure them all + if (servlet_class != null && "org.apache.jasper.servlet.JspServlet".equals(servlet_class)) + { if (holder.getInitParameter("scratchdir") == null) { File tmp = context.getTempDirectory(); @@ -316,12 +319,12 @@ public class StandardDescriptorProcessor extends IterativeDescriptorProcessor } } - // Handler JSP file + // Handle JSP file String jsp_file = node.getString("jsp-file", false, true); if (jsp_file != null) { holder.setForcedPath(jsp_file); - holder.setClassName(jspServletClass); + holder.setClassName(jspServletClass); //only use our default instance //set the system classpath explicitly for the holder that will represent the JspServlet instance holder.setInitParameter("com.sun.appserv.jsp.classpath", getSystemClassPath(context)); } From 28c3c3e917d5979d83bca4174473427c55ef571c Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Tue, 5 Jun 2012 16:03:08 +0200 Subject: [PATCH 5/5] 381639 - CrossOriginFilter does not support Access-Control-Expose-Headers. --- .../jetty/servlets/CrossOriginFilter.java | 23 +++++++++++++++---- .../jetty/servlets/CrossOriginFilterTest.java | 21 +++++++++++++++++ 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/CrossOriginFilter.java b/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/CrossOriginFilter.java index bb57608c4b4..eb1b9859959 100644 --- a/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/CrossOriginFilter.java +++ b/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/CrossOriginFilter.java @@ -54,15 +54,18 @@ import org.eclipse.jetty.util.log.Logger; * and any 3 letter top-level domain (.com, .net, .org, etc.). *
  • allowedMethods, a comma separated list of HTTP methods that * are allowed to be used when accessing the resources. Default value is - * GET,POST
    • + * GET,POST,HEAD *
  • allowedHeaders, a comma separated list of HTTP headers that * are allowed to be specified when accessing the resources. Default value - * is X-Requested-With
    • + * is X-Requested-With,Content-Type,Accept,Origin *
  • preflightMaxAge, the number of seconds that preflight requests * can be cached by the client. Default value is 1800 seconds, or 30 * minutes
    • *
  • allowCredentials, a boolean indicating if the resource allows * requests with credentials. Default value is false
    • + *
  • exposeHeaders, a comma separated list of HTTP headers that + * are allowed to be exposed on the client. Default value is the + * empty list
    • *

    *

    A typical configuration could be: * 

    @@ -79,8 +82,6 @@ import org.eclipse.jetty.util.log.Logger;
  *     ...
  * </web-app>
  *

    - * - * @version $Revision$ $Date$ */ public class CrossOriginFilter implements Filter { @@ -96,12 +97,14 @@ public class CrossOriginFilter implements Filter public static final String ACCESS_CONTROL_ALLOW_HEADERS_HEADER = "Access-Control-Allow-Headers"; public static final String ACCESS_CONTROL_MAX_AGE_HEADER = "Access-Control-Max-Age"; public static final String ACCESS_CONTROL_ALLOW_CREDENTIALS_HEADER = "Access-Control-Allow-Credentials"; + public static final String ACCESS_CONTROL_EXPOSE_HEADERS_HEADER = "Access-Control-Expose-Headers"; // Implementation constants public static final String ALLOWED_ORIGINS_PARAM = "allowedOrigins"; public static final String ALLOWED_METHODS_PARAM = "allowedMethods"; public static final String ALLOWED_HEADERS_PARAM = "allowedHeaders"; public static final String PREFLIGHT_MAX_AGE_PARAM = "preflightMaxAge"; public static final String ALLOW_CREDENTIALS_PARAM = "allowCredentials"; + public static final String EXPOSED_HEADERS_PARAM = "exposedHeaders"; private static final String ANY_ORIGIN = "*"; private static final List SIMPLE_HTTP_METHODS = Arrays.asList("GET", "POST", "HEAD"); @@ -109,6 +112,7 @@ public class CrossOriginFilter implements Filter private List allowedOrigins = new ArrayList(); private List allowedMethods = new ArrayList(); private List allowedHeaders = new ArrayList(); + private List exposedHeaders = new ArrayList(); private int preflightMaxAge = 0; private boolean allowCredentials; @@ -163,6 +167,11 @@ public class CrossOriginFilter implements Filter allowedCredentialsConfig = "true"; allowCredentials = Boolean.parseBoolean(allowedCredentialsConfig); + String exposedHeadersConfig = config.getInitParameter(EXPOSED_HEADERS_PARAM); + if (exposedHeadersConfig == null) + exposedHeadersConfig = ""; + exposedHeaders.addAll(Arrays.asList(exposedHeadersConfig.split(","))); + if (LOG.isDebugEnabled()) { LOG.debug("Cross-origin filter configuration: " + @@ -170,7 +179,9 @@ public class CrossOriginFilter implements Filter ALLOWED_METHODS_PARAM + " = " + allowedMethodsConfig + ", " + ALLOWED_HEADERS_PARAM + " = " + allowedHeadersConfig + ", " + PREFLIGHT_MAX_AGE_PARAM + " = " + preflightMaxAgeConfig + ", " + - ALLOW_CREDENTIALS_PARAM + " = " + allowedCredentialsConfig); + ALLOW_CREDENTIALS_PARAM + " = " + allowedCredentialsConfig + "," + + EXPOSED_HEADERS_PARAM + " = " + exposedHeadersConfig + ); } } @@ -305,6 +316,8 @@ public class CrossOriginFilter implements Filter response.setHeader(ACCESS_CONTROL_ALLOW_ORIGIN_HEADER, origin); if (allowCredentials) response.setHeader(ACCESS_CONTROL_ALLOW_CREDENTIALS_HEADER, "true"); + if (!exposedHeaders.isEmpty()) + response.setHeader(ACCESS_CONTROL_EXPOSE_HEADERS_HEADER, commify(exposedHeaders)); } private void handlePreflightResponse(HttpServletRequest request, HttpServletResponse response, String origin) diff --git a/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/CrossOriginFilterTest.java b/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/CrossOriginFilterTest.java index fb8d6bba490..bd631ebf37c 100644 --- a/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/CrossOriginFilterTest.java +++ b/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/CrossOriginFilterTest.java @@ -371,6 +371,27 @@ public class CrossOriginFilterTest Assert.assertTrue(latch.await(1, TimeUnit.SECONDS)); } + @Test + public void testSimpleRequestWithExposedHeaders() throws Exception + { + FilterHolder filterHolder = new FilterHolder(new CrossOriginFilter()); + filterHolder.setInitParameter("exposedHeaders", "Content-Length"); + tester.getContext().addFilter(filterHolder, "/*", FilterMapping.DEFAULT); + + CountDownLatch latch = new CountDownLatch(1); + tester.getContext().addServlet(new ServletHolder(new ResourceServlet(latch)), "/*"); + + String request = "" + + "GET / HTTP/1.1\r\n" + + "Host: localhost\r\n" + + "Origin: http://localhost\r\n" + + "\r\n"; + String response = tester.getResponses(request); + Assert.assertTrue(response.contains("HTTP/1.1 200")); + Assert.assertTrue(response.contains(CrossOriginFilter.ACCESS_CONTROL_EXPOSE_HEADERS_HEADER)); + Assert.assertTrue(latch.await(1, TimeUnit.SECONDS)); + } + public static class ResourceServlet extends HttpServlet { private static final long serialVersionUID = 1L;