From d4061fcfeb5a959e67547404a1fdfa5a203680db Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Wed, 29 Nov 2017 18:56:54 +0100 Subject: [PATCH 1/6] Issue #1966 Case Sensitive method (PR #1967) Issue #1966 Case Sensitive method (PR #1967) * Modified the compliance violations to warn if case insensitivety is applied to a header * removed duplicated if * Fixed string comparison * Improved compliance messages and comments * updated expected violation messages * Require a header colon only when in 7230 compliance mode --- .../org/eclipse/jetty/http/HttpParser.java | 41 +++++++++++----- .../eclipse/jetty/http/HttpParserTest.java | 48 +++++++++++++++---- .../servlet/ComplianceViolations2616Test.java | 9 ++-- 3 files changed, 73 insertions(+), 25 deletions(-) diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java index d2db919ccbb..8dbc5024ea6 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java @@ -331,9 +331,10 @@ public class HttpParser } /* ------------------------------------------------------------------------------- */ - protected String legacyString(String orig, String cached) + protected String caseInsensitiveHeader(String orig, String normative) { - return (_compliance!=LEGACY || orig.equals(cached) || complianceViolation(RFC2616,"case sensitive"))?cached:orig; + return (_compliance!=LEGACY || orig.equals(normative) || complianceViolation(RFC2616,"https://tools.ietf.org/html/rfc2616#section-4.2 case sensitive header: "+orig)) + ?normative:orig; } /* ------------------------------------------------------------------------------- */ @@ -646,9 +647,27 @@ public class HttpParser { _length=_string.length(); _methodString=takeString(); + + // TODO #1966 This cache lookup is case insensitive when it should be case sensitive by RFC2616, RFC7230 HttpMethod method=HttpMethod.CACHE.get(_methodString); if (method!=null) - _methodString=legacyString(_methodString,method.asString()); + { + switch(_compliance) + { + case LEGACY: + // Legacy correctly allows case sensitive header; + break; + + case RFC2616: + case RFC7230: + if (!method.asString().equals(_methodString) && _complianceHandler!=null) + _complianceHandler.onComplianceViolation(_compliance,HttpCompliance.LEGACY, + "https://tools.ietf.org/html/rfc7230#section-3.1.1 case insensitive method "+_methodString); + // TODO Good to used cached version for faster equals checking, but breaks case sensitivity because cache is insensitive + _methodString = method.asString(); + break; + } + } setState(State.SPACE1); } else if (b < SPACE) @@ -749,7 +768,7 @@ public class HttpParser else if (b < HttpTokens.SPACE && b>=0) { // HTTP/0.9 - if (complianceViolation(RFC7230,"HTTP/0.9")) + if (complianceViolation(RFC7230,"https://tools.ietf.org/html/rfc7230#appendix-A.2 HTTP/0.9")) throw new BadMessageException("HTTP/0.9 not supported"); handle=_requestHandler.startRequest(_methodString,_uri.toString(), HttpVersion.HTTP_0_9); setState(State.END); @@ -816,7 +835,7 @@ public class HttpParser else { // HTTP/0.9 - if (complianceViolation(RFC7230,"HTTP/0.9")) + if (complianceViolation(RFC7230,"https://tools.ietf.org/html/rfc7230#appendix-A.2 HTTP/0.9")) throw new BadMessageException("HTTP/0.9 not supported"); handle=_requestHandler.startRequest(_methodString,_uri.toString(), HttpVersion.HTTP_0_9); @@ -934,7 +953,7 @@ public class HttpParser _host=true; if (!(_field instanceof HostPortHttpField) && _valueString!=null && !_valueString.isEmpty()) { - _field=new HostPortHttpField(_header,legacyString(_headerString,_header.asString()),_valueString); + _field=new HostPortHttpField(_header,caseInsensitiveHeader(_headerString,_header.asString()),_valueString); add_to_connection_trie=_fieldCache!=null; } break; @@ -963,7 +982,7 @@ public class HttpParser if (add_to_connection_trie && !_fieldCache.isFull() && _header!=null && _valueString!=null) { if (_field==null) - _field=new HttpField(_header,legacyString(_headerString,_header.asString()),_valueString); + _field=new HttpField(_header,caseInsensitiveHeader(_headerString,_header.asString()),_valueString); _fieldCache.put(_field); } } @@ -1031,7 +1050,7 @@ public class HttpParser case HttpTokens.SPACE: case HttpTokens.TAB: { - if (complianceViolation(RFC7230,"header folding")) + if (complianceViolation(RFC7230,"https://tools.ietf.org/html/rfc7230#section-3.2.4 folding")) throw new BadMessageException(HttpStatus.BAD_REQUEST_400,"Header Folding"); // header value without name - continuation? @@ -1154,13 +1173,13 @@ public class HttpParser { // Have to get the fields exactly from the buffer to match case String fn=field.getName(); - n=legacyString(BufferUtil.toString(buffer,buffer.position()-1,fn.length(),StandardCharsets.US_ASCII),fn); + n=caseInsensitiveHeader(BufferUtil.toString(buffer,buffer.position()-1,fn.length(),StandardCharsets.US_ASCII),fn); String fv=field.getValue(); if (fv==null) v=null; else { - v=legacyString(BufferUtil.toString(buffer,buffer.position()+fn.length()+1,fv.length(),StandardCharsets.ISO_8859_1),fv); + v=caseInsensitiveHeader(BufferUtil.toString(buffer,buffer.position()+fn.length()+1,fv.length(),StandardCharsets.ISO_8859_1),fv); field=new HttpField(field.getHeader(),n,v); } } @@ -1253,7 +1272,7 @@ public class HttpParser break; } - if (b==HttpTokens.LINE_FEED && !complianceViolation(RFC7230,"name only header")) + if (b==HttpTokens.LINE_FEED && !complianceViolation(RFC7230,"https://tools.ietf.org/html/rfc7230#section-3.2 No colon")) { if (_headerString==null) { diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java index b2d65e39d2a..a049c3cd91d 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java @@ -350,7 +350,7 @@ public class HttpParserTest } @Test - public void testNoColon2616() throws Exception + public void testNoColonLegacy() throws Exception { ByteBuffer buffer = BufferUtil.toBuffer( "GET / HTTP/1.0\r\n" + @@ -360,7 +360,7 @@ public class HttpParserTest "\r\n"); HttpParser.RequestHandler handler = new Handler(); - HttpParser parser = new HttpParser(handler,HttpCompliance.RFC2616); + HttpParser parser = new HttpParser(handler,HttpCompliance.LEGACY); parseAll(parser, buffer); Assert.assertTrue(_headerCompleted); @@ -375,7 +375,7 @@ public class HttpParserTest Assert.assertEquals("Other", _hdr[2]); Assert.assertEquals("value", _val[2]); Assert.assertEquals(2, _headers); - Assert.assertThat(_complianceViolation, Matchers.containsString("name only")); + Assert.assertThat(_complianceViolation, Matchers.containsString("No colon")); } @Test @@ -674,10 +674,42 @@ public class HttpParserTest } @Test - public void testCaseInsensitive() throws Exception + public void testCaseSensitiveMethod() throws Exception { ByteBuffer buffer = BufferUtil.toBuffer( - "get / http/1.0\r\n" + + "gEt / http/1.0\r\n" + + "Host: localhost\r\n" + + "Connection: close\r\n" + + "\r\n"); + HttpParser.RequestHandler handler = new Handler(); + HttpParser parser = new HttpParser(handler, -1, HttpCompliance.RFC7230); + parseAll(parser, buffer); + Assert.assertNull(_bad); + Assert.assertEquals("GET", _methodOrVersion); + Assert.assertThat(_complianceViolation, Matchers.containsString("case insensitive method gEt")); + } + + @Test + public void testCaseSensitiveMethodLegacy() throws Exception + { + ByteBuffer buffer = BufferUtil.toBuffer( + "gEt / http/1.0\r\n" + + "Host: localhost\r\n" + + "Connection: close\r\n" + + "\r\n"); + HttpParser.RequestHandler handler = new Handler(); + HttpParser parser = new HttpParser(handler, -1, HttpCompliance.LEGACY); + parseAll(parser, buffer); + Assert.assertNull(_bad); + Assert.assertEquals("gEt", _methodOrVersion); + Assert.assertNull(_complianceViolation); + } + + @Test + public void testCaseInsensitiveHeader() throws Exception + { + ByteBuffer buffer = BufferUtil.toBuffer( + "GET / http/1.0\r\n" + "HOST: localhost\r\n" + "cOnNeCtIoN: ClOsE\r\n" + "\r\n"); @@ -697,10 +729,10 @@ public class HttpParserTest } @Test - public void testCaseSensitiveLegacy() throws Exception + public void testCaseInSensitiveHeaderLegacy() throws Exception { ByteBuffer buffer = BufferUtil.toBuffer( - "gEt / http/1.0\r\n" + + "GET / http/1.0\r\n" + "HOST: localhost\r\n" + "cOnNeCtIoN: ClOsE\r\n" + "\r\n"); @@ -708,7 +740,7 @@ public class HttpParserTest HttpParser parser = new HttpParser(handler, -1, HttpCompliance.LEGACY); parseAll(parser, buffer); Assert.assertNull(_bad); - Assert.assertEquals("gEt", _methodOrVersion); + Assert.assertEquals("GET", _methodOrVersion); Assert.assertEquals("/", _uriOrStatus); Assert.assertEquals("HTTP/1.0", _versionOrReason); Assert.assertEquals("HOST", _hdr[0]); diff --git a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ComplianceViolations2616Test.java b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ComplianceViolations2616Test.java index 68e72bf3084..6b805040edb 100644 --- a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ComplianceViolations2616Test.java +++ b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ComplianceViolations2616Test.java @@ -147,8 +147,7 @@ public class ComplianceViolations2616Test String response = connector.getResponse(req1.toString()); assertThat("Response status", response, containsString("HTTP/1.1 200 OK")); - assertThat("Response headers", response, containsString("X-Http-Violation-0: RFC2616 Date: Thu, 30 Nov 2017 15:03:37 +0100 Subject: [PATCH 2/6] Issue #1986 ServletContextHandler.Context addListener() methods support session listeners (#2000) * Issue #1986 Support session listeners in ServletContextHandler.addEventListener method. Signed-off-by: Jan Bartel --- .../jetty/servlet/ServletContextHandler.java | 26 +++++++ .../servlet/ServletContextHandlerTest.java | 74 +++++++++++++++++++ .../eclipse/jetty/webapp/WebAppContext.java | 20 +---- .../jetty/webapp/WebAppContextTest.java | 41 ++++++++++ 4 files changed, 142 insertions(+), 19 deletions(-) diff --git a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletContextHandler.java b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletContextHandler.java index 558f1bd95fd..0b6ecc6a73c 100644 --- a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletContextHandler.java +++ b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletContextHandler.java @@ -46,6 +46,11 @@ import javax.servlet.SessionTrackingMode; import javax.servlet.descriptor.JspConfigDescriptor; import javax.servlet.descriptor.JspPropertyGroupDescriptor; import javax.servlet.descriptor.TaglibDescriptor; +import javax.servlet.http.HttpSessionActivationListener; +import javax.servlet.http.HttpSessionAttributeListener; +import javax.servlet.http.HttpSessionBindingListener; +import javax.servlet.http.HttpSessionIdListener; +import javax.servlet.http.HttpSessionListener; import org.eclipse.jetty.security.ConstraintAware; import org.eclipse.jetty.security.ConstraintMapping; @@ -174,6 +179,27 @@ public class ServletContextHandler extends ContextHandler setErrorHandler(errorHandler); } + /* ------------------------------------------------------------ */ + /** Add EventListener + * Adds an EventListener to the list. @see org.eclipse.jetty.server.handler.ContextHandler#addEventListener(). + * Also adds any listeners that are session related to the SessionHandler. + * @param listener the listener to add + */ + @Override + public void addEventListener(EventListener listener) + { + super.addEventListener(listener); + if ((listener instanceof HttpSessionActivationListener) + || (listener instanceof HttpSessionAttributeListener) + || (listener instanceof HttpSessionBindingListener) + || (listener instanceof HttpSessionListener) + || (listener instanceof HttpSessionIdListener)) + { + if (_sessionHandler!=null) + _sessionHandler.addEventListener(listener); + } + } + @Override public void setHandler(Handler handler) { diff --git a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ServletContextHandlerTest.java b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ServletContextHandlerTest.java index 1cf59587989..fcd82976ea7 100644 --- a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ServletContextHandlerTest.java +++ b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ServletContextHandlerTest.java @@ -22,6 +22,7 @@ import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.nullValue; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; @@ -38,6 +39,10 @@ import javax.servlet.ServletException; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import javax.servlet.http.HttpSessionAttributeListener; +import javax.servlet.http.HttpSessionBindingEvent; +import javax.servlet.http.HttpSessionEvent; +import javax.servlet.http.HttpSessionListener; import org.eclipse.jetty.security.ConstraintSecurityHandler; import org.eclipse.jetty.security.SecurityHandler; @@ -68,6 +73,57 @@ public class ServletContextHandlerTest private static final AtomicInteger __testServlets = new AtomicInteger(); + public static class MySessionHandler extends SessionHandler + { + public void checkSessionListeners (int size) + { + assertNotNull(_sessionListeners); + assertEquals(size, _sessionListeners.size()); + } + + public void checkSessionAttributeListeners(int size) + { + assertNotNull(_sessionAttributeListeners); + assertEquals(size, _sessionAttributeListeners.size()); + } + + public void checkSessionIdListeners(int size) + { + assertNotNull(_sessionIdListeners); + assertEquals(size, _sessionIdListeners.size()); + } + } + + public static class MyTestSessionListener implements HttpSessionAttributeListener, HttpSessionListener + { + + @Override + public void sessionCreated(HttpSessionEvent se) + { + } + + @Override + public void sessionDestroyed(HttpSessionEvent se) + { + } + + @Override + public void attributeAdded(HttpSessionBindingEvent event) + { + } + + @Override + public void attributeRemoved(HttpSessionBindingEvent event) + { + } + + @Override + public void attributeReplaced(HttpSessionBindingEvent event) + { + } + + } + @Before public void createServer() { @@ -84,6 +140,24 @@ public class ServletContextHandlerTest _server.stop(); _server.join(); } + + @Test + public void testAddSessionListener() throws Exception + { + ContextHandlerCollection contexts = new ContextHandlerCollection(); + _server.setHandler(contexts); + + ServletContextHandler root = new ServletContextHandler(contexts,"/",ServletContextHandler.SESSIONS); + + MySessionHandler sessions = new MySessionHandler(); + root.setSessionHandler(sessions); + assertNotNull(sessions); + + root.addEventListener(new MyTestSessionListener()); + sessions.checkSessionAttributeListeners(1); + sessions.checkSessionIdListeners(0); + sessions.checkSessionListeners(1); + } @Test public void testFindContainer() throws Exception diff --git a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppContext.java b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppContext.java index c26faa5bd47..39302b92610 100644 --- a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppContext.java +++ b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppContext.java @@ -1191,25 +1191,7 @@ public class WebAppContext extends ServletContextHandler implements WebAppClassL super.setEventListeners(eventListeners); } - /* ------------------------------------------------------------ */ - /** Add EventListener - * Convenience method that calls {@link #setEventListeners(EventListener[])} - * @param listener the listener to add - */ - @Override - public void addEventListener(EventListener listener) - { - super.addEventListener(listener); - if ((listener instanceof HttpSessionActivationListener) - || (listener instanceof HttpSessionAttributeListener) - || (listener instanceof HttpSessionBindingListener) - || (listener instanceof HttpSessionListener) - || (listener instanceof HttpSessionIdListener)) - { - if (_sessionHandler!=null) - _sessionHandler.addEventListener(listener); - } - } + @Override public void removeEventListener(EventListener listener) diff --git a/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/WebAppContextTest.java b/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/WebAppContextTest.java index acfa5878e7d..ca9319daf80 100644 --- a/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/WebAppContextTest.java +++ b/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/WebAppContextTest.java @@ -28,6 +28,7 @@ import java.io.IOException; import java.nio.file.Path; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.List; import java.util.stream.Collectors; @@ -38,6 +39,8 @@ import javax.servlet.ServletContextListener; import javax.servlet.ServletException; import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; +import javax.servlet.http.HttpSessionEvent; +import javax.servlet.http.HttpSessionListener; import org.eclipse.jetty.server.LocalConnector; import org.eclipse.jetty.server.Server; @@ -57,6 +60,44 @@ import org.junit.Test; public class WebAppContextTest { + public class MySessionListener implements HttpSessionListener + { + + @Override + public void sessionCreated(HttpSessionEvent se) + { + // TODO Auto-generated method stub + + } + + @Override + public void sessionDestroyed(HttpSessionEvent se) + { + // TODO Auto-generated method stub + + } + + } + + @Test + public void testSessionListeners () + throws Exception + { + Server server = new Server(); + + WebAppContext wac = new WebAppContext(); + + wac.setServer(server); + server.setHandler(wac); + wac.addEventListener(new MySessionListener()); + + Collection listeners = wac.getSessionHandler().getBeans(org.eclipse.jetty.webapp.WebAppContextTest.MySessionListener.class); + assertNotNull(listeners); + assertEquals(1, listeners.size()); + } + + + @Test public void testConfigurationClassesFromDefault () { From 2067eac701799d4f545129f8b96350df030049c2 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Fri, 1 Dec 2017 18:34:49 +0100 Subject: [PATCH 3/6] Issue #2003 - Do not submit blocking tasks as managed selector actions. CreateEndPoint and DestroyEndPoint are now submitted directly to the Executor, rather than being submitted as selector actions. Signed-off-by: Simone Bordet --- .../org/eclipse/jetty/io/ManagedSelector.java | 33 ++++++++++++------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/ManagedSelector.java b/jetty-io/src/main/java/org/eclipse/jetty/io/ManagedSelector.java index ad08872056e..7732d38bd4a 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/ManagedSelector.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/ManagedSelector.java @@ -36,6 +36,7 @@ import java.util.Queue; import java.util.Set; import java.util.concurrent.CountDownLatch; import java.util.concurrent.Executor; +import java.util.concurrent.RejectedExecutionException; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; @@ -148,7 +149,20 @@ public class ManagedSelector extends ContainerLifeCycle implements Dumpable selector.wakeup(); } - private Runnable processConnect(SelectionKey key, final Connect connect) + private void execute(Runnable task) + { + try + { + _selectorManager.execute(task); + } + catch (RejectedExecutionException x) + { + if (task instanceof Closeable) + closeNoExceptions((Closeable)task); + } + } + + private void processConnect(SelectionKey key, final Connect connect) { SelectableChannel channel = key.channel(); try @@ -162,7 +176,7 @@ public class ManagedSelector extends ContainerLifeCycle implements Dumpable if (connect.timeout.cancel()) { key.interestOps(0); - return new CreateEndPoint(channel, key) + execute(new CreateEndPoint(channel, key) { @Override protected void failed(Throwable failure) @@ -170,7 +184,7 @@ public class ManagedSelector extends ContainerLifeCycle implements Dumpable super.failed(failure); connect.failed(failure); } - }; + }); } else { @@ -185,7 +199,6 @@ public class ManagedSelector extends ContainerLifeCycle implements Dumpable catch (Throwable x) { connect.failed(x); - return null; } } @@ -217,7 +230,7 @@ public class ManagedSelector extends ContainerLifeCycle implements Dumpable public void destroyEndPoint(final EndPoint endPoint) { - submit(new DestroyEndPoint(endPoint)); + execute(new DestroyEndPoint(endPoint)); } private int getActionSize() @@ -424,9 +437,7 @@ public class ManagedSelector extends ContainerLifeCycle implements Dumpable } else if (key.isConnectable()) { - Runnable task = processConnect(key, (Connect)attachment); - if (task != null) - return task; + processConnect(key, (Connect)attachment); } else { @@ -621,7 +632,7 @@ public class ManagedSelector extends ContainerLifeCycle implements Dumpable try { final SelectionKey key = channel.register(_selector, 0, attachment); - submit(new CreateEndPoint(channel, key)); + execute(new CreateEndPoint(channel, key)); } catch (Throwable x) { @@ -631,7 +642,7 @@ public class ManagedSelector extends ContainerLifeCycle implements Dumpable } } - private class CreateEndPoint implements Runnable, Invocable, Closeable + private class CreateEndPoint implements Runnable, Closeable { private final SelectableChannel channel; private final SelectionKey key; @@ -823,7 +834,7 @@ public class ManagedSelector extends ContainerLifeCycle implements Dumpable } } - private class DestroyEndPoint implements Runnable, Invocable, Closeable + private class DestroyEndPoint implements Runnable, Closeable { private final EndPoint endPoint; From 15c0f7959349bec8f0f2d74698ffc97b38d46bb0 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Sat, 2 Dec 2017 10:00:39 +0100 Subject: [PATCH 4/6] Issue #1614 made authentication extensible in request log (#2004) Issue #1614 made authentication extensible in request log (#2004) Signed-off-by: Greg Wilkins --- .../jetty/server/AbstractNCSARequestLog.java | 22 +++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/AbstractNCSARequestLog.java b/jetty-server/src/main/java/org/eclipse/jetty/server/AbstractNCSARequestLog.java index 1e451accd79..359510704a5 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/AbstractNCSARequestLog.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/AbstractNCSARequestLog.java @@ -127,8 +127,9 @@ public abstract class AbstractNCSARequestLog extends AbstractLifeCycle implement buf.append(addr); buf.append(" - "); - Authentication authentication = request.getAuthentication(); - append(buf,(authentication instanceof Authentication.User)?((Authentication.User)authentication).getUserIdentity().getUserPrincipal().getName():null); + + String auth = getAuthentication(request); + append(buf,auth==null?"-":auth); buf.append(" ["); if (_logDateCache != null) @@ -220,6 +221,23 @@ public abstract class AbstractNCSARequestLog extends AbstractLifeCycle implement LOG.warn(e); } } + + /** + * Extract the user authentication + * @param request The request to extract from + * @return The string to log for authenticated user. + */ + protected String getAuthentication(Request request) + { + Authentication authentication = request.getAuthentication(); + + if (authentication instanceof Authentication.User) + return ((Authentication.User)authentication).getUserIdentity().getUserPrincipal().getName(); + + // TODO extract the user name if it is Authentication.Deferred and return as '?username' + + return null; + } /** * Writes extended request and response information to the output stream. From 5ce07dddd645771885ab1f262e97f363cfb91577 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Sat, 2 Dec 2017 12:14:43 +0100 Subject: [PATCH 5/6] #2006 Use a NOOP to allow unhandle to cope with stolen read (#2009) #2006 Use a NOOP to allow unhandle to cope with stolen read Signed-off-by: Greg Wilkins --- .../org/eclipse/jetty/server/HttpChannel.java | 5 + .../jetty/server/HttpChannelState.java | 9 +- .../jetty/servlet/AsyncServletIOTest.java | 190 +++++++++++++++++- 3 files changed, 199 insertions(+), 5 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java index add0b3301bc..05d47a91ace 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java @@ -318,7 +318,12 @@ public class HttpChannel implements Runnable, HttpOutput.Interceptor { case TERMINATED: case WAIT: + // break loop without calling unhandle break loop; + + case NOOP: + // do nothing other than call unhandle + break; case DISPATCH: { diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java index c5e43b59516..3abddd890d0 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java @@ -73,6 +73,7 @@ public class HttpChannelState */ public enum Action { + NOOP, // No action DISPATCH, // handle a normal request dispatch ASYNC_DISPATCH, // handle an async request dispatch ERROR_DISPATCH, // handle a normal error @@ -243,6 +244,8 @@ public class HttpChannelState case IDLE: case REGISTERED: break; + default: + throw new IllegalStateException(getStatusStringLocked()); } if (_asyncWritePossible) @@ -269,14 +272,13 @@ public class HttpChannelState case STARTED: case EXPIRING: case ERRORING: - return Action.WAIT; + _state=State.ASYNC_WAIT; + return Action.NOOP; case NOT_ASYNC: - break; default: throw new IllegalStateException(getStatusStringLocked()); } - return Action.WAIT; case ASYNC_ERROR: return Action.ASYNC_ERROR; @@ -408,6 +410,7 @@ public class HttpChannelState case DISPATCHED: case ASYNC_IO: case ASYNC_ERROR: + case ASYNC_WAIT: break; default: diff --git a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/AsyncServletIOTest.java b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/AsyncServletIOTest.java index 55d9a47767d..690c3ebb95b 100644 --- a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/AsyncServletIOTest.java +++ b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/AsyncServletIOTest.java @@ -22,7 +22,6 @@ import static java.nio.charset.StandardCharsets.ISO_8859_1; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.greaterThanOrEqualTo; -import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.startsWith; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThat; @@ -41,6 +40,9 @@ import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Function; +import java.util.function.UnaryOperator; import javax.servlet.AsyncContext; import javax.servlet.AsyncEvent; @@ -63,6 +65,7 @@ import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.ServerConnector; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; +import org.eclipse.jetty.util.thread.QueuedThreadPool; import org.hamcrest.Matchers; import org.junit.After; import org.junit.Assert; @@ -77,14 +80,19 @@ public class AsyncServletIOTest protected AsyncIOServlet2 _servlet2=new AsyncIOServlet2(); protected AsyncIOServlet3 _servlet3=new AsyncIOServlet3(); protected AsyncIOServlet4 _servlet4=new AsyncIOServlet4(); + protected StolenAsyncReadServlet _servletStolenAsyncRead=new StolenAsyncReadServlet(); protected int _port; - protected Server _server = new Server(); + protected WrappingQTP _wQTP; + protected Server _server; protected ServletHandler _servletHandler; protected ServerConnector _connector; @Before public void setUp() throws Exception { + _wQTP = new WrappingQTP(); + _server = new Server(_wQTP); + HttpConfiguration http_config = new HttpConfiguration(); http_config.setOutputBufferSize(4096); _connector = new ServerConnector(_server,new HttpConnectionFactory(http_config)); @@ -113,6 +121,10 @@ public class AsyncServletIOTest holder4.setAsyncSupported(true); _servletHandler.addServletWithMapping(holder4,"/path4/*"); + ServletHolder holder5=new ServletHolder(_servletStolenAsyncRead); + holder5.setAsyncSupported(true); + _servletHandler.addServletWithMapping(holder5,"/stolen/*"); + _server.start(); _port=_connector.getLocalPort(); @@ -787,5 +799,179 @@ public class AsyncServletIOTest } } + + @Test + public void testStolenAsyncRead() throws Exception + { + StringBuilder request = new StringBuilder(512); + request.append("POST /ctx/stolen/info HTTP/1.1\r\n") + .append("Host: localhost\r\n") + .append("Content-Type: text/plain\r\n") + .append("Content-Length: 2\r\n") + .append("\r\n") + .append("1"); + int port=_port; + try (Socket socket = new Socket("localhost",port)) + { + socket.setSoTimeout(10000); + OutputStream out = socket.getOutputStream(); + out.write(request.toString().getBytes(ISO_8859_1)); + out.flush(); + + // wait until server is ready + _servletStolenAsyncRead.ready.await(); + final CountDownLatch wait = new CountDownLatch(1); + + // Stop any dispatches until we want them + UnaryOperator old = _wQTP.wrapper.getAndSet(r-> + ()-> + { + try + { + wait.await(); + r.run(); + } + catch (InterruptedException e) + { + e.printStackTrace(); + } + } + ); + + // We are an unrelated thread, let's mess with the input stream + ServletInputStream sin = _servletStolenAsyncRead.listener.in; + sin.setReadListener(_servletStolenAsyncRead.listener); + // thread should be dispatched to handle, but held by our wQTP wait. + + // Let's steal our read + Assert.assertTrue(sin.isReady()); + Assert.assertThat(sin.read(),Matchers.is((int)'1')); + Assert.assertFalse(sin.isReady()); + + // let the ODA call go + _wQTP.wrapper.set(old); + wait.countDown(); + + // ODA should not be called + Assert.assertFalse(_servletStolenAsyncRead.oda.await(500,TimeUnit.MILLISECONDS)); + + // Send some more data + out.write((int)'2'); + out.flush(); + + // ODA should now be called!! + Assert.assertTrue(_servletStolenAsyncRead.oda.await(500,TimeUnit.MILLISECONDS)); + + // We can not read some more + Assert.assertTrue(sin.isReady()); + Assert.assertThat(sin.read(),Matchers.is((int)'2')); + + // read EOF + Assert.assertTrue(sin.isReady()); + Assert.assertThat(sin.read(),Matchers.is(-1)); + + BufferedReader in = new BufferedReader(new InputStreamReader(socket.getInputStream())); + + // response line + String line = in.readLine(); + LOG.debug("response-line: "+line); + Assert.assertThat(line,startsWith("HTTP/1.1 200 OK")); + + // Skip headers + while (line!=null) + { + line = in.readLine(); + LOG.debug("header-line: "+line); + if (line.length()==0) + break; + } + } + + assertTrue(_servletStolenAsyncRead.completed.await(5, TimeUnit.SECONDS)); + } + + @SuppressWarnings("serial") + public class StolenAsyncReadServlet extends HttpServlet + { + public CountDownLatch ready = new CountDownLatch(1); + public CountDownLatch oda = new CountDownLatch(1); + public CountDownLatch completed = new CountDownLatch(1); + public volatile StealingListener listener; + + @Override + public void doPost(final HttpServletRequest request, final HttpServletResponse response) throws IOException + { + listener = new StealingListener(request); + ready.countDown(); + } + + public class StealingListener implements ReadListener, AsyncListener + { + final HttpServletRequest request; + final ServletInputStream in; + final AsyncContext asyncContext; + + StealingListener(HttpServletRequest request) throws IOException + { + asyncContext = request.startAsync(); + asyncContext.setTimeout(10000L); + asyncContext.addListener(this); + this.request=request; + in = request.getInputStream(); + } + + @Override + public void onDataAvailable() + { + oda.countDown(); + } + + @Override + public void onAllDataRead() throws IOException + { + asyncContext.complete(); + } + + @Override + public void onError(final Throwable t) + { + t.printStackTrace(); + asyncContext.complete(); + } + + @Override + public void onComplete(final AsyncEvent event) + { + completed.countDown(); + } + + @Override + public void onTimeout(final AsyncEvent event) + { + asyncContext.complete(); + } + + @Override + public void onError(final AsyncEvent event) + { + asyncContext.complete(); + } + + @Override + public void onStartAsync(AsyncEvent event) + { + } + } + } + private class WrappingQTP extends QueuedThreadPool + { + AtomicReference> wrapper = new AtomicReference<>(UnaryOperator.identity()); + + @Override + public void execute(Runnable job) + { + super.execute(wrapper.get().apply(job)); + } + } } From e82649bf9dac0e1b927cf4bb6bca11d22efc9b6e Mon Sep 17 00:00:00 2001 From: WalkerWatch Date: Wed, 6 Dec 2017 08:56:07 -0500 Subject: [PATCH 6/6] Issue #1993 - Updated maven doc. Resolves #1993. --- .../development/maven/jetty-maven-plugin.adoc | 24 ++++++++++++------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/jetty-documentation/src/main/asciidoc/development/maven/jetty-maven-plugin.adoc b/jetty-documentation/src/main/asciidoc/development/maven/jetty-maven-plugin.adoc index 2a22123ae59..1a8d6a673ae 100644 --- a/jetty-documentation/src/main/asciidoc/development/maven/jetty-maven-plugin.adoc +++ b/jetty-documentation/src/main/asciidoc/development/maven/jetty-maven-plugin.adoc @@ -116,7 +116,7 @@ soLinger;; The socket linger time. + You could instead configure the connectors in a standard link:#jetty-xml-config[jetty xml config file] and put its location into the `jettyXml` parameter. -Note that since jetty-9.0 it is no longer possible to configure a link:#maven-config-https[https connector] directly in the pom.xml: you need to link:#maven-config-https[use jetty xml config files to do it]. +Note that since Jetty 9.0 it is no longer possible to configure a link:#maven-config-https[https connector] directly in the pom.xml: you need to link:#maven-config-https[use jetty xml config files to do it]. jettyXml:: Optional. A comma separated list of locations of Jetty xml files to apply in addition to any plugin configuration parameters. @@ -356,7 +356,7 @@ Any changes you make are immediately reflected in the running instance of Jetty, ____ [NOTE] -As of jetty-9.4.7, when using jetty:run in a multi-module build, it is no longer necessary to build each of the modules that form dependencies of the webapp first. +As of Jetty 9.4.7, when using jetty:run in a multi-module build, it is no longer necessary to build each of the modules that form dependencies of the webapp first. Thus, if your webapp depends on other modules in your project and they are present in the reactor at the same time, jetty will use their compiled classes rather than their jar files from your local maven repository. ____ @@ -619,7 +619,7 @@ This is maximum number of times the parent process checks to see if the child pr Only relevant if `waitForChild` is `false`. maxChildCheckInterval:: Default is `100`. -This is the time in milliseconds between checks to see if the child process has started. +This is the time in milliseconds between checks to see if the child process has started. Only relevant if `waitForChild` is `false`. forkWebXml:: Default is `target/fork-web.xml`. @@ -674,18 +674,18 @@ ____ [[jetty-run-distro-goal]] ==== jetty:run-distro -Introduced in jetty-9.4.8, this goal allows you to execute your unassembled webapp in a local distribution of jetty. +Introduced in Jetty 9.4.8, this goal allows you to execute your unassembled webapp in a local distribution of Jetty. This can be useful if your webapp requires a highly customized environment in which to run. -If your webapp is designed to run in the jetty distribution in production, then this goal is the closest approximation to that environment. +If your webapp is designed to run in the jetty distribution in production, then this goal is the closest approximation to that environment. -Similar to the jetty:run-forked goal, this goal will fork a child process in which to execute your webapp in the distro. +Similar to the `jetty:run-forked` goal, this goal will fork a child process in which to execute your webapp in the distro. ===== Configuration The configuration parameters are mostly the same as the `jetty:run` goal (although see below for some exceptions), with the addition of: jettyBase:: -Optional. +Optional. The location of an existing jetty base directory to use to deploy the webapp. The existing base will be copied to the `target/` directory before the webapp is deployed. If there is no existing jetty base, a fresh one will be made in `target/jetty-base`. @@ -718,7 +718,7 @@ Only applicable if `waitForChild` is `false`. ____ [NOTE] -Use the `modules` parameter to configure the jetty distribution appropriately rather than using jetty artifacts as `plugin dependencies`. +Use the `modules` parameter to configure the Jetty distribution appropriately rather than using jetty artifacts as `plugin dependencies`. ____ The following `jetty:run` parameters are *NOT* applicable to this goal: @@ -771,6 +771,7 @@ Here's an example of using the configuration parameters: apache-jsp apache-jstl + jmx jetty.server.dumpAfterStart=true @@ -787,6 +788,13 @@ Here's an example of using the configuration parameters: ---- +____ +[NOTE] +When defining modules for this goal, use the standard link:#startup-modules[Jetty module] names and not the name of the related Jetty sub-project. +For example, in the configuration above support for JMX is configured by adding the `jmx` Jetty module and not the `jetty-jmx` sub-project. +____ + + To deploy your unassembled web app to jetty running as a local distribution: [source, screen, subs="{sub-order}"]