From 156f1387805bdadc0c72e547facd6078360b8597 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Wed, 7 Mar 2012 11:24:39 +0100 Subject: [PATCH 1/8] Introduced onException() callback in SessionFrameListener in case of processing exceptions. Added test for unsupported SYN_STREAM versions. --- .../eclipse/jetty/spdy/SessionException.java | 5 ++ .../eclipse/jetty/spdy/StandardSession.java | 37 +++++++-- .../eclipse/jetty/spdy/StreamException.java | 7 -- .../jetty/spdy/api/SessionFrameListener.java | 20 +++++ .../jetty/spdy/parser/ControlFrameParser.java | 11 --- .../spdy/parser/SynStreamBodyParser.java | 12 +++ .../org/eclipse/jetty/spdy/AbstractTest.java | 5 +- .../jetty/spdy/UnsupportedVersionTest.java | 79 +++++++++++++++++++ 8 files changed, 151 insertions(+), 25 deletions(-) create mode 100644 jetty-spdy/spdy-jetty/src/test/java/org/eclipse/jetty/spdy/UnsupportedVersionTest.java diff --git a/jetty-spdy/spdy-core/src/main/java/org/eclipse/jetty/spdy/SessionException.java b/jetty-spdy/spdy-core/src/main/java/org/eclipse/jetty/spdy/SessionException.java index a66e5a01f17..fa9a55e7c3a 100644 --- a/jetty-spdy/spdy-core/src/main/java/org/eclipse/jetty/spdy/SessionException.java +++ b/jetty-spdy/spdy-core/src/main/java/org/eclipse/jetty/spdy/SessionException.java @@ -38,4 +38,9 @@ public class SessionException extends RuntimeException super(cause); this.sessionStatus = sessionStatus; } + + public SessionStatus getSessionStatus() + { + return sessionStatus; + } } diff --git a/jetty-spdy/spdy-core/src/main/java/org/eclipse/jetty/spdy/StandardSession.java b/jetty-spdy/spdy-core/src/main/java/org/eclipse/jetty/spdy/StandardSession.java index c811050864e..2631c656f97 100644 --- a/jetty-spdy/spdy-core/src/main/java/org/eclipse/jetty/spdy/StandardSession.java +++ b/jetty-spdy/spdy-core/src/main/java/org/eclipse/jetty/spdy/StandardSession.java @@ -217,20 +217,30 @@ public class StandardSession implements ISession, Parser.Listener, Handler goAway() + { + return goAway(SessionStatus.OK); + } + + private Future goAway(SessionStatus sessionStatus) { Promise result = new Promise<>(); - goAway(0, TimeUnit.MILLISECONDS, result); + goAway(sessionStatus, 0, TimeUnit.MILLISECONDS, result); return result; } @Override public void goAway(long timeout, TimeUnit unit, Handler handler) + { + goAway(SessionStatus.OK, timeout, unit, handler); + } + + private void goAway(SessionStatus sessionStatus, long timeout, TimeUnit unit, Handler handler) { if (goAwaySent.compareAndSet(false, true)) { if (!goAwayReceived.get()) { - GoAwayFrame frame = new GoAwayFrame(version, lastStreamId.get(), SessionStatus.OK.getCode()); + GoAwayFrame frame = new GoAwayFrame(version, lastStreamId.get(), sessionStatus.getCode()); control(null, frame, timeout, unit, handler, null); return; } @@ -369,15 +379,16 @@ public class StandardSession implements ISession, Parser.Listener, HandlerA {@link SessionFrameListener} is the passive counterpart of a {@link Session} and receives events happening * on a SPDY session.

@@ -105,11 +108,22 @@ public interface SessionFrameListener extends EventListener */ public void onGoAway(Session session, GoAwayInfo goAwayInfo); + /** + *

Callback invoked when an exception is thrown during the processing of an event on a + * SPDY session.

+ *

Examples of such conditions are invalid frames received, corrupted headers compression state, etc.

+ * + * @param x the exception that caused the event processing failure + */ + public void onException(Throwable x); + /** *

Empty implementation of {@link SessionFrameListener}

*/ public static class Adapter implements SessionFrameListener { + private static final Logger logger = LoggerFactory.getLogger(Adapter.class); + @Override public StreamFrameListener onSyn(Stream stream, SynInfo synInfo) { @@ -135,5 +149,11 @@ public interface SessionFrameListener extends EventListener public void onGoAway(Session session, GoAwayInfo goAwayInfo) { } + + @Override + public void onException(Throwable x) + { + logger.info("", x); + } } } diff --git a/jetty-spdy/spdy-core/src/main/java/org/eclipse/jetty/spdy/parser/ControlFrameParser.java b/jetty-spdy/spdy-core/src/main/java/org/eclipse/jetty/spdy/parser/ControlFrameParser.java index 7c9eecf1091..af8b78f6548 100644 --- a/jetty-spdy/spdy-core/src/main/java/org/eclipse/jetty/spdy/parser/ControlFrameParser.java +++ b/jetty-spdy/spdy-core/src/main/java/org/eclipse/jetty/spdy/parser/ControlFrameParser.java @@ -20,9 +20,6 @@ import java.nio.ByteBuffer; import java.util.EnumMap; import org.eclipse.jetty.spdy.CompressionFactory; -import org.eclipse.jetty.spdy.SessionException; -import org.eclipse.jetty.spdy.api.SPDY; -import org.eclipse.jetty.spdy.api.SessionStatus; import org.eclipse.jetty.spdy.frames.ControlFrame; import org.eclipse.jetty.spdy.frames.ControlFrameType; @@ -77,7 +74,6 @@ public abstract class ControlFrameParser if (buffer.remaining() >= 2) { version = (short)(buffer.getShort() & 0x7F_FF); - checkVersion(version); state = State.TYPE; } else @@ -95,7 +91,6 @@ public abstract class ControlFrameParser if (cursor == 0) { version &= 0x7F_FF; - checkVersion(version); state = State.TYPE; } break; @@ -171,12 +166,6 @@ public abstract class ControlFrameParser return false; } - private void checkVersion(short version) - { - if (version != SPDY.V2 && version != SPDY.V3) - throw new SessionException(SessionStatus.PROTOCOL_ERROR, "Unrecognized version " + version); - } - private void reset() { state = State.VERSION; diff --git a/jetty-spdy/spdy-core/src/main/java/org/eclipse/jetty/spdy/parser/SynStreamBodyParser.java b/jetty-spdy/spdy-core/src/main/java/org/eclipse/jetty/spdy/parser/SynStreamBodyParser.java index 56de363bc28..986b3751414 100644 --- a/jetty-spdy/spdy-core/src/main/java/org/eclipse/jetty/spdy/parser/SynStreamBodyParser.java +++ b/jetty-spdy/spdy-core/src/main/java/org/eclipse/jetty/spdy/parser/SynStreamBodyParser.java @@ -19,8 +19,10 @@ package org.eclipse.jetty.spdy.parser; import java.nio.ByteBuffer; import org.eclipse.jetty.spdy.CompressionFactory; +import org.eclipse.jetty.spdy.StreamException; import org.eclipse.jetty.spdy.api.Headers; import org.eclipse.jetty.spdy.api.SPDY; +import org.eclipse.jetty.spdy.api.StreamStatus; import org.eclipse.jetty.spdy.api.SynInfo; import org.eclipse.jetty.spdy.frames.ControlFrameType; import org.eclipse.jetty.spdy.frames.SynStreamFrame; @@ -77,6 +79,10 @@ public class SynStreamBodyParser extends ControlFrameBodyParser } case ASSOCIATED_STREAM_ID: { + // Now we know the streamId, we can do the version check + // and if it is wrong, issue a RST_STREAM + checkVersion(controlFrameParser.getVersion(), streamId); + if (buffer.remaining() >= 4) { associatedStreamId = buffer.getInt() & 0x7F_FF_FF_FF; @@ -145,6 +151,12 @@ public class SynStreamBodyParser extends ControlFrameBodyParser return false; } + private void checkVersion(short version, int streamId) + { + if (version != SPDY.V2 && version != SPDY.V3) + throw new StreamException(streamId, StreamStatus.UNSUPPORTED_VERSION); + } + private byte readPriority(short version, byte currByte) { // Right shift retains the sign bit when operated on a byte, diff --git a/jetty-spdy/spdy-jetty/src/test/java/org/eclipse/jetty/spdy/AbstractTest.java b/jetty-spdy/spdy-jetty/src/test/java/org/eclipse/jetty/spdy/AbstractTest.java index ba92c9307ca..b30db27724d 100644 --- a/jetty-spdy/spdy-jetty/src/test/java/org/eclipse/jetty/spdy/AbstractTest.java +++ b/jetty-spdy/spdy-jetty/src/test/java/org/eclipse/jetty/spdy/AbstractTest.java @@ -53,9 +53,10 @@ public abstract class AbstractTest protected InetSocketAddress startServer(ServerSessionFrameListener listener) throws Exception { - server = new Server(); - connector = newSPDYServerConnector(listener); + if (connector == null) + connector = newSPDYServerConnector(listener); connector.setPort(0); + server = new Server(); server.addConnector(connector); server.start(); return new InetSocketAddress("localhost", connector.getLocalPort()); diff --git a/jetty-spdy/spdy-jetty/src/test/java/org/eclipse/jetty/spdy/UnsupportedVersionTest.java b/jetty-spdy/spdy-jetty/src/test/java/org/eclipse/jetty/spdy/UnsupportedVersionTest.java new file mode 100644 index 00000000000..78f2163272d --- /dev/null +++ b/jetty-spdy/spdy-jetty/src/test/java/org/eclipse/jetty/spdy/UnsupportedVersionTest.java @@ -0,0 +1,79 @@ +package org.eclipse.jetty.spdy; + +import java.net.InetSocketAddress; +import java.nio.ByteBuffer; +import java.nio.channels.SocketChannel; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; + +import org.eclipse.jetty.spdy.api.Headers; +import org.eclipse.jetty.spdy.api.SPDY; +import org.eclipse.jetty.spdy.api.Stream; +import org.eclipse.jetty.spdy.api.StreamFrameListener; +import org.eclipse.jetty.spdy.api.StreamStatus; +import org.eclipse.jetty.spdy.api.SynInfo; +import org.eclipse.jetty.spdy.api.server.ServerSessionFrameListener; +import org.eclipse.jetty.spdy.frames.ControlFrame; +import org.eclipse.jetty.spdy.frames.ControlFrameType; +import org.eclipse.jetty.spdy.frames.RstStreamFrame; +import org.eclipse.jetty.spdy.frames.SynStreamFrame; +import org.eclipse.jetty.spdy.generator.Generator; +import org.eclipse.jetty.spdy.parser.Parser; +import org.junit.Assert; +import org.junit.Test; + +public class UnsupportedVersionTest extends AbstractTest +{ + @Test + public void testSynWithUnsupportedVersion() throws Exception + { + final CountDownLatch synLatch = new CountDownLatch(1); + InetSocketAddress address = startServer(new ServerSessionFrameListener.Adapter() + { + @Override + public StreamFrameListener onSyn(Stream stream, SynInfo synInfo) + { + synLatch.countDown(); + return null; + } + + @Override + public void onException(Throwable x) + { + // Suppress exception logging for this test + } + }); + + SynStreamFrame frame = new SynStreamFrame(SPDY.V2, SynInfo.FLAG_CLOSE, 1, 0, (byte)0, new Headers()); + Generator generator = new Generator(new StandardByteBufferPool(), new StandardCompressionFactory.StandardCompressor()); + ByteBuffer buffer = generator.control(frame); + // Replace the version byte with an unsupported version + buffer.putShort(0, (short)0x8001); + + SocketChannel channel = SocketChannel.open(address); + channel.write(buffer); + Assert.assertFalse(buffer.hasRemaining()); + + Assert.assertFalse(synLatch.await(1, TimeUnit.SECONDS)); + + buffer = ByteBuffer.allocate(1024); + channel.read(buffer); + buffer.flip(); + + final CountDownLatch rstLatch = new CountDownLatch(1); + Parser parser = new Parser(new StandardCompressionFactory.StandardDecompressor()); + parser.addListener(new Parser.Listener.Adapter() + { + @Override + public void onControlFrame(ControlFrame frame) + { + Assert.assertSame(ControlFrameType.RST_STREAM, frame.getType()); + Assert.assertEquals(StreamStatus.UNSUPPORTED_VERSION.getCode(frame.getVersion()), ((RstStreamFrame)frame).getStatusCode()); + rstLatch.countDown(); + } + }); + parser.parse(buffer); + + Assert.assertTrue(rstLatch.await(5, TimeUnit.SECONDS)); + } +} From c67357f005700f0af19e7b760b213b245cb35059 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Wed, 7 Mar 2012 11:55:20 +0100 Subject: [PATCH 2/8] Fixed handling of unknown control frames. --- .../parser/UnknownControlFrameBodyParser.java | 42 ++++++++++-- .../spdy/parser/UnknownControlFrameTest.java | 64 +++++++++++++++++++ 2 files changed, 101 insertions(+), 5 deletions(-) create mode 100644 jetty-spdy/spdy-core/src/test/java/org/eclipse/jetty/spdy/parser/UnknownControlFrameTest.java diff --git a/jetty-spdy/spdy-core/src/main/java/org/eclipse/jetty/spdy/parser/UnknownControlFrameBodyParser.java b/jetty-spdy/spdy-core/src/main/java/org/eclipse/jetty/spdy/parser/UnknownControlFrameBodyParser.java index 2083249e8d8..ea5890ff6f0 100644 --- a/jetty-spdy/spdy-core/src/main/java/org/eclipse/jetty/spdy/parser/UnknownControlFrameBodyParser.java +++ b/jetty-spdy/spdy-core/src/main/java/org/eclipse/jetty/spdy/parser/UnknownControlFrameBodyParser.java @@ -20,19 +20,51 @@ import java.nio.ByteBuffer; public class UnknownControlFrameBodyParser extends ControlFrameBodyParser { + private final ControlFrameParser controlFrameParser; + private State state = State.BODY; private int remaining; public UnknownControlFrameBodyParser(ControlFrameParser controlFrameParser) { - this.remaining = controlFrameParser.getLength(); + this.controlFrameParser = controlFrameParser; } @Override public boolean parse(ByteBuffer buffer) { - int consumed = Math.min(remaining, buffer.remaining()); - buffer.position(buffer.position() + consumed); - remaining -= consumed; - return remaining == 0; + switch (state) + { + case BODY: + { + remaining = controlFrameParser.getLength(); + state = State.CONSUME; + // Fall down + } + case CONSUME: + { + int consume = Math.min(remaining, buffer.remaining()); + buffer.position(buffer.position() + consume); + remaining -= consume; + if (remaining > 0) + return false; + reset(); + return true; + } + default: + { + throw new IllegalStateException(); + } + } + } + + private void reset() + { + state = State.BODY; + remaining = 0; + } + + private enum State + { + BODY, CONSUME } } diff --git a/jetty-spdy/spdy-core/src/test/java/org/eclipse/jetty/spdy/parser/UnknownControlFrameTest.java b/jetty-spdy/spdy-core/src/test/java/org/eclipse/jetty/spdy/parser/UnknownControlFrameTest.java new file mode 100644 index 00000000000..750f407d4b4 --- /dev/null +++ b/jetty-spdy/spdy-core/src/test/java/org/eclipse/jetty/spdy/parser/UnknownControlFrameTest.java @@ -0,0 +1,64 @@ +package org.eclipse.jetty.spdy.parser; + +import java.nio.ByteBuffer; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; + +import org.eclipse.jetty.spdy.SessionException; +import org.eclipse.jetty.spdy.StandardByteBufferPool; +import org.eclipse.jetty.spdy.StandardCompressionFactory; +import org.eclipse.jetty.spdy.StreamException; +import org.eclipse.jetty.spdy.api.Headers; +import org.eclipse.jetty.spdy.api.SPDY; +import org.eclipse.jetty.spdy.api.SynInfo; +import org.eclipse.jetty.spdy.frames.ControlFrame; +import org.eclipse.jetty.spdy.frames.DataFrame; +import org.eclipse.jetty.spdy.frames.SynStreamFrame; +import org.eclipse.jetty.spdy.generator.Generator; +import org.junit.Assert; +import org.junit.Test; + +public class UnknownControlFrameTest +{ + @Test + public void testUnknownControlFrame() throws Exception + { + SynStreamFrame frame = new SynStreamFrame(SPDY.V2, SynInfo.FLAG_CLOSE, 1, 0, (byte)0, new Headers()); + Generator generator = new Generator(new StandardByteBufferPool(), new StandardCompressionFactory.StandardCompressor()); + ByteBuffer buffer = generator.control(frame); + // Change the frame type to unknown + buffer.putShort(2, (short)0); + + final CountDownLatch latch = new CountDownLatch(1); + Parser parser = new Parser(new StandardCompressionFactory.StandardDecompressor()); + parser.addListener(new Parser.Listener.Adapter() + { + @Override + public void onControlFrame(ControlFrame frame) + { + latch.countDown(); + } + + @Override + public void onDataFrame(DataFrame frame, ByteBuffer data) + { + latch.countDown(); + } + + @Override + public void onStreamException(StreamException x) + { + latch.countDown(); + } + + @Override + public void onSessionException(SessionException x) + { + latch.countDown(); + } + }); + parser.parse(buffer); + + Assert.assertFalse(latch.await(1, TimeUnit.SECONDS)); + } +} From 8f37221b56b1aaf7754b746309d3b0712f673238 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Wed, 7 Mar 2012 13:40:58 +0100 Subject: [PATCH 3/8] Updated v3 stream status codes. --- .../org/eclipse/jetty/spdy/api/StreamStatus.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/jetty-spdy/spdy-core/src/main/java/org/eclipse/jetty/spdy/api/StreamStatus.java b/jetty-spdy/spdy-core/src/main/java/org/eclipse/jetty/spdy/api/StreamStatus.java index 7d8b4d00e23..c9908c8a146 100644 --- a/jetty-spdy/spdy-core/src/main/java/org/eclipse/jetty/spdy/api/StreamStatus.java +++ b/jetty-spdy/spdy-core/src/main/java/org/eclipse/jetty/spdy/api/StreamStatus.java @@ -47,27 +47,27 @@ public enum StreamStatus /** *

The stream status indicating an implementation error

*/ - INTERNAL_ERROR(6, 11), + INTERNAL_ERROR(6, 6), /** *

The stream status indicating a flow control error

*/ - FLOW_CONTROL_ERROR(7, 6), + FLOW_CONTROL_ERROR(7, 7), /** *

The stream status indicating a stream opened more than once

*/ - STREAM_IN_USE(-1, 7), + STREAM_IN_USE(-1, 8), /** *

The stream status indicating data on a stream already closed

*/ - STREAM_ALREADY_CLOSED(-1, 8), + STREAM_ALREADY_CLOSED(-1, 9), /** *

The stream status indicating credentials not valid

*/ - INVALID_CREDENTIALS(-1, 9), + INVALID_CREDENTIALS(-1, 10), /** *

The stream status indicating that the implementation could not support a frame too large

*/ - FRAME_TOO_LARGE(-1, 10); + FRAME_TOO_LARGE(-1, 11); /** * @param version the SPDY protocol version From c5c4425a0bab5439348709e6dc15e0e0d8aac56a Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Wed, 7 Mar 2012 14:04:27 +0100 Subject: [PATCH 4/8] Ignoring data frames after RST_STREAM. --- .../eclipse/jetty/spdy/StandardSession.java | 50 +++---- .../eclipse/jetty/spdy/ResetStreamTest.java | 123 ++++++++++++++++++ 2 files changed, 150 insertions(+), 23 deletions(-) create mode 100644 jetty-spdy/spdy-jetty/src/test/java/org/eclipse/jetty/spdy/ResetStreamTest.java diff --git a/jetty-spdy/spdy-core/src/main/java/org/eclipse/jetty/spdy/StandardSession.java b/jetty-spdy/spdy-core/src/main/java/org/eclipse/jetty/spdy/StandardSession.java index 2631c656f97..a3db9e860cd 100644 --- a/jetty-spdy/spdy-core/src/main/java/org/eclipse/jetty/spdy/StandardSession.java +++ b/jetty-spdy/spdy-core/src/main/java/org/eclipse/jetty/spdy/StandardSession.java @@ -135,7 +135,7 @@ public class StandardSession implements ISession, Parser.Listener, Handler handler) + public void syn(SynInfo synInfo, StreamFrameListener listener, long timeout, TimeUnit unit, Handler handler) { // Synchronization is necessary. // SPEC v3, 2.3.1 requires that the stream creation be monotonically crescent @@ -154,7 +154,7 @@ public class StandardSession implements ISession, Parser.Listener, Handler handler) + public void ping(long timeout, TimeUnit unit, Handler handler) { int pingId = pingIds.getAndAdd(2); PingInfo pingInfo = new PingInfo(pingId); @@ -330,7 +334,7 @@ public class StandardSession implements ISession, Parser.Listener, Handler void control(IStream stream, ControlFrame frame, long timeout, TimeUnit unit, final Handler handler, C context) + public void control(IStream stream, ControlFrame frame, long timeout, TimeUnit unit, Handler handler, C context) { try { @@ -732,7 +736,7 @@ public class StandardSession implements ISession, Parser.Listener, Handler handler, FrameBytes frameBytes) + protected void write(ByteBuffer buffer, Handler handler, FrameBytes frameBytes) { if (controller != null) controller.write(buffer, handler, frameBytes); @@ -937,7 +941,7 @@ public class StandardSession implements ISession, Parser.Listener, Handler serverSessionRef = new AtomicReference<>(); + final CountDownLatch synLatch = new CountDownLatch(1); + final CountDownLatch rstLatch = new CountDownLatch(1); + Session clientSession = startClient(startServer(new ServerSessionFrameListener.Adapter() + { + @Override + public StreamFrameListener onSyn(Stream stream, SynInfo synInfo) + { + Session serverSession = stream.getSession(); + serverSessionRef.set(serverSession); + serverSession.rst(new RstInfo(stream.getId(), StreamStatus.REFUSED_STREAM)); + synLatch.countDown(); + return null; + } + }), new SessionFrameListener.Adapter() + { + @Override + public void onRst(Session session, RstInfo rstInfo) + { + rstLatch.countDown(); + } + }); + + clientSession.syn(new SynInfo(false), null).get(5, TimeUnit.SECONDS); + + Assert.assertTrue(synLatch.await(5, TimeUnit.SECONDS)); + Session serverSession = serverSessionRef.get(); + Assert.assertEquals(0, serverSession.getStreams().size()); + + Assert.assertTrue(rstLatch.await(5, TimeUnit.SECONDS)); + Assert.assertEquals(0, clientSession.getStreams().size()); + } + + @Test + public void testRefusedStreamIgnoresData() throws Exception + { + final CountDownLatch synLatch = new CountDownLatch(1); + final CountDownLatch dataLatch = new CountDownLatch(1); + final CountDownLatch rstLatch = new CountDownLatch(1); + Session session = startClient(startServer(new ServerSessionFrameListener.Adapter() + { + @Override + public StreamFrameListener onSyn(Stream stream, SynInfo synInfo) + { + try + { + // Refuse the stream, we must ignore data frames + Assert.assertTrue(synLatch.await(5, TimeUnit.SECONDS)); + stream.getSession().rst(new RstInfo(stream.getId(), StreamStatus.REFUSED_STREAM)); + return new StreamFrameListener.Adapter() + { + @Override + public void onData(Stream stream, DataInfo dataInfo) + { + dataLatch.countDown(); + } + }; + } + catch (InterruptedException x) + { + x.printStackTrace(); + return null; + } + } + }), new SessionFrameListener.Adapter() + { + @Override + public void onRst(Session session, RstInfo rstInfo) + { + rstLatch.countDown(); + } + }); + + Stream stream = session.syn(new SynInfo(false), null).get(5, TimeUnit.SECONDS); + stream.data(new StringDataInfo("data", true), 5, TimeUnit.SECONDS, new Handler.Adapter() + { + @Override + public void completed(Void context) + { + synLatch.countDown(); + } + }); + + Assert.assertTrue(rstLatch.await(5, TimeUnit.SECONDS)); + Assert.assertFalse(dataLatch.await(1, TimeUnit.SECONDS)); + } +} From 3442e79b384171eebb4a88087e001a1b54ab7529 Mon Sep 17 00:00:00 2001 From: Jesse McConnell Date: Wed, 7 Mar 2012 17:21:38 -0600 Subject: [PATCH 5/8] updated gzipfilter and user agent filter to take excluded paths, excluded path patterns, and excluded agent patterns. The latter two take regular expressions in order to match paths and user agents, thanks travis --- .../eclipse/jetty/servlets/GzipFilter.java | 118 ++++++++++++++++-- .../jetty/servlets/UserAgentFilter.java | 37 +++--- 2 files changed, 129 insertions(+), 26 deletions(-) diff --git a/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/GzipFilter.java b/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/GzipFilter.java index cf2a898ef54..1b993e6eedf 100644 --- a/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/GzipFilter.java +++ b/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/GzipFilter.java @@ -20,6 +20,7 @@ import java.io.UnsupportedEncodingException; import java.util.HashSet; import java.util.Set; import java.util.StringTokenizer; +import java.util.regex.Pattern; import javax.servlet.FilterChain; import javax.servlet.FilterConfig; @@ -67,12 +68,17 @@ public class GzipFilter extends UserAgentFilter protected Set _mimeTypes; protected int _bufferSize=8192; protected int _minGzipSize=256; - protected Set _excluded; + protected Set _excludedAgents; + protected Set _excludedAgentPatterns; + protected Set _excludedPaths; + protected Set _excludedPathPatterns; + /* ------------------------------------------------------------ */ /** * @see org.eclipse.jetty.servlets.UserAgentFilter#init(javax.servlet.FilterConfig) */ + @Override public void init(FilterConfig filterConfig) throws ServletException { super.init(filterConfig); @@ -93,21 +99,48 @@ public class GzipFilter extends UserAgentFilter while (tok.hasMoreTokens()) _mimeTypes.add(tok.nextToken()); } - tmp=filterConfig.getInitParameter("excludedAgents"); if (tmp!=null) { - _excluded=new HashSet(); + _excludedAgents=new HashSet(); StringTokenizer tok = new StringTokenizer(tmp,",",false); while (tok.hasMoreTokens()) - _excluded.add(tok.nextToken()); + _excludedAgents.add(tok.nextToken()); } + + tmp=filterConfig.getInitParameter("excludeAgentPatterns"); + if (tmp!=null) + { + _excludedAgentPatterns=new HashSet(); + StringTokenizer tok = new StringTokenizer(tmp,",",false); + while (tok.hasMoreTokens()) + _excludedAgentPatterns.add(Pattern.compile(tok.nextToken())); + } + + tmp=filterConfig.getInitParameter("excludePaths"); + if (tmp!=null) + { + _excludedPaths=new HashSet(); + StringTokenizer tok = new StringTokenizer(tmp,",",false); + while (tok.hasMoreTokens()) + _excludedPaths.add(tok.nextToken()); + } + + tmp=filterConfig.getInitParameter("excludePathPatterns"); + if (tmp!=null) + { + _excludedPathPatterns=new HashSet(); + StringTokenizer tok = new StringTokenizer(tmp,",",false); + while (tok.hasMoreTokens()) + _excludedPathPatterns.add(Pattern.compile(tok.nextToken())); + } } /* ------------------------------------------------------------ */ /** * @see org.eclipse.jetty.servlets.UserAgentFilter#destroy() */ + @Override public void destroy() { } @@ -116,6 +149,7 @@ public class GzipFilter extends UserAgentFilter /** * @see org.eclipse.jetty.servlets.UserAgentFilter#doFilter(javax.servlet.ServletRequest, javax.servlet.ServletResponse, javax.servlet.FilterChain) */ + @Override public void doFilter(ServletRequest req, ServletResponse res, FilterChain chain) throws IOException, ServletException { @@ -126,14 +160,17 @@ public class GzipFilter extends UserAgentFilter if (ae != null && ae.indexOf("gzip")>=0 && !response.containsHeader("Content-Encoding") && !HttpMethods.HEAD.equalsIgnoreCase(request.getMethod())) { - if (_excluded!=null) + String ua = getUserAgent(request); + if (isExcludedAgent(ua)) { - String ua=getUserAgent(request); - if (_excluded.contains(ua)) - { - super.doFilter(request,response,chain); - return; - } + super.doFilter(request,response,chain); + return; + } + String requestURI = request.getRequestURI(); + if (isExcludedPath(requestURI)) + { + super.doFilter(request,response,chain); + return; } final GzipResponseWrapper wrappedResponse=newGzipResponseWrapper(request,response); @@ -181,7 +218,64 @@ public class GzipFilter extends UserAgentFilter super.doFilter(request,response,chain); } } - + + /** + * Checks to see if the UserAgent is excluded + * + * @param ua + * the user agent + * @return boolean true if excluded + */ + private boolean isExcludedAgent(String ua) + { + if (ua == null) + return false; + + if (_excludedAgents != null) + { + if (_excludedAgents.contains(ua)) + { + return true; + } + } + else if (_excludedAgentPatterns != null) + { + for (Pattern pattern : _excludedAgentPatterns) + { + if (pattern.matcher(ua).matches()) + { + return true; + } + } + } + + return false; + } + + /** + * Checks to see if the Path is excluded + * + * @param ua + * the request uri + * @return boolean true if excluded + */ + private boolean isExcludedPath(String requestURI) + { + if (requestURI == null) + return false; + if (_excludedPathPatterns != null) + { + for (Pattern pattern : _excludedPathPatterns) + { + if (pattern.matcher(requestURI).matches()) + { + return true; + } + } + } + return false; + } + /** * Allows derived implementations to replace ResponseWrapper implementation. * diff --git a/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/UserAgentFilter.java b/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/UserAgentFilter.java index 78ef05ced14..71d60024c62 100644 --- a/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/UserAgentFilter.java +++ b/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/UserAgentFilter.java @@ -118,29 +118,38 @@ public class UserAgentFilter implements Filter String tag = (String)_agentCache.get(ua); - if (tag==null) + if (tag == null) { - Matcher matcher=_pattern.matcher(ua); - if (matcher.matches()) + if (_pattern != null) { - if(matcher.groupCount()>0) + Matcher matcher = _pattern.matcher(ua); + if (matcher.matches()) { - for (int g=1;g<=matcher.groupCount();g++) + if (matcher.groupCount() > 0) { - String group=matcher.group(g); - if (group!=null) - tag=tag==null?group:(tag+group); + for (int g = 1; g <= matcher.groupCount(); g++) + { + String group = matcher.group(g); + if (group != null) + tag = tag == null?group:(tag + group); + } + } + else + { + tag = matcher.group(); } } - else - tag=matcher.group(); } else - tag=ua; + { + tag = ua; + } - if (_agentCache.size()>=_agentCacheSize) - _agentCache.clear(); - _agentCache.put(ua,tag); + if (_agentCache.size() >= _agentCacheSize) + { + _agentCache.clear(); + } + _agentCache.put(ua,tag); } return tag; From c66f3d2a9d4b15eadbe2be0613005280536b5e10 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Thu, 8 Mar 2012 10:16:47 +0100 Subject: [PATCH 6/8] Removed @Override in interface methods to compile with JDK5. --- .../server/handler/ConnectHandlerSSLTest.java | 13 ++++------ .../jetty/server/ssl/SSLCloseTest.java | 24 +++++++------------ 2 files changed, 12 insertions(+), 25 deletions(-) diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ConnectHandlerSSLTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ConnectHandlerSSLTest.java index 18e4cd25a3e..25284911cae 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ConnectHandlerSSLTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ConnectHandlerSSLTest.java @@ -1,8 +1,5 @@ package org.eclipse.jetty.server.handler; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; - import java.io.BufferedReader; import java.io.ByteArrayOutputStream; import java.io.IOException; @@ -13,7 +10,6 @@ import java.net.Socket; import java.security.SecureRandom; import java.security.cert.CertificateException; import java.security.cert.X509Certificate; - import javax.net.ssl.SSLContext; import javax.net.ssl.SSLSocket; import javax.net.ssl.SSLSocketFactory; @@ -31,6 +27,9 @@ import org.eclipse.jetty.util.ssl.SslContextFactory; import org.junit.BeforeClass; import org.junit.Test; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; + /** * @version $Revision$ $Date$ */ @@ -85,7 +84,7 @@ public class ConnectHandlerSSLTest extends AbstractConnectHandlerTest output = sslSocket.getOutputStream(); input = new BufferedReader(new InputStreamReader(sslSocket.getInputStream())); - request = + request = "GET /echo HTTP/1.1\r\n" + "Host: " + hostPort + "\r\n" + "\r\n"; @@ -178,17 +177,14 @@ public class ConnectHandlerSSLTest extends AbstractConnectHandlerTest private class AlwaysTrustManager implements X509TrustManager { - @Override public void checkClientTrusted(X509Certificate[] x509Certificates, String s) throws CertificateException { } - @Override public void checkServerTrusted(X509Certificate[] x509Certificates, String s) throws CertificateException { } - @Override public X509Certificate[] getAcceptedIssuers() { return new X509Certificate[]{}; @@ -197,7 +193,6 @@ public class ConnectHandlerSSLTest extends AbstractConnectHandlerTest private static class ServerHandler extends AbstractHandler { - @Override public void handle(String target, Request request, HttpServletRequest httpRequest, HttpServletResponse httpResponse) throws IOException, ServletException { request.setHandled(true); diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/ssl/SSLCloseTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/ssl/SSLCloseTest.java index c55f8d077d6..eb927154972 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/ssl/SSLCloseTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/ssl/SSLCloseTest.java @@ -3,7 +3,7 @@ //------------------------------------------------------------------------ //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 +//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, @@ -27,7 +27,6 @@ import java.io.OutputStream; import java.net.Socket; import java.security.cert.CertificateException; import java.security.cert.X509Certificate; - import javax.net.ssl.SSLContext; import javax.net.ssl.TrustManager; import javax.net.ssl.X509TrustManager; @@ -36,7 +35,6 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import junit.framework.TestCase; - import org.eclipse.jetty.io.AsyncEndPoint; import org.eclipse.jetty.server.Connector; import org.eclipse.jetty.server.Request; @@ -51,25 +49,20 @@ public class SSLCloseTest extends TestCase private static AsyncEndPoint __endp; private static class CredulousTM implements TrustManager, X509TrustManager { - @Override public X509Certificate[] getAcceptedIssuers() { return new X509Certificate[]{}; } - @Override public void checkClientTrusted(X509Certificate[] arg0, String arg1) throws CertificateException { - return; } - @Override public void checkServerTrusted(X509Certificate[] arg0, String arg1) throws CertificateException { - return; } } - + private static final TrustManager[] s_dummyTrustManagers=new TrustManager[] { new CredulousTM() }; // ~ Methods @@ -77,7 +70,7 @@ public class SSLCloseTest extends TestCase /** * Feed the server the entire request at once. - * + * * @throws Exception */ public void testClose() throws Exception @@ -86,7 +79,7 @@ public class SSLCloseTest extends TestCase SslSelectChannelConnector connector=new SslSelectChannelConnector(); String keystore = System.getProperty("user.dir")+File.separator+"src"+File.separator+"test"+File.separator+"resources"+File.separator+"keystore"; - + connector.setPort(0); connector.getSslContextFactory().setKeyStorePath(keystore); connector.getSslContextFactory().setKeyStorePassword("storepwd"); @@ -95,7 +88,7 @@ public class SSLCloseTest extends TestCase server.setConnectors(new Connector[] { connector }); server.setHandler(new WriteHandler()); - + server.start(); @@ -110,7 +103,7 @@ public class SSLCloseTest extends TestCase os.write("GET /test HTTP/1.1\r\nHost:test\r\nConnection:close\r\n\r\n".getBytes()); os.flush(); - + BufferedReader in =new BufferedReader(new InputStreamReader(socket.getInputStream())); String line; @@ -126,13 +119,12 @@ public class SSLCloseTest extends TestCase while ((line=in.readLine())!=null) System.err.println(line); - + } private static class WriteHandler extends AbstractHandler { - @Override public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException { try @@ -141,7 +133,7 @@ public class SSLCloseTest extends TestCase response.setStatus(200); response.setHeader("test","value"); __endp=(AsyncEndPoint)baseRequest.getConnection().getEndPoint(); - + OutputStream out=response.getOutputStream(); String data = "Now is the time for all good men to come to the aid of the party.\n"; From 0ab47e41bc7a59d4814ca720a9009af03a3fc27f Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Thu, 8 Mar 2012 11:42:14 +0100 Subject: [PATCH 7/8] Fixed NPE in case user agent pattern does not match. --- .../jetty/servlets/UserAgentFilter.java | 43 ++++++++----------- 1 file changed, 19 insertions(+), 24 deletions(-) diff --git a/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/UserAgentFilter.java b/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/UserAgentFilter.java index 71d60024c62..c489c9c21de 100644 --- a/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/UserAgentFilter.java +++ b/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/UserAgentFilter.java @@ -4,11 +4,11 @@ // All rights reserved. This program and the accompanying materials // are made available under the terms of the Eclipse Public License v1.0 // and Apache License v2.0 which accompanies this distribution. -// The Eclipse Public License is available at +// The Eclipse Public License is available at // http://www.eclipse.org/legal/epl-v10.html // The Apache License v2.0 is available at // http://www.opensource.org/licenses/apache2.0.php -// You may elect to redistribute this code under either of these licenses. +// You may elect to redistribute this code under either of these licenses. // ======================================================================== package org.eclipse.jetty.servlets; @@ -18,7 +18,6 @@ import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.regex.Matcher; import java.util.regex.Pattern; - import javax.servlet.Filter; import javax.servlet.FilterChain; import javax.servlet.FilterConfig; @@ -39,20 +38,20 @@ import javax.servlet.http.HttpServletRequest; *
attribute
If set, then the request attribute of this name is set with the matched user agent string
*
cacheSize
The size of the user-agent cache, used to avoid reparsing of user agent strings. The entire cache is flushed * when this size is reached
- *
userAgent
A regex {@link Pattern} to extract the essential elements of the user agent. + *
userAgent
A regex {@link Pattern} to extract the essential elements of the user agent. * The concatenation of matched pattern groups is used as the user agent name
- *
+ *
* An example value for pattern is (?:Mozilla[^\(]*\(compatible;\s*+([^;]*);.*)|(?:.*?([^\s]+/[^\s]+).*). These two * pattern match the common compatibility user-agent strings and extract the real user agent, failing that, the first - * element of the agent string is returned. - * + * element of the agent string is returned. + * * */ public class UserAgentFilter implements Filter { private static final String __defaultPattern = "(?:Mozilla[^\\(]*\\(compatible;\\s*+([^;]*);.*)|(?:.*?([^\\s]+/[^\\s]+).*)"; private Pattern _pattern = Pattern.compile(__defaultPattern); - private Map _agentCache = new ConcurrentHashMap(); + private Map _agentCache = new ConcurrentHashMap(); private int _agentCacheSize=1024; private String _attribute; @@ -71,7 +70,7 @@ public class UserAgentFilter implements Filter public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException { if (_attribute!=null && _pattern!=null) - { + { String ua=getUserAgent(request); request.setAttribute(_attribute,ua); } @@ -85,11 +84,11 @@ public class UserAgentFilter implements Filter public void init(FilterConfig filterConfig) throws ServletException { _attribute=filterConfig.getInitParameter("attribute"); - + String p=filterConfig.getInitParameter("userAgent"); if (p!=null) _pattern=Pattern.compile(p); - + String size=filterConfig.getInitParameter("cacheSize"); if (size!=null) _agentCacheSize=Integer.parseInt(size); @@ -101,7 +100,7 @@ public class UserAgentFilter implements Filter String ua=((HttpServletRequest)request).getHeader("User-Agent"); return getUserAgent(ua); } - + /* ------------------------------------------------------------ */ /** Get UserAgent. * The configured agent patterns are used to match against the passed user agent string. @@ -112,11 +111,10 @@ public class UserAgentFilter implements Filter */ public String getUserAgent(String ua) { - if (ua==null) + if (ua == null) return null; - - String tag = (String)_agentCache.get(ua); - + + String tag = _agentCache.get(ua); if (tag == null) { @@ -131,7 +129,7 @@ public class UserAgentFilter implements Filter { String group = matcher.group(g); if (group != null) - tag = tag == null?group:(tag + group); + tag = tag == null ? group : tag + group; } } else @@ -140,18 +138,15 @@ public class UserAgentFilter implements Filter } } } - else - { + + if (tag == null) tag = ua; - } if (_agentCache.size() >= _agentCacheSize) - { _agentCache.clear(); - } - _agentCache.put(ua,tag); - + _agentCache.put(ua, tag); } + return tag; } } From c559383bb20278768aae68b3d29748c09cda4347 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Thu, 8 Mar 2012 12:16:51 +0100 Subject: [PATCH 8/8] Fixed test refactoring common parts and always destroying the "env" subcontext at end of tests. --- .../resources/TestResourceAnnotations.java | 71 ++++++++++--------- 1 file changed, 36 insertions(+), 35 deletions(-) diff --git a/jetty-annotations/src/test/java/org/eclipse/jetty/annotations/resources/TestResourceAnnotations.java b/jetty-annotations/src/test/java/org/eclipse/jetty/annotations/resources/TestResourceAnnotations.java index af1f0aa3fd2..451e899b95f 100644 --- a/jetty-annotations/src/test/java/org/eclipse/jetty/annotations/resources/TestResourceAnnotations.java +++ b/jetty-annotations/src/test/java/org/eclipse/jetty/annotations/resources/TestResourceAnnotations.java @@ -1,22 +1,19 @@ package org.eclipse.jetty.annotations.resources; import java.lang.reflect.Field; -import java.util.ArrayList; import java.util.List; import javax.naming.Context; import javax.naming.InitialContext; import org.eclipse.jetty.annotations.AnnotationIntrospector; -import org.eclipse.jetty.annotations.AnnotationParser; -import org.eclipse.jetty.annotations.ClassNameResolver; import org.eclipse.jetty.annotations.ResourceAnnotationHandler; import org.eclipse.jetty.annotations.ResourcesAnnotationHandler; import org.eclipse.jetty.plus.annotation.Injection; import org.eclipse.jetty.plus.annotation.InjectionCollection; import org.eclipse.jetty.server.Server; -import org.eclipse.jetty.util.resource.Resource; -import org.eclipse.jetty.webapp.MetaData; import org.eclipse.jetty.webapp.WebAppContext; +import org.junit.After; +import org.junit.Before; import org.junit.Test; import static org.junit.Assert.assertEquals; @@ -24,24 +21,39 @@ import static org.junit.Assert.assertNotNull; public class TestResourceAnnotations { - Object objA=new Integer(1000); - Object objB=new Integer(2000); - + private Server server; + private WebAppContext wac; + private InjectionCollection injections; + private Context comp; + private Context env; + private Object objA = 1000; + private Object objB = 2000; + + @Before + public void init() throws Exception + { + server = new Server(); + wac = new WebAppContext(); + wac.setServer(server); + injections = new InjectionCollection(); + wac.setAttribute(InjectionCollection.INJECTION_COLLECTION, injections); + InitialContext ic = new InitialContext(); + comp = (Context)ic.lookup("java:comp"); + env = comp.createSubcontext("env"); + } + + @After + public void destroy() throws Exception + { + comp.destroySubcontext("env"); + } + @Test public void testResourceAnnotations () throws Exception { - Server server = new Server(); - WebAppContext wac = new WebAppContext(); - wac.setServer(server); - InjectionCollection injections = new InjectionCollection(); - wac.setAttribute(InjectionCollection.INJECTION_COLLECTION, injections); - InitialContext ic = new InitialContext(); - Context comp = (Context)ic.lookup("java:comp"); - Context env = comp.createSubcontext("env"); - - org.eclipse.jetty.plus.jndi.EnvEntry resourceA = new org.eclipse.jetty.plus.jndi.EnvEntry(server, "resA", objA, false); - org.eclipse.jetty.plus.jndi.EnvEntry resourceB = new org.eclipse.jetty.plus.jndi.EnvEntry(server, "resB", objB, false); + new org.eclipse.jetty.plus.jndi.EnvEntry(server, "resA", objA, false); + new org.eclipse.jetty.plus.jndi.EnvEntry(server, "resB", objB, false); AnnotationIntrospector parser = new AnnotationIntrospector(); ResourceAnnotationHandler handler = new ResourceAnnotationHandler(wac); @@ -116,32 +128,21 @@ public class TestResourceAnnotations f = ResourceA.class.getDeclaredField("n"); f.setAccessible(true); assertEquals(objB, f.get(binst)); - - comp.destroySubcontext("env"); } - - + @Test public void testResourcesAnnotation () throws Exception { - Server server = new Server(); - WebAppContext wac = new WebAppContext(); - wac.setServer(server); - InjectionCollection injections = new InjectionCollection(); - wac.setAttribute(InjectionCollection.INJECTION_COLLECTION, injections); - InitialContext ic = new InitialContext(); - Context comp = (Context)ic.lookup("java:comp"); - Context env = comp.createSubcontext("env"); - org.eclipse.jetty.plus.jndi.EnvEntry resourceA = new org.eclipse.jetty.plus.jndi.EnvEntry(server, "resA", objA, false); - org.eclipse.jetty.plus.jndi.EnvEntry resourceB = new org.eclipse.jetty.plus.jndi.EnvEntry(server, "resB", objB, false); - + new org.eclipse.jetty.plus.jndi.EnvEntry(server, "resA", objA, false); + new org.eclipse.jetty.plus.jndi.EnvEntry(server, "resB", objB, false); + AnnotationIntrospector introspector = new AnnotationIntrospector(); ResourcesAnnotationHandler handler = new ResourcesAnnotationHandler(wac); introspector.registerHandler(handler); introspector.introspect(ResourceA.class); introspector.introspect(ResourceB.class); - + assertEquals(objA, env.lookup("peach")); assertEquals(objB, env.lookup("pear")); }