From 3c671aa8b6b06e724889da4fb52741ef865a0eee Mon Sep 17 00:00:00 2001
From: Greg Wilkins <gregw@webtide.com>
Date: Tue, 23 Feb 2016 17:33:53 +0100
Subject: [PATCH 1/5] Issue #346 HttpParser RFC2616 Compliance mode

Reimplmented HTTP/0.9 support in RFC2616 compliance mode
---
 .../jetty/client/ssl/SslBytesServerTest.java  |  3 +-
 .../eclipse/jetty/http/HttpCompliance.java    | 31 ++++++++++
 .../org/eclipse/jetty/http/HttpGenerator.java |  7 ++-
 .../org/eclipse/jetty/http/HttpParser.java    | 59 ++++++++++++-------
 .../jetty/http/HttpGeneratorServerTest.java   | 35 +++++++++++
 .../eclipse/jetty/http/HttpParserTest.java    | 44 +++++++++++---
 .../src/main/config/etc/jetty-http.xml        |  1 +
 .../src/main/config/etc/jetty-https.xml       |  1 +
 jetty-server/src/main/config/modules/http.mod |  3 +
 .../jetty/server/HttpChannelOverHttp.java     |  7 ++-
 .../eclipse/jetty/server/HttpConnection.java  |  5 +-
 .../jetty/server/HttpConnectionFactory.java   | 18 ++++--
 .../jetty/server/ExtendedServerTest.java      |  4 +-
 .../jetty/server/HttpConnectionTest.java      | 13 ++++
 14 files changed, 191 insertions(+), 40 deletions(-)
 create mode 100644 jetty-http/src/main/java/org/eclipse/jetty/http/HttpCompliance.java

diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/ssl/SslBytesServerTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/ssl/SslBytesServerTest.java
index 70bef72db40..0aedb4b0f44 100644
--- a/jetty-client/src/test/java/org/eclipse/jetty/client/ssl/SslBytesServerTest.java
+++ b/jetty-client/src/test/java/org/eclipse/jetty/client/ssl/SslBytesServerTest.java
@@ -53,6 +53,7 @@ import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
 import org.eclipse.jetty.client.ssl.SslBytesTest.TLSRecord.Type;
+import org.eclipse.jetty.http.HttpCompliance;
 import org.eclipse.jetty.http.HttpParser;
 import org.eclipse.jetty.io.Connection;
 import org.eclipse.jetty.io.EndPoint;
@@ -114,7 +115,7 @@ public class SslBytesServerTest extends SslBytesTest
                 return configure(new HttpConnection(getHttpConfiguration(), connector, endPoint,getHttpCompliance())
                 {
                     @Override
-                    protected HttpParser newHttpParser(HttpParser.Compliance compliance)
+                    protected HttpParser newHttpParser(HttpCompliance compliance)
                     {
                         return new HttpParser(newRequestHandler(), getHttpConfiguration().getRequestHeaderSize(),compliance)
                         {
diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpCompliance.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpCompliance.java
new file mode 100644
index 00000000000..dc2a1e307ee
--- /dev/null
+++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpCompliance.java
@@ -0,0 +1,31 @@
+//
+//  ========================================================================
+//  Copyright (c) 1995-2016 Mort Bay Consulting Pty. Ltd.
+//  ------------------------------------------------------------------------
+//  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
+//      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.
+//  ========================================================================
+//
+
+package org.eclipse.jetty.http;
+
+
+/**
+ * HTTP compliance modes:
+ * <dl>
+ * <dt>RFC7230</dt><dd>(default) Compliance with RFC7230</dd>
+ * <dt>RFC2616</dt><dd>Wrapped/Continued headers and HTTP/0.9 supported</dd>
+ * <dt>LEGACY</dt><dd>(aka STRICT) Adherence to Servlet Specification requirement for 
+ * exact case of header names, bypassing the header caches, which are case insensitive, 
+ * otherwise equivalent to RFC2616</dd>
+ */
+public enum HttpCompliance { LEGACY, RFC2616, RFC7230 }
\ No newline at end of file
diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpGenerator.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpGenerator.java
index 642d35ee00e..c21da96778b 100644
--- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpGenerator.java
+++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpGenerator.java
@@ -359,7 +359,12 @@ public class HttpGenerator
                         break;
                         
                     default:
-                        throw new IllegalArgumentException(version+" not supported");
+                        _persistent = false;
+                        _endOfContent=EndOfContent.EOF_CONTENT;
+                        if (BufferUtil.hasContent(content))
+                            _contentPrepared+=content.remaining();
+                        _state = last?State.COMPLETING:State.COMMITTED;
+                        return Result.FLUSH;
                 }
                 
                 // Do we need a response header
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 3b963adb4dd..12244277c87 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
@@ -77,8 +77,8 @@ import static org.eclipse.jetty.http.HttpTokens.TAB;
  * The parser can work in varying compliance modes:
  * <dl>
  * <dt>RFC7230</dt><dd>(default) Compliance with RFC7230</dd>
- * <dt>RFC2616</dt><dd>Wrapped headers supported</dd>
- * <dt>STRICT</dt><dd>(misnomer) Adherence to Servlet Specification requirement for 
+ * <dt>RFC2616</dt><dd>Wrapped headers and HTTP/0.9 supported</dd>
+ * <dt>LEGACY</dt><dd>(aka STRICT) Adherence to Servlet Specification requirement for 
  * exact case of header names, bypassing the header caches, which are case insensitive, 
  * otherwise equivalent to RFC2616</dd>
  * </p>
@@ -106,9 +106,6 @@ public class HttpParser
      */
     public final static Trie<HttpField> CACHE = new ArrayTrie<>(2048);
 
-    // Compliance 
-    public enum Compliance { STRICT, RFC2616, RFC7230 };
-    
     // States
     public enum State
     {
@@ -146,7 +143,7 @@ public class HttpParser
     private final RequestHandler _requestHandler;
     private final ResponseHandler _responseHandler;
     private final int _maxHeaderBytes;
-    private final Compliance _compliance;
+    private final HttpCompliance _compliance;
     private HttpField _field;
     private HttpHeader _header;
     private String _headerString;
@@ -226,10 +223,10 @@ public class HttpParser
         CACHE.put(new HttpField(HttpHeader.COOKIE,(String)null));
     }
     
-    private static Compliance compliance()
+    private static HttpCompliance compliance()
     {
         Boolean strict = Boolean.getBoolean(__STRICT);        
-        return strict?Compliance.STRICT:Compliance.RFC7230;
+        return strict?HttpCompliance.LEGACY:HttpCompliance.RFC7230;
     }
 
     /* ------------------------------------------------------------------------------- */
@@ -260,18 +257,24 @@ public class HttpParser
     @Deprecated
     public HttpParser(RequestHandler handler,int maxHeaderBytes,boolean strict)
     {
-        this(handler,maxHeaderBytes,strict?Compliance.STRICT:compliance());
+        this(handler,maxHeaderBytes,strict?HttpCompliance.LEGACY:compliance());
     }
     
     /* ------------------------------------------------------------------------------- */
     @Deprecated
     public HttpParser(ResponseHandler handler,int maxHeaderBytes,boolean strict)
     {
-        this(handler,maxHeaderBytes,strict?Compliance.STRICT:compliance());
+        this(handler,maxHeaderBytes,strict?HttpCompliance.LEGACY:compliance());
     }
-        
+
     /* ------------------------------------------------------------------------------- */
-    public HttpParser(RequestHandler handler,int maxHeaderBytes,Compliance compliance)
+    public HttpParser(RequestHandler handler,HttpCompliance compliance)
+    {
+        this(handler,-1,compliance);
+    }
+    
+    /* ------------------------------------------------------------------------------- */
+    public HttpParser(RequestHandler handler,int maxHeaderBytes,HttpCompliance compliance)
     {
         _handler=handler;
         _requestHandler=handler;
@@ -281,7 +284,7 @@ public class HttpParser
     }
 
     /* ------------------------------------------------------------------------------- */
-    public HttpParser(ResponseHandler handler,int maxHeaderBytes,Compliance compliance)
+    public HttpParser(ResponseHandler handler,int maxHeaderBytes,HttpCompliance compliance)
     {
         _handler=handler;
         _requestHandler=null;
@@ -583,7 +586,7 @@ public class HttpParser
                         _length=_string.length();
                         _methodString=takeString();
                         HttpMethod method=HttpMethod.CACHE.get(_methodString);
-                        if (method!=null && _compliance!=Compliance.STRICT)
+                        if (method!=null && _compliance!=HttpCompliance.LEGACY)
                             _methodString=method.asString();
                         setState(State.SPACE1);
                     }
@@ -685,7 +688,15 @@ public class HttpParser
                     else if (ch < HttpTokens.SPACE && ch>=0)
                     {
                         // HTTP/0.9
-                        throw new BadMessageException("HTTP/0.9 not supported");
+                        if (_compliance.ordinal()>=HttpCompliance.RFC7230.ordinal())
+                            throw new BadMessageException("HTTP/0.9 not supported");
+
+                        handle=_requestHandler.startRequest(_methodString,_uri.toString(), HttpVersion.HTTP_0_9);
+                        setState(State.END);
+                        BufferUtil.clear(buffer);
+                        handle=_handler.headerComplete()||handle;
+                        handle=_handler.messageComplete()||handle;
+                        return handle;
                     }
                     else
                     {
@@ -747,7 +758,15 @@ public class HttpParser
                         else
                         {
                             // HTTP/0.9
-                            throw new BadMessageException("HTTP/0.9 not supported");
+                            if (_compliance.ordinal()>=HttpCompliance.RFC7230.ordinal())
+                                throw new BadMessageException("HTTP/0.9 not supported");
+
+                            handle=_requestHandler.startRequest(_methodString,_uri.toString(), HttpVersion.HTTP_0_9);
+                            setState(State.END);
+                            BufferUtil.clear(buffer);
+                            handle=_handler.headerComplete()||handle;
+                            handle=_handler.messageComplete()||handle;
+                            return handle;
                         }
                     }
                     else if (ch<0)
@@ -859,7 +878,7 @@ public class HttpParser
                         _host=true;
                         if (!(_field instanceof HostPortHttpField))
                         {
-                            _field=new HostPortHttpField(_header,_compliance==Compliance.STRICT?_headerString:_header.asString(),_valueString);
+                            _field=new HostPortHttpField(_header,_compliance==HttpCompliance.LEGACY?_headerString:_header.asString(),_valueString);
                             add_to_connection_trie=_connectionFields!=null;
                         }
                       break;
@@ -888,7 +907,7 @@ public class HttpParser
                 if (add_to_connection_trie && !_connectionFields.isFull() && _header!=null && _valueString!=null)
                 {
                     if (_field==null)
-                        _field=new HttpField(_header,_compliance==Compliance.STRICT?_headerString:_header.asString(),_valueString);
+                        _field=new HttpField(_header,_compliance==HttpCompliance.LEGACY?_headerString:_header.asString(),_valueString);
                     _connectionFields.put(_field);
                 }
             }
@@ -933,7 +952,7 @@ public class HttpParser
                         case HttpTokens.SPACE:
                         case HttpTokens.TAB:
                         {
-                            if (_compliance.ordinal()>=Compliance.RFC7230.ordinal())
+                            if (_compliance.ordinal()>=HttpCompliance.RFC7230.ordinal())
                                 throw new BadMessageException(HttpStatus.BAD_REQUEST_400,"Bad Continuation");
 
                             // header value without name - continuation?
@@ -1035,7 +1054,7 @@ public class HttpParser
                                     final String n;
                                     final String v;
 
-                                    if (_compliance==Compliance.STRICT)
+                                    if (_compliance==HttpCompliance.LEGACY)
                                     {
                                         // Have to get the fields exactly from the buffer to match case
                                         String fn=field.getName();
diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpGeneratorServerTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpGeneratorServerTest.java
index d965279603a..34b485fcf91 100644
--- a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpGeneratorServerTest.java
+++ b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpGeneratorServerTest.java
@@ -32,6 +32,41 @@ import org.junit.Test;
 
 public class HttpGeneratorServerTest
 { 
+    @Test
+    public void test_0_9() throws Exception
+    {
+        ByteBuffer header = BufferUtil.allocate(8096);
+        ByteBuffer content = BufferUtil.toBuffer("0123456789");
+
+        HttpGenerator gen = new HttpGenerator();
+
+        HttpGenerator.Result result = gen.generateResponse(null, null, null, content, true);
+        assertEquals(HttpGenerator.Result.NEED_INFO, result);
+        assertEquals(HttpGenerator.State.START, gen.getState());
+
+        MetaData.Response info = new MetaData.Response(HttpVersion.HTTP_0_9, 200, null, new HttpFields(), 10);
+        info.getFields().add("Content-Type", "test/data");
+        info.getFields().add("Last-Modified", DateGenerator.__01Jan1970);
+
+        result = gen.generateResponse(info, null, null, content, true);
+        assertEquals(HttpGenerator.Result.FLUSH, result);
+        assertEquals(HttpGenerator.State.COMPLETING, gen.getState());
+        String response = BufferUtil.toString(header);
+        BufferUtil.clear(header);
+        response += BufferUtil.toString(content);
+        BufferUtil.clear(content);        
+
+        result = gen.generateResponse(null, null, null, content, false);
+        assertEquals(HttpGenerator.Result.SHUTDOWN_OUT, result);
+        assertEquals(HttpGenerator.State.END, gen.getState());
+        
+        assertEquals(10, gen.getContentPrepared());
+        
+        assertThat(response, not(containsString("200 OK")));
+        assertThat(response, not(containsString("Last-Modified: Thu, 01 Jan 1970 00:00:00 GMT")));
+        assertThat(response, not(containsString("Content-Length: 10")));
+        assertThat(response, containsString("0123456789"));
+    }
     
     @Test
     public void testSimple() throws Exception
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 aaa78622637..c4bcfa5f7a7 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
@@ -114,19 +114,49 @@ public class HttpParserTest
         assertEquals(-1, _headers);
     }
 
+    @Test
+    public void testLineParse1_RFC2616() throws Exception
+    {
+        ByteBuffer buffer= BufferUtil.toBuffer("GET /999\015\012");
+        
+        HttpParser.RequestHandler handler  = new Handler();
+        HttpParser parser= new HttpParser(handler,HttpCompliance.RFC2616);
+        parseAll(parser,buffer);
+
+        assertNull(_bad);
+        assertEquals("GET", _methodOrVersion);
+        assertEquals("/999", _uriOrStatus);
+        assertEquals("HTTP/0.9", _versionOrReason);
+        assertEquals(-1, _headers);
+    }
+    
     @Test
     public void testLineParse1() throws Exception
     {
         ByteBuffer buffer= BufferUtil.toBuffer("GET /999\015\012");
 
-        _versionOrReason= null;
         HttpParser.RequestHandler handler  = new Handler();
         HttpParser parser= new HttpParser(handler);
         parseAll(parser,buffer);
-        
         assertEquals("HTTP/0.9 not supported", _bad);
     }
 
+    @Test
+    public void testLineParse2_RFC2616() throws Exception
+    {
+        ByteBuffer buffer= BufferUtil.toBuffer("POST /222  \015\012");
+
+        HttpParser.RequestHandler handler  = new Handler();
+        HttpParser parser= new HttpParser(handler,HttpCompliance.RFC2616);
+        parseAll(parser,buffer);
+
+        assertNull(_bad);
+        assertEquals("POST", _methodOrVersion);
+        assertEquals("/222", _uriOrStatus);
+        assertEquals("HTTP/0.9", _versionOrReason);
+        assertEquals(-1, _headers);
+    }
+
     @Test
     public void testLineParse2() throws Exception
     {
@@ -136,7 +166,7 @@ public class HttpParserTest
         HttpParser.RequestHandler handler  = new Handler();
         HttpParser parser= new HttpParser(handler);
         parseAll(parser,buffer);
-        assertEquals("HTTP/0.9 not supported", _bad);
+        assertEquals("HTTP/0.9 not supported", _bad);        
     }
 
     @Test
@@ -230,7 +260,7 @@ public class HttpParserTest
                 "\015\012");
         
         HttpParser.RequestHandler handler  = new Handler();
-        HttpParser parser= new HttpParser(handler,4096,HttpParser.Compliance.RFC2616);
+        HttpParser parser= new HttpParser(handler,4096,HttpCompliance.RFC2616);
         parseAll(parser,buffer);
 
         Assert.assertThat(_bad,Matchers.nullValue());
@@ -252,7 +282,7 @@ public class HttpParserTest
                 "\015\012");
         
         HttpParser.RequestHandler handler  = new Handler();
-        HttpParser parser= new HttpParser(handler,4096,HttpParser.Compliance.RFC7230);
+        HttpParser parser= new HttpParser(handler,4096,HttpCompliance.RFC7230);
         parseAll(parser,buffer);
 
         Assert.assertThat(_bad,Matchers.notNullValue());
@@ -602,7 +632,7 @@ public class HttpParserTest
                 "cOnNeCtIoN: ClOsE\015\012"+
                 "\015\012");
         HttpParser.RequestHandler handler  = new Handler();
-        HttpParser parser= new HttpParser(handler,-1,HttpParser.Compliance.RFC7230);
+        HttpParser parser= new HttpParser(handler,-1,HttpCompliance.RFC7230);
         parseAll(parser,buffer);
         assertNull(_bad);
         assertEquals("GET", _methodOrVersion);
@@ -624,7 +654,7 @@ public class HttpParserTest
                 "cOnNeCtIoN: ClOsE\015\012"+
                 "\015\012");
         HttpParser.RequestHandler handler  = new Handler();
-        HttpParser parser= new HttpParser(handler,-1,HttpParser.Compliance.STRICT);
+        HttpParser parser= new HttpParser(handler,-1,HttpCompliance.LEGACY);
         parseAll(parser,buffer);
         assertNull(_bad);
         assertEquals("gEt", _methodOrVersion);
diff --git a/jetty-server/src/main/config/etc/jetty-http.xml b/jetty-server/src/main/config/etc/jetty-http.xml
index ebeed5d1fc5..70d317bab86 100644
--- a/jetty-server/src/main/config/etc/jetty-http.xml
+++ b/jetty-server/src/main/config/etc/jetty-http.xml
@@ -29,6 +29,7 @@
             <Item>
               <New class="org.eclipse.jetty.server.HttpConnectionFactory">
                 <Arg name="config"><Ref refid="httpConfig" /></Arg>
+                <Arg name="compliance"><Call class="org.eclipse.jetty.http.HttpCompliance" name="valueOf"><Arg><Property name="jetty.http.compliance" default="RFC7230"/></Arg></Call></Arg>
               </New>
             </Item>
           </Array>
diff --git a/jetty-server/src/main/config/etc/jetty-https.xml b/jetty-server/src/main/config/etc/jetty-https.xml
index c9d497e2c15..71a0837796c 100644
--- a/jetty-server/src/main/config/etc/jetty-https.xml
+++ b/jetty-server/src/main/config/etc/jetty-https.xml
@@ -21,6 +21,7 @@
     <Arg>
       <New class="org.eclipse.jetty.server.HttpConnectionFactory">
         <Arg name="config"><Ref refid="sslHttpConfig" /></Arg>
+        <Arg name="compliance"><Call class="org.eclipse.jetty.http.HttpCompliance" name="valueOf"><Arg><Property name="jetty.http.compliance" default="RFC7230"/></Arg></Call></Arg>
       </New>
     </Arg>
   </Call>
diff --git a/jetty-server/src/main/config/modules/http.mod b/jetty-server/src/main/config/modules/http.mod
index 01e986243e5..d729bc2da6c 100644
--- a/jetty-server/src/main/config/modules/http.mod
+++ b/jetty-server/src/main/config/modules/http.mod
@@ -34,3 +34,6 @@ etc/jetty-http.xml
 
 ## Thread priority delta to give to acceptor threads
 # jetty.http.acceptorPriorityDelta=0
+
+## HTTP Compliance: RFC7230, RFC2616, LEGACY
+# jetty.http.compliance=RFC7230
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelOverHttp.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelOverHttp.java
index e6d24079a13..080cd67081e 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelOverHttp.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelOverHttp.java
@@ -260,6 +260,11 @@ class HttpChannelOverHttp extends HttpChannel implements HttpParser.RequestHandl
 
         switch (_metadata.getVersion())
         {
+            case HTTP_0_9:
+            {
+                persistent=false;
+                break;
+            }
             case HTTP_1_0:
             {
                 if (getHttpConfiguration().isPersistentConnectionsEnabled())
@@ -337,7 +342,7 @@ class HttpChannelOverHttp extends HttpChannel implements HttpParser.RequestHandl
 
             default:
             {
-                throw new IllegalStateException();
+                throw new IllegalStateException("unsupported version "+_metadata.getVersion());
             }
         }
 
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java
index 9d4f31516a2..29ae4927be2 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java
@@ -24,6 +24,7 @@ import java.nio.channels.WritePendingException;
 import java.util.concurrent.RejectedExecutionException;
 import java.util.concurrent.atomic.AtomicInteger;
 
+import org.eclipse.jetty.http.HttpCompliance;
 import org.eclipse.jetty.http.HttpField;
 import org.eclipse.jetty.http.HttpGenerator;
 import org.eclipse.jetty.http.HttpHeader;
@@ -90,7 +91,7 @@ public class HttpConnection extends AbstractConnection implements Runnable, Http
         return last;
     }
 
-    public HttpConnection(HttpConfiguration config, Connector connector, EndPoint endPoint, HttpParser.Compliance compliance)
+    public HttpConnection(HttpConfiguration config, Connector connector, EndPoint endPoint, HttpCompliance compliance)
     {
         super(endPoint, connector.getExecutor());
         _config = config;
@@ -119,7 +120,7 @@ public class HttpConnection extends AbstractConnection implements Runnable, Http
         return new HttpChannelOverHttp(this, _connector, _config, getEndPoint(), this);
     }
 
-    protected HttpParser newHttpParser(HttpParser.Compliance compliance)
+    protected HttpParser newHttpParser(HttpCompliance compliance)
     {
         return new HttpParser(newRequestHandler(), getHttpConfiguration().getRequestHeaderSize(), compliance);
     }
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnectionFactory.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnectionFactory.java
index 78297bf7986..8024422adc1 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnectionFactory.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnectionFactory.java
@@ -18,7 +18,7 @@
 
 package org.eclipse.jetty.server;
 
-import org.eclipse.jetty.http.HttpParser;
+import org.eclipse.jetty.http.HttpCompliance;
 import org.eclipse.jetty.http.HttpVersion;
 import org.eclipse.jetty.io.Connection;
 import org.eclipse.jetty.io.EndPoint;
@@ -32,17 +32,23 @@ import org.eclipse.jetty.util.annotation.Name;
 public class HttpConnectionFactory extends AbstractConnectionFactory implements HttpConfiguration.ConnectionFactory
 {
     private final HttpConfiguration _config;
-    private HttpParser.Compliance _httpCompliance=HttpParser.Compliance.RFC7230;
+    private HttpCompliance _httpCompliance;
 
     public HttpConnectionFactory()
     {
         this(new HttpConfiguration());
     }
-
+    
     public HttpConnectionFactory(@Name("config") HttpConfiguration config)
+    {
+        this(config,null);
+    }
+
+    public HttpConnectionFactory(@Name("config") HttpConfiguration config, @Name("compliance") HttpCompliance compliance)
     {
         super(HttpVersion.HTTP_1_1.asString());
         _config=config;
+        _httpCompliance=compliance==null?HttpCompliance.RFC7230:compliance;
         if (config==null)
             throw new IllegalArgumentException("Null HttpConfiguration");
         addBean(_config);
@@ -54,15 +60,15 @@ public class HttpConnectionFactory extends AbstractConnectionFactory implements
         return _config;
     }
 
-    public HttpParser.Compliance getHttpCompliance()
+    public HttpCompliance getHttpCompliance()
     {
         return _httpCompliance;
     }
 
     /**
-     * @param httpCompliance String value of {@link HttpParser.Compliance}
+     * @param httpCompliance String value of {@link HttpCompliance}
      */
-    public void setHttpCompliance(HttpParser.Compliance httpCompliance)
+    public void setHttpCompliance(HttpCompliance httpCompliance)
     {
         _httpCompliance = httpCompliance;
     }
diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/ExtendedServerTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/ExtendedServerTest.java
index 9c04d55c944..78864a78487 100644
--- a/jetty-server/src/test/java/org/eclipse/jetty/server/ExtendedServerTest.java
+++ b/jetty-server/src/test/java/org/eclipse/jetty/server/ExtendedServerTest.java
@@ -29,7 +29,7 @@ import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
-import org.eclipse.jetty.http.HttpParser;
+import org.eclipse.jetty.http.HttpCompliance;
 import org.eclipse.jetty.http.HttpVersion;
 import org.eclipse.jetty.io.Connection;
 import org.eclipse.jetty.io.EndPoint;
@@ -93,7 +93,7 @@ public class ExtendedServerTest extends HttpServerTestBase
 
     private static class ExtendedHttpConnection extends HttpConnection
     {
-        public ExtendedHttpConnection(HttpConfiguration config, Connector connector, EndPoint endPoint, HttpParser.Compliance compliance)
+        public ExtendedHttpConnection(HttpConfiguration config, Connector connector, EndPoint endPoint, HttpCompliance compliance)
         {
             super(config,connector,endPoint,compliance);
         }
diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/HttpConnectionTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/HttpConnectionTest.java
index 56f4899bf05..706c34144de 100644
--- a/jetty-server/src/test/java/org/eclipse/jetty/server/HttpConnectionTest.java
+++ b/jetty-server/src/test/java/org/eclipse/jetty/server/HttpConnectionTest.java
@@ -42,6 +42,7 @@ import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
+import org.eclipse.jetty.http.HttpCompliance;
 import org.eclipse.jetty.http.HttpHeader;
 import org.eclipse.jetty.http.HttpParser;
 import org.eclipse.jetty.http.MimeTypes;
@@ -236,6 +237,18 @@ public class HttpConnectionTest
         checkContains(response,0,"HTTP/1.1 400 Bad URI");
     }
 
+    @Test
+    public void test_0_9() throws Exception
+    {
+        connector.getConnectionFactory(HttpConnectionFactory.class).setHttpCompliance(HttpCompliance.RFC2616);
+        String response=connector.getResponses("GET /R1\n");
+
+        int offset=0;
+        checkNotContained(response,offset,"HTTP/1.1");
+        checkNotContained(response,offset,"200");
+        checkContains(response,offset,"pathInfo=/R1");
+    }
+    
     @Test
     public void testSimple() throws Exception
     {

From c3a54e9d0bbb5c9d3014abf9d43f348698ed5049 Mon Sep 17 00:00:00 2001
From: Greg Wilkins <gregw@webtide.com>
Date: Tue, 23 Feb 2016 18:23:23 +0100
Subject: [PATCH 2/5] fixed javadoc

---
 .../src/main/java/org/eclipse/jetty/http/HttpCompliance.java   | 1 +
 .../src/main/java/org/eclipse/jetty/http/HttpParser.java       | 3 +--
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpCompliance.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpCompliance.java
index dc2a1e307ee..8a74370430f 100644
--- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpCompliance.java
+++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpCompliance.java
@@ -27,5 +27,6 @@ package org.eclipse.jetty.http;
  * <dt>LEGACY</dt><dd>(aka STRICT) Adherence to Servlet Specification requirement for 
  * exact case of header names, bypassing the header caches, which are case insensitive, 
  * otherwise equivalent to RFC2616</dd>
+ * </dl>
  */
 public enum HttpCompliance { LEGACY, RFC2616, RFC7230 }
\ No newline at end of file
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 12244277c87..0b91a5582ed 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
@@ -81,8 +81,7 @@ import static org.eclipse.jetty.http.HttpTokens.TAB;
  * <dt>LEGACY</dt><dd>(aka STRICT) Adherence to Servlet Specification requirement for 
  * exact case of header names, bypassing the header caches, which are case insensitive, 
  * otherwise equivalent to RFC2616</dd>
- * </p>
- * <p>
+ * </dl>
  * @see <a href="http://tools.ietf.org/html/rfc7230">RFC 7230</a>
  */
 public class HttpParser

From 1eeecdaee515daf4e4fdf41e80d2b61678ba537d Mon Sep 17 00:00:00 2001
From: Greg Wilkins <gregw@webtide.com>
Date: Wed, 24 Feb 2016 13:36:06 +0100
Subject: [PATCH 3/5] Issue #355 Improve close behaviour

Inverted the logic of the handling of exceptions thrown in CommitCallback.
Now only if a BadMessageException is throw do we attempt to write a 500 response.
All other exceptions just abort the connection
---
 .../eclipse/jetty/http/BadMessageException.java    |  6 ++++++
 .../java/org/eclipse/jetty/http/HttpGenerator.java | 14 +++++++-------
 .../java/org/eclipse/jetty/server/HttpChannel.java | 14 +++++++-------
 3 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/BadMessageException.java b/jetty-http/src/main/java/org/eclipse/jetty/http/BadMessageException.java
index 0ef7fa65d7b..c3a68d8ef96 100644
--- a/jetty-http/src/main/java/org/eclipse/jetty/http/BadMessageException.java
+++ b/jetty-http/src/main/java/org/eclipse/jetty/http/BadMessageException.java
@@ -19,6 +19,12 @@
 package org.eclipse.jetty.http;
 
 /* ------------------------------------------------------------------------------- */
+/** 
+ * <p>Exception thrown to indicate a Bad HTTP Message has either been received
+ * or attempted to be generated.  Typically these are handled with either 400
+ * or 500 responses.</p>
+ */
+@SuppressWarnings("serial")
 public class BadMessageException extends RuntimeException
 {
     final int _code;
diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpGenerator.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpGenerator.java
index c21da96778b..f743379eb4b 100644
--- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpGenerator.java
+++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpGenerator.java
@@ -228,7 +228,7 @@ public class HttpGenerator
                     generateRequestLine(info,header);
 
                     if (info.getVersion()==HttpVersion.HTTP_0_9)
-                        throw new IllegalArgumentException("HTTP/0.9 not supported");
+                        throw new BadMessageException(500,"HTTP/0.9 not supported");
                     
                     generateHeaders(info,header,content,last);
 
@@ -256,7 +256,7 @@ public class HttpGenerator
                 catch(Exception e)
                 {
                     String message= (e instanceof BufferOverflowException)?"Request header too large":e.getMessage();
-                    throw new IOException(message,e);
+                    throw new BadMessageException(500,message,e);
                 }
                 finally
                 {
@@ -345,7 +345,7 @@ public class HttpGenerator
                     return Result.NEED_INFO;
                 HttpVersion version=info.getVersion();
                 if (version==null)
-                    throw new IllegalStateException("No version");
+                    throw new BadMessageException(500,"No version");
                 switch(version)
                 {
                     case HTTP_1_0:
@@ -411,7 +411,7 @@ public class HttpGenerator
                 catch(Exception e)
                 {
                     String message= (e instanceof BufferOverflowException)?"Response header too large":e.getMessage();
-                    throw new IOException(message,e);
+                    throw new BadMessageException(500,message,e);
                 }
                 finally
                 {
@@ -762,7 +762,7 @@ public class HttpGenerator
                     long actual_length = _contentPrepared+BufferUtil.length(content);
 
                     if (content_length>=0 && content_length!=actual_length)
-                        throw new IllegalArgumentException("Content-Length header("+content_length+") != actual("+actual_length+")");
+                        throw new BadMessageException(500,"Content-Length header("+content_length+") != actual("+actual_length+")");
                     
                     // Do we need to tell the headers about it
                     putContentLength(header,actual_length,content_type,request,response);
@@ -787,7 +787,7 @@ public class HttpGenerator
             }
 
             case NO_CONTENT:
-                throw new IllegalStateException();
+                throw new BadMessageException(500);
 
             case EOF_CONTENT:
                 _persistent = request!=null;
@@ -810,7 +810,7 @@ public class HttpGenerator
                 if (c.endsWith(HttpHeaderValue.CHUNKED.toString()))
                     putTo(transfer_encoding,header);
                 else
-                    throw new IllegalArgumentException("BAD TE");
+                    throw new BadMessageException(500,"BAD TE");
             }
             else
                 header.put(TRANSFER_ENCODING_CHUNKED);
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 4ba55f74806..50020249adb 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
@@ -785,12 +785,7 @@ public class HttpChannel implements Runnable, HttpOutput.Interceptor
             if (LOG.isDebugEnabled())
                 LOG.debug("Commit failed", x);
 
-            if (x instanceof EofException || x instanceof ClosedChannelException)
-            {
-                _callback.failed(x);
-                _response.getHttpOutput().closed();
-            }
-            else
+            if (x instanceof BadMessageException)
             {
                 _transport.send(HttpGenerator.RESPONSE_500_INFO, false, null, true, new Callback()
                 {
@@ -804,11 +799,16 @@ public class HttpChannel implements Runnable, HttpOutput.Interceptor
                     @Override
                     public void failed(Throwable th)
                     {
+                        _transport.abort(x);
                         _callback.failed(x);
-                        _response.getHttpOutput().closed();
                     }
                 });
             }
+            else
+            {
+                _transport.abort(x);
+                _callback.failed(x);
+            }
         }
     }
 

From 7b6c1af4a793350d51c9eda1dcd10f7e0c60da23 Mon Sep 17 00:00:00 2001
From: Greg Wilkins <gregw@webtide.com>
Date: Wed, 24 Feb 2016 13:47:35 +0100
Subject: [PATCH 4/5] Removed extra ByteBuffer duplicate

---
 .../src/main/java/org/eclipse/jetty/server/HttpOutput.java    | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java
index 71dc4d0ea42..d61c33a06de 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java
@@ -425,11 +425,9 @@ public class HttpOutput extends ServletOutputStream implements Runnable
         // write any remaining content in the buffer directly
         if (len>0)
         {
-            ByteBuffer wrap = ByteBuffer.wrap(b, off, len);
-            ByteBuffer view = wrap.duplicate();
-
             // write a buffer capacity at a time to avoid JVM pooling large direct buffers
             // http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6210541
+            ByteBuffer view = ByteBuffer.wrap(b, off, len);
             while (len>getBufferSize())
             {
                 int p=view.position();

From eb5e25ce1e0045dfd8b5cc07e41531da821317b5 Mon Sep 17 00:00:00 2001
From: Jan Bartel <janb@webtide.com>
Date: Wed, 24 Feb 2016 17:58:18 +0100
Subject: [PATCH 5/5] Fix javadoc comments

---
 .../jetty/server/SessionIdManager.java        |  14 ++-
 .../session/AbstractSessionDataStore.java     |  17 ++-
 .../session/AbstractSessionIdManager.java     |  38 ++++++-
 .../server/session/AbstractSessionStore.java  |  45 ++++++--
 .../session/CachingSessionDataStore.java      |   8 +-
 .../server/session/FileSessionDataStore.java  |  36 +++++-
 .../server/session/HashSessionIdManager.java  |   3 +-
 .../server/session/JDBCSessionDataStore.java  |   7 +-
 .../server/session/JDBCSessionIdManager.java  |  15 ++-
 .../server/session/MemorySessionStore.java    |  14 +--
 .../server/session/NullSessionDataStore.java  |  10 +-
 .../eclipse/jetty/server/session/Session.java |   2 +-
 .../jetty/server/session/SessionData.java     | 106 ++++++++++++++++++
 .../server/session/SessionDataStore.java      |  25 +++--
 14 files changed, 279 insertions(+), 61 deletions(-)

diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/SessionIdManager.java b/jetty-server/src/main/java/org/eclipse/jetty/server/SessionIdManager.java
index 68bcb3452ef..e4b545e7e88 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/SessionIdManager.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/SessionIdManager.java
@@ -28,19 +28,21 @@ import org.eclipse.jetty.util.component.LifeCycle;
  */
 public interface SessionIdManager extends LifeCycle
 {
+    /* ------------------------------------------------------------ */
     /**
      * @param id The plain session ID (ie no workername extension)
      * @return True if the session ID is in use by at least one context.
      */
     public boolean isIdInUse(String id);
     
-    
+    /* ------------------------------------------------------------ */
     /**
      * Notify the sessionid manager that a particular session id is in use
-     * @param the session whose id is being used
+     * @param session the session whose id is being used
      */
     public void useId (Session session);
     
+    /* ------------------------------------------------------------ */
     /**
      * Remove id
      * @param id the plain session id (no workername extension) of the session to remove
@@ -48,6 +50,8 @@ public interface SessionIdManager extends LifeCycle
      */
     public boolean removeId (String id);
     
+    
+    /* ------------------------------------------------------------ */
     /**
      * Invalidate all sessions on all contexts that share the same id.
      * 
@@ -55,6 +59,7 @@ public interface SessionIdManager extends LifeCycle
      */
     public void expireAll(String id);
     
+    /* ------------------------------------------------------------ */
     /**
      * Create a new Session ID.
      * 
@@ -65,7 +70,10 @@ public interface SessionIdManager extends LifeCycle
     public String newSessionId(HttpServletRequest request,long created);
     
     
-    
+    /* ------------------------------------------------------------ */
+    /** 
+     * @return the unique name of this server instance
+     */
     public String getWorkerName();
     
     
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionDataStore.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionDataStore.java
index 4b1d7cce973..57f88db1a61 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionDataStore.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionDataStore.java
@@ -32,11 +32,22 @@ public abstract class AbstractSessionDataStore extends AbstractLifeCycle impleme
     protected SessionContext _context; //context associated with this session data store
 
 
+    /**
+     * Store the session data persistently.
+     * 
+     * @param id identity of session to store
+     * @param data info of the session
+     * @param isNew has session been written out before or not
+     * @throws Exception
+     */
     public abstract void doStore(String id, SessionData data, boolean isNew) throws Exception;
 
    
 
     
+    /** 
+     * @see org.eclipse.jetty.server.session.SessionDataStore#initialize(org.eclipse.jetty.server.session.SessionContext)
+     */
     public void initialize (SessionContext context)
     {
         if (isStarted())
@@ -69,8 +80,9 @@ public abstract class AbstractSessionDataStore extends AbstractLifeCycle impleme
     }
     
 
+
     /** 
-     * @see org.eclipse.jetty.server.session.SessionDataStore#newSessionData(org.eclipse.jetty.server.session.SessionKey, long, long, long, long)
+     * @see org.eclipse.jetty.server.session.SessionDataStore#newSessionData(java.lang.String, long, long, long, long)
      */
     @Override
     public SessionData newSessionData(String id, long created, long accessed, long lastAccessed, long maxInactiveMs)
@@ -78,6 +90,9 @@ public abstract class AbstractSessionDataStore extends AbstractLifeCycle impleme
         return new SessionData(id, _context.getCanonicalContextPath(), _context.getVhost(), created, accessed, lastAccessed, maxInactiveMs);
     }
  
+    /**
+     * @throws IllegalStateException
+     */
     protected void checkStarted () throws IllegalStateException
     {
         if (isStarted())
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionIdManager.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionIdManager.java
index 20217aba909..f4e4aee5300 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionIdManager.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionIdManager.java
@@ -58,12 +58,19 @@ public abstract class AbstractSessionIdManager extends AbstractLifeCycle impleme
     protected SessionScavenger _scavenger;
 
     /* ------------------------------------------------------------ */
+    /**
+     * @param server the server associated with the id manager
+     */
     public AbstractSessionIdManager(Server server)
     {
         _server = server;
     }
 
     /* ------------------------------------------------------------ */
+    /**
+     * @param server the server associated with the id manager
+     * @param random a random number generator to use for ids
+     */
     public AbstractSessionIdManager(Server server, Random random)
     {
         this(server);
@@ -71,6 +78,9 @@ public abstract class AbstractSessionIdManager extends AbstractLifeCycle impleme
     }
 
     /* ------------------------------------------------------------ */
+    /**
+     * @param server the server associated with this id manager
+     */
     public void setServer (Server server)
     {
         _server = server;
@@ -78,7 +88,9 @@ public abstract class AbstractSessionIdManager extends AbstractLifeCycle impleme
     
     
     /* ------------------------------------------------------------ */
-
+    /**
+     * @return the server associated with this id manager
+     */
     public Server getServer ()
     {
         return _server;
@@ -88,7 +100,7 @@ public abstract class AbstractSessionIdManager extends AbstractLifeCycle impleme
     
     /* ------------------------------------------------------------ */
     /**
-     * @param scavenger
+     * @param scavenger a SessionScavenger 
      */
     public void setSessionScavenger (SessionScavenger scavenger)
     {
@@ -103,7 +115,7 @@ public abstract class AbstractSessionIdManager extends AbstractLifeCycle impleme
      * Get the workname. If set, the workername is dot appended to the session
      * ID and can be used to assist session affinity in a load balancer.
      *
-     * @return String or null
+     * @return name or null
      */
     @Override
     public String getWorkerName()
@@ -133,12 +145,18 @@ public abstract class AbstractSessionIdManager extends AbstractLifeCycle impleme
     }
 
     /* ------------------------------------------------------------ */
+    /**
+     * @return the random number generator
+     */
     public Random getRandom()
     {
         return _random;
     }
 
     /* ------------------------------------------------------------ */
+    /**
+     * @param random a random number generator for generating ids
+     */
     public synchronized void setRandom(Random random)
     {
         _random=random;
@@ -166,7 +184,7 @@ public abstract class AbstractSessionIdManager extends AbstractLifeCycle impleme
     /* ------------------------------------------------------------ */
     /**
      * Create a new session id if necessary.
-     *
+     * 
      * @see org.eclipse.jetty.server.SessionIdManager#newSessionId(javax.servlet.http.HttpServletRequest, long)
      */
     @Override
@@ -200,6 +218,10 @@ public abstract class AbstractSessionIdManager extends AbstractLifeCycle impleme
     }
 
     /* ------------------------------------------------------------ */
+    /**
+     * @param seedTerm
+     * @return a new unique session id
+     */
     public String newSessionId(long seedTerm)
     {
         // pick a new unique ID!
@@ -249,6 +271,9 @@ public abstract class AbstractSessionIdManager extends AbstractLifeCycle impleme
 
     
     /* ------------------------------------------------------------ */
+    /** 
+     * @see org.eclipse.jetty.util.component.AbstractLifeCycle#doStart()
+     */
     @Override
     protected void doStart() throws Exception
     {
@@ -268,6 +293,9 @@ public abstract class AbstractSessionIdManager extends AbstractLifeCycle impleme
     }
 
     /* ------------------------------------------------------------ */
+    /** 
+     * @see org.eclipse.jetty.util.component.AbstractLifeCycle#doStop()
+     */
     @Override
     protected void doStop() throws Exception
     {
@@ -418,7 +446,7 @@ public abstract class AbstractSessionIdManager extends AbstractLifeCycle impleme
     /* ------------------------------------------------------------ */
     /** Get SessionManager for every context.
      * 
-     * @return
+     * @return all session managers
      */
     protected Set<SessionManager> getSessionManagers()
     {
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionStore.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionStore.java
index eb2a54f752f..e5484d4634e 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionStore.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionStore.java
@@ -50,7 +50,7 @@ public abstract class AbstractSessionStore extends AbstractLifeCycle implements
     /**
      * Create a new Session object from session data
      * @param data
-     * @return
+     * @return a new Session object
      */
     public abstract Session newSession (SessionData data);
 
@@ -59,7 +59,7 @@ public abstract class AbstractSessionStore extends AbstractLifeCycle implements
     /**
      * Get the session matching the key
      * @param id session id
-     * @return
+     * @return the Session object matching the id
      */
     public abstract Session doGet(String id);
     
@@ -79,7 +79,7 @@ public abstract class AbstractSessionStore extends AbstractLifeCycle implements
     /**
      * Check to see if the session exists in the store
      * @param id
-     * @return
+     * @return true if the Session object exists in the session store
      */
     public abstract boolean doExists (String id);
     
@@ -97,7 +97,7 @@ public abstract class AbstractSessionStore extends AbstractLifeCycle implements
     
     /**
      * Get a list of keys for sessions that the store thinks has expired
-     * @return
+     * @return ids of all Session objects that might have expired
      */
     public abstract Set<String> doGetExpiredCandidates();
     
@@ -112,11 +112,17 @@ public abstract class AbstractSessionStore extends AbstractLifeCycle implements
     }
     
     
+    /**
+     * @param manager
+     */
     public void setSessionManager (SessionManager manager)
     {
         _manager = manager;
     }
     
+    /**
+     * @return the SessionManger
+     */
     public SessionManager getSessionManager()
     {
         return _manager;
@@ -124,6 +130,9 @@ public abstract class AbstractSessionStore extends AbstractLifeCycle implements
     
     
 
+    /** 
+     * @see org.eclipse.jetty.server.session.SessionStore#initialize(org.eclipse.jetty.server.session.SessionContext)
+     */
     public void initialize (SessionContext context)
     {
         if (isStarted())
@@ -131,6 +140,9 @@ public abstract class AbstractSessionStore extends AbstractLifeCycle implements
         _context = context;
     }
     
+    /** 
+     * @see org.eclipse.jetty.util.component.AbstractLifeCycle#doStart()
+     */
     @Override
     protected void doStart() throws Exception
     {
@@ -149,6 +161,9 @@ public abstract class AbstractSessionStore extends AbstractLifeCycle implements
         super.doStart();
     }
 
+    /** 
+     * @see org.eclipse.jetty.util.component.AbstractLifeCycle#doStop()
+     */
     @Override
     protected void doStop() throws Exception
     {
@@ -156,34 +171,47 @@ public abstract class AbstractSessionStore extends AbstractLifeCycle implements
         super.doStop();
     }
 
+    /**
+     * @return the SessionDataStore or null if there isn't one
+     */
     public SessionDataStore getSessionDataStore()
     {
         return _sessionDataStore;
     }
 
+    /**
+     * @param sessionDataStore
+     */
     public void setSessionDataStore(SessionDataStore sessionDataStore)
     {
         _sessionDataStore = sessionDataStore;
     }
     
+    /**
+     * @return the strategy for detecting stale sessions or null if there isn't one
+     */
     public StalenessStrategy getStaleStrategy()
     {
         return _staleStrategy;
     }
 
+    /**
+     * @param staleStrategy
+     */
     public void setStaleStrategy(StalenessStrategy staleStrategy)
     {
         _staleStrategy = staleStrategy;
     }
 
+
     /** 
-     * Get a session object.
+     *  Get a session object.
      * 
      * If the session object is not in this session store, try getting
      * the data for it from a SessionDataStore associated with the 
      * session manager.
      * 
-     * @see org.eclipse.jetty.server.session.SessionStore#get(java.lang.String)
+     * @see org.eclipse.jetty.server.session.SessionStore#get(java.lang.String, boolean)
      */
     @Override
     public Session get(String id, boolean staleCheck) throws Exception
@@ -292,7 +320,7 @@ public abstract class AbstractSessionStore extends AbstractLifeCycle implements
     
     /**
      * @param session
-     * @return
+     * @return true or false according to the StaleStrategy
      */
     public boolean isStale (Session session)
     {
@@ -319,6 +347,9 @@ public abstract class AbstractSessionStore extends AbstractLifeCycle implements
 
 
 
+    /** 
+     * @see org.eclipse.jetty.server.session.SessionStore#newSession(javax.servlet.http.HttpServletRequest, java.lang.String, long, long)
+     */
     @Override
     public Session newSession(HttpServletRequest request, String id, long time, long maxInactiveMs)
     {
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/CachingSessionDataStore.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/CachingSessionDataStore.java
index 463834dc767..9ba116a2b10 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/CachingSessionDataStore.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/CachingSessionDataStore.java
@@ -74,7 +74,7 @@ public class CachingSessionDataStore extends AbstractSessionDataStore
     
     
     /** 
-     * @see org.eclipse.jetty.server.session.SessionDataStore#load(org.eclipse.jetty.server.session.SessionKey)
+     * @see org.eclipse.jetty.server.session.SessionDataStore#load(java.lang.String)
      */
     @Override
     public SessionData load(String id) throws Exception
@@ -101,8 +101,9 @@ public class CachingSessionDataStore extends AbstractSessionDataStore
         return d;
     }
 
+
     /** 
-     * @see org.eclipse.jetty.server.session.SessionDataStore#delete(org.eclipse.jetty.server.session.SessionKey)
+     * @see org.eclipse.jetty.server.session.SessionDataStore#delete(java.lang.String)
      */
     @Override
     public boolean delete(String id) throws Exception
@@ -124,8 +125,9 @@ public class CachingSessionDataStore extends AbstractSessionDataStore
         return null;
     }
 
+
     /** 
-     * @see org.eclipse.jetty.server.session.AbstractSessionDataStore#doStore(org.eclipse.jetty.server.session.SessionKey, org.eclipse.jetty.server.session.SessionData, boolean)
+     * @see org.eclipse.jetty.server.session.AbstractSessionDataStore#doStore(java.lang.String, org.eclipse.jetty.server.session.SessionData, boolean)
      */
     @Override
     public void doStore(String id, SessionData data, boolean isNew) throws Exception
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/FileSessionDataStore.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/FileSessionDataStore.java
index fc8349f59f1..0686a5ddd5f 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/FileSessionDataStore.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/FileSessionDataStore.java
@@ -88,8 +88,9 @@ public class FileSessionDataStore extends AbstractSessionDataStore
     }
 
  
+
     /** 
-     * @see org.eclipse.jetty.server.session.SessionDataStore#delete(org.eclipse.jetty.server.session.SessionKey)
+     * @see org.eclipse.jetty.server.session.SessionDataStore#delete(java.lang.String)
      */
     @Override
     public boolean delete(String id) throws Exception
@@ -108,8 +109,9 @@ public class FileSessionDataStore extends AbstractSessionDataStore
         return false;
     }
 
+
     /** 
-     * @see org.eclipse.jetty.server.session.SessionDataStore#getExpired()
+     * @see org.eclipse.jetty.server.session.SessionDataStore#getExpired(java.util.Set)
      */
     @Override
     public Set<String> getExpired(Set<String> candidates)
@@ -119,8 +121,9 @@ public class FileSessionDataStore extends AbstractSessionDataStore
     }
 
 
+
     /** 
-     * @see org.eclipse.jetty.server.session.SessionDataStore#load(org.eclipse.jetty.server.session.SessionKey)
+     * @see org.eclipse.jetty.server.session.SessionDataStore#load(java.lang.String)
      */
     @Override
     public SessionData load(String id) throws Exception
@@ -173,9 +176,8 @@ public class FileSessionDataStore extends AbstractSessionDataStore
     
         
 
-    
     /** 
-     * @see org.eclipse.jetty.server.session.AbstractSessionDataStore#doStore(org.eclipse.jetty.server.session.SessionKey, org.eclipse.jetty.server.session.SessionData)
+     * @see org.eclipse.jetty.server.session.AbstractSessionDataStore#doStore(java.lang.String, org.eclipse.jetty.server.session.SessionData, boolean)
      */
     @Override
     public void doStore(String id, SessionData data, boolean isNew) throws Exception
@@ -200,6 +202,9 @@ public class FileSessionDataStore extends AbstractSessionDataStore
         }
     }
     
+    /**
+     * 
+     */
     public void initializeStore ()
     {
         if (_storeDir == null)
@@ -219,6 +224,12 @@ public class FileSessionDataStore extends AbstractSessionDataStore
     }
     
     /* ------------------------------------------------------------ */
+    /**
+     * @param os the output stream to save to
+     * @param id identity of the session
+     * @param data the info of the session
+     * @throws IOException
+     */
     private void save(OutputStream os, String id, SessionData data)  throws IOException
     {    
         DataOutputStream out = new DataOutputStream(os);
@@ -243,12 +254,21 @@ public class FileSessionDataStore extends AbstractSessionDataStore
         }
     }
 
+    /**
+     * @param id identity of session
+     * @return
+     */
     private String getFileName (String id)
     {
         return _context.getCanonicalContextPath()+"_"+_context.getVhost()+"_"+id;
     }
 
 
+    /**
+     * @param is inputstream containing session data
+     * @return
+     * @throws Exception
+     */
     private SessionData load (InputStream is)
             throws Exception
     {
@@ -289,6 +309,12 @@ public class FileSessionDataStore extends AbstractSessionDataStore
         }
     }
 
+    /**
+     * @param is inputstream containing session data
+     * @param size number of attributes
+     * @param data the data to restore to
+     * @throws Exception
+     */
     private void restoreAttributes (InputStream is, int size, SessionData data)
             throws Exception
     {
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/HashSessionIdManager.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/HashSessionIdManager.java
index ca632820d01..6bb37a2273a 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/HashSessionIdManager.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/HashSessionIdManager.java
@@ -54,8 +54,7 @@ public class HashSessionIdManager extends AbstractSessionIdManager
 
     
     /** 
-     * @see org.eclipse.jetty.server.SessionIdManager#useId(java.lang.String)
-     * @param session the session whose id to use
+     * @see org.eclipse.jetty.server.SessionIdManager#useId(org.eclipse.jetty.server.session.Session)
      */
     @Override
     public void useId(Session session)
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/JDBCSessionDataStore.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/JDBCSessionDataStore.java
index 4c9cf2d1a6c..ad029ec9215 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/JDBCSessionDataStore.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/JDBCSessionDataStore.java
@@ -630,7 +630,7 @@ public class JDBCSessionDataStore extends AbstractSessionDataStore
 
  
     /** 
-     * @see org.eclipse.jetty.server.session.SessionDataStore#load(org.eclipse.jetty.server.session.SessionKey)
+     * @see org.eclipse.jetty.server.session.SessionDataStore#load(java.lang.String)
      */
     @Override
     public SessionData load(String id) throws Exception
@@ -748,7 +748,7 @@ public class JDBCSessionDataStore extends AbstractSessionDataStore
 
 
     /** 
-     * @see org.eclipse.jetty.server.session.AbstractSessionDataStore#doStore()
+     * @see org.eclipse.jetty.server.session.AbstractSessionDataStore#doStore(java.lang.String, org.eclipse.jetty.server.session.SessionData, boolean)
      */
     @Override
     public void doStore(String id, SessionData data, boolean isNew) throws Exception
@@ -849,8 +849,9 @@ public class JDBCSessionDataStore extends AbstractSessionDataStore
     }
 
 
+
     /** 
-     * @see org.eclipse.jetty.server.session.SessionDataStore#getExpired()
+     * @see org.eclipse.jetty.server.session.SessionDataStore#getExpired(java.util.Set)
      */
     @Override
     public Set<String> getExpired(Set<String> candidates)
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/JDBCSessionIdManager.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/JDBCSessionIdManager.java
index cf1b5cb6424..6a82f9da872 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/JDBCSessionIdManager.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/JDBCSessionIdManager.java
@@ -160,10 +160,9 @@ public class JDBCSessionIdManager extends org.eclipse.jetty.server.session.Abstr
 
 
     
+
     /** 
-     * Record the session id as being in use
-     * 
-     * @see org.eclipse.jetty.server.SessionIdManager#useId(java.lang.String)
+     * @see org.eclipse.jetty.server.SessionIdManager#useId(org.eclipse.jetty.server.session.Session)
      */
     @Override
     public void useId (Session session)
@@ -225,7 +224,7 @@ public class JDBCSessionIdManager extends org.eclipse.jetty.server.session.Abstr
     /**
      * Insert a new used session id into the table.
      *
-     * @param id
+     * @param id the id to put into the table
      * @throws SQLException
      */
     private void insert (String id)
@@ -254,7 +253,7 @@ public class JDBCSessionIdManager extends org.eclipse.jetty.server.session.Abstr
     /**
      * Remove a session id from the table.
      *
-     * @param id
+     * @param id the id to remove from the table
      * @throws SQLException
      */
     private boolean delete (String id)
@@ -273,8 +272,8 @@ public class JDBCSessionIdManager extends org.eclipse.jetty.server.session.Abstr
     /**
      * Check if a session id exists.
      *
-     * @param id
-     * @return
+     * @param id the id to check
+     * @return true if the id exists in the table, false otherwise
      * @throws SQLException
      */
     private boolean exists (String id)
@@ -350,7 +349,7 @@ public class JDBCSessionIdManager extends org.eclipse.jetty.server.session.Abstr
 
     /**
      * Get the database adaptor in order to configure it
-     * @return
+     * @return the database adpator
      */
     public DatabaseAdaptor getDatabaseAdaptor ()
     {
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/MemorySessionStore.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/MemorySessionStore.java
index 92c70e03a2d..a4d62fbdd4a 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/MemorySessionStore.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/MemorySessionStore.java
@@ -51,11 +51,9 @@ public class MemorySessionStore extends AbstractSessionStore
      */
     public class MemorySession extends Session
     {
-
         /**
-         * @param manager
-         * @param request
-         * @param data
+         * @param request the request associated with the new session
+         * @param data the info for the session
          */
         public MemorySession(HttpServletRequest request, SessionData data)
         {
@@ -63,11 +61,8 @@ public class MemorySessionStore extends AbstractSessionStore
         }
         
         
-        
-
         /**
-         * @param manager
-         * @param data
+         * @param data the info for the restored session object
          */
         public MemorySession(SessionData data)
         {
@@ -222,8 +217,9 @@ public class MemorySessionStore extends AbstractSessionStore
     }
 
 
+ 
     /** 
-     * @see org.eclipse.jetty.server.session.SessionStore#newSession(java.lang.String)
+     * @see org.eclipse.jetty.server.session.AbstractSessionStore#newSession(javax.servlet.http.HttpServletRequest, java.lang.String, long, long)
      */
     @Override
     public Session newSession(HttpServletRequest request, String id, long time, long maxInactiveMs)
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/NullSessionDataStore.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/NullSessionDataStore.java
index 1e7a8e870c9..dc0f686f78a 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/NullSessionDataStore.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/NullSessionDataStore.java
@@ -38,9 +38,9 @@ public class NullSessionDataStore extends AbstractSessionDataStore
         return null;
     }
 
-    
+
     /** 
-     * @see org.eclipse.jetty.server.session.SessionDataStore#newSessionData(org.eclipse.jetty.server.session.SessionKey, long, long, long, long)
+     * @see org.eclipse.jetty.server.session.AbstractSessionDataStore#newSessionData(java.lang.String, long, long, long, long)
      */
     @Override
     public SessionData newSessionData(String id, long created, long accessed, long lastAccessed, long maxInactiveMs)
@@ -57,8 +57,9 @@ public class NullSessionDataStore extends AbstractSessionDataStore
        return true;
     }
 
+
     /** 
-     * @see org.eclipse.jetty.server.session.AbstractSessionDataStore#doStore()
+     * @see org.eclipse.jetty.server.session.AbstractSessionDataStore#doStore(java.lang.String, org.eclipse.jetty.server.session.SessionData, boolean)
      */
     @Override
     public void doStore(String id, SessionData data, boolean isNew) throws Exception
@@ -66,8 +67,9 @@ public class NullSessionDataStore extends AbstractSessionDataStore
         //noop
     }
 
+ 
     /** 
-     * @see org.eclipse.jetty.server.session.SessionDataStore#getExpired()
+     * @see org.eclipse.jetty.server.session.SessionDataStore#getExpired(java.util.Set)
      */
     @Override
     public Set<String> getExpired(Set<String> candidates)
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/Session.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/Session.java
index 6237fbeb174..43dd9a80604 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/Session.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/Session.java
@@ -755,7 +755,7 @@ public class Session implements SessionManager.SessionIf
     
     /* ------------------------------------------------------------- */
     /** 
-     * @see org.eclipse.jetty.server.session.AbstractSessionManager.SessionIf#getSession()
+     * @see org.eclipse.jetty.server.session.SessionManager.SessionIf#getSession()
      */
     @Override
     public Session getSession()
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionData.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionData.java
index 5588e299f16..7d0ea1b0ebf 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionData.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionData.java
@@ -84,6 +84,9 @@ public class SessionData implements Serializable
     }
     
 
+    /**
+     * @return time at which session was last written out
+     */
     public long getLastSaved()
     {
         return _lastSaved;
@@ -91,32 +94,53 @@ public class SessionData implements Serializable
 
 
 
+    /**
+     * @param lastSaved
+     */
     public void setLastSaved(long lastSaved)
     {
         _lastSaved = lastSaved;
     }
 
 
+    /**
+     * @return true if a session needs to be written out
+     */
     public boolean isDirty()
     {
         return _dirty;
     }
 
+    /**
+     * @param dirty
+     */
     public void setDirty(boolean dirty)
     {
         _dirty = dirty;
     }
     
+    /**
+     * @param name
+     * @return the value of the attribute named
+     */
     public Object getAttribute (String name)
     {
         return _attributes.get(name);
     }
 
+    /**
+     * @return a Set of attribute names
+     */
     public Set<String> getKeys()
     {
         return _attributes.keySet();
     }
     
+    /**
+     * @param name
+     * @param value
+     * @return
+     */
     public Object setAttribute (String name, Object value)
     {
         Object old = (value==null?_attributes.remove(name):_attributes.put(name,value));
@@ -128,6 +152,9 @@ public class SessionData implements Serializable
     }
     
     
+    /**
+     * @param name
+     */
     public void setDirty (String name)
     {
         setDirty (true);
@@ -135,101 +162,161 @@ public class SessionData implements Serializable
     
     
     
+    /**
+     * @param attributes
+     */
     public void putAllAttributes (Map<String,Object> attributes)
     {
         _attributes.putAll(attributes);
     }
     
+    /**
+     * @return an unmodifiable map of the attributes
+     */
     public Map<String,Object> getAllAttributes()
     {
         return Collections.unmodifiableMap(_attributes);
     }
     
+    /**
+     * @return the id of the session
+     */
     public String getId()
     {
         return _id;
     }
 
+    /**
+     * @param id
+     */
     public void setId(String id)
     {
         _id = id;
     }
 
+    /**
+     * @return the context path associated with this session
+     */
     public String getContextPath()
     {
         return _contextPath;
     }
 
+    /**
+     * @param contextPath
+     */
     public void setContextPath(String contextPath)
     {
         _contextPath = contextPath;
     }
 
+    /**
+     * @return virtual host of context associated with session
+     */
     public String getVhost()
     {
         return _vhost;
     }
 
+    /**
+     * @param vhost
+     */
     public void setVhost(String vhost)
     {
         _vhost = vhost;
     }
 
+    /**
+     * @return last node to manage the session
+     */
     public String getLastNode()
     {
         return _lastNode;
     }
 
+    /**
+     * @param lastNode
+     */
     public void setLastNode(String lastNode)
     {
         _lastNode = lastNode;
     }
 
+    /**
+     * @return time at which session expires
+     */
     public long getExpiry()
     {
         return _expiry;
     }
 
+    /**
+     * @param expiry
+     */
     public void setExpiry(long expiry)
     {
         _expiry = expiry;
     }
 
+    /**
+     * @return
+     */
     public long getCreated()
     {
         return _created;
     }
 
+    /**
+     * @param created
+     */
     public void setCreated(long created)
     {
         _created = created;
     }
 
+    /**
+     * @return time cookie was set
+     */
     public long getCookieSet()
     {
         return _cookieSet;
     }
 
+    /**
+     * @param cookieSet
+     */
     public void setCookieSet(long cookieSet)
     {
         _cookieSet = cookieSet;
     }
 
+    /**
+     * @return time session was accessed
+     */
     public long getAccessed()
     {
         return _accessed;
     }
 
+    /**
+     * @param accessed
+     */
     public void setAccessed(long accessed)
     {
         _accessed = accessed;
     }
 
+    /**
+     * @return previous time session was accessed
+     */
     public long getLastAccessed()
     {
         return _lastAccessed;
     }
 
+    /**
+     * @param lastAccessed
+     */
     public void setLastAccessed(long lastAccessed)
     {
         _lastAccessed = lastAccessed;
@@ -242,16 +329,26 @@ public class SessionData implements Serializable
 */
  
 
+    /**
+     * @return
+     */
     public long getMaxInactiveMs()
     {
         return _maxInactiveMs;
     }
 
+    /**
+     * @param maxInactive
+     */
     public void setMaxInactiveMs(long maxInactive)
     {
         _maxInactiveMs = maxInactive;
     }
 
+    /**
+     * @param out
+     * @throws IOException
+     */
     private void writeObject(java.io.ObjectOutputStream out) throws IOException
     {  
         out.writeUTF(_id); //session id
@@ -269,6 +366,11 @@ public class SessionData implements Serializable
         out.writeObject(_attributes);
     }
     
+    /**
+     * @param in
+     * @throws IOException
+     * @throws ClassNotFoundException
+     */
     private void readObject(java.io.ObjectInputStream in) throws IOException, ClassNotFoundException
     {
         _id = in.readUTF();
@@ -286,6 +388,10 @@ public class SessionData implements Serializable
     }
     
     
+    /**
+     * @param time
+     * @return
+     */
     public boolean isExpiredAt (long time)
     {
         if (LOG.isDebugEnabled())
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionDataStore.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionDataStore.java
index 785249ed74e..367d6f549d7 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionDataStore.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionDataStore.java
@@ -36,7 +36,7 @@ public interface SessionDataStore extends LifeCycle
      * given context. A SessionDataStore can only 
      * be used by one context(/session manager).
      * 
-     * @param context
+     * @param context context associated
      */
     void initialize(SessionContext context);
     
@@ -44,8 +44,8 @@ public interface SessionDataStore extends LifeCycle
     
     /**
      * Read in session data from storage
-     * @param id
-     * @return
+     * @param id identity of session to load
+     * @return the SessionData matching the id
      * @throws Exception
      */
     public SessionData load (String id) throws Exception;
@@ -53,7 +53,12 @@ public interface SessionDataStore extends LifeCycle
     
     /**
      * Create a new SessionData 
-     * @return
+     * @param id
+     * @param created
+     * @param accessed
+     * @param lastAccessed
+     * @param maxInactiveMs
+     * @return  a new SessionData object
      */
     public SessionData newSessionData (String id, long created, long accessed, long lastAccessed, long maxInactiveMs);
     
@@ -62,8 +67,8 @@ public interface SessionDataStore extends LifeCycle
     
     /**
      * Write out session data to storage
-     * @param id
-     * @param data
+     * @param id identity of session to store
+     * @param data info of session to store
      * @throws Exception
      */
     public void store (String id, SessionData data) throws Exception;
@@ -72,8 +77,8 @@ public interface SessionDataStore extends LifeCycle
     
     /**
      * Delete session data from storage
-     * @param id
-     * @return
+     * @param id identity of session to delete
+     * @return true if the session was deleted from the permanent store
      * @throws Exception
      */
     public boolean delete (String id) throws Exception;
@@ -88,7 +93,7 @@ public interface SessionDataStore extends LifeCycle
      * @param candidates if provided, these are keys of sessions that
      * the SessionStore thinks has expired and should be verified by the
      * SessionDataStore
-     * @return
+     * @return set of session ids
      */
     public Set<String> getExpired (Set<String> candidates);
     
@@ -96,7 +101,7 @@ public interface SessionDataStore extends LifeCycle
     
     /**
      * True if this type of datastore will passivate session objects
-     * @return
+     * @return true if this store can passivate sessions, false otherwise
      */
     public boolean isPassivating ();