315715 Improved Cookie version handling. Server.setMaxCookieVersion

git-svn-id: svn+ssh://dev.eclipse.org/svnroot/rt/org.eclipse.jetty/jetty/trunk@1917 7e9141cc-0065-0410-87d8-b60c137991c4

VERSION.txt

@@ -8,6 +8,7 @@ jetty-7.1.4-SNAPSHOT
  + 314299 Create test harness for JDBCLoginService
  + 314581 Implement the Sec-Websocket handshake
  + 315190 CrossOriginFilter adds headers not understood by Chrome 5 WebSocket implementation
+ + 315715 Improved Cookie version handling. Server.setMaxCookieVersion
 
 jetty-7.1.3.v20100526
  + 296567 HttpClient RedirectListener handles new HttpDestination

jetty-http/src/main/java/org/eclipse/jetty/http/HttpFields.java

@@ -283,17 +283,15 @@ public class HttpFields
     };
     
     
-   
-    
-    
-    
     public final static String __01Jan1970=formatCookieDate(0);
     public final static Buffer __01Jan1970_BUFFER=new ByteArrayBuffer(__01Jan1970);
 
     /* -------------------------------------------------------------- */
-    protected final ArrayList<Field> _fields = new ArrayList<Field>(20);
+    private final ArrayList<Field> _fields = new ArrayList<Field>(20);
-    protected final HashMap<Buffer,Field> _bufferMap = new HashMap<Buffer,Field>(32);
+    private final HashMap<Buffer,Field> _bufferMap = new HashMap<Buffer,Field>(32);
-    protected int _revision;
+    private final int _maxCookieVersion;
+    private int _revision;
+    
 
    
     
@@ -303,8 +301,18 @@ public class HttpFields
      */
     public HttpFields()
     {
+    	_maxCookieVersion=1;
     }
 
+    /* ------------------------------------------------------------ */
+    /**
+     * Constructor.
+     */
+    public HttpFields(int maxCookieVersion)
+    {
+    	_maxCookieVersion=maxCookieVersion;
+    }
+    
     /* -------------------------------------------------------------- */
     /**
      * Get enumeration of header _names. Returns an enumeration of strings representing the header
@@ -973,20 +981,29 @@ public class HttpFields
             final String comment, 
             final boolean isSecure,
             final boolean isHttpOnly, 
-            final int version)
+            int version)
     {
+    	String delim=_maxCookieVersion==0?"":"\"\\\n\r\t\f\b%+ ;=";
+    	
         // Check arguments
         if (name == null || name.length() == 0) throw new IllegalArgumentException("Bad cookie name");
 
         // Format value and params
         StringBuilder buf = new StringBuilder(128);
         String name_value_params;
-        QuotedStringTokenizer.quoteIfNeeded(buf, name);
+        boolean quoted = QuotedStringTokenizer.quoteIfNeeded(buf, name, delim);
         buf.append('=');
         String start=buf.toString();
         if (value != null && value.length() > 0)
-            QuotedStringTokenizer.quoteIfNeeded(buf, value);
+            quoted|=QuotedStringTokenizer.quoteIfNeeded(buf, value, delim);
+        
+        // upgrade to version 1 cookies if quoted.
+        if (quoted&&version==0 && _maxCookieVersion>=1)
+            version=1;
 
+        if (version>_maxCookieVersion)
+            version=_maxCookieVersion;
+        
         if (version > 0)
         {
             buf.append(";Version=");
@@ -994,7 +1011,7 @@ public class HttpFields
             if (comment != null && comment.length() > 0)
             {
                 buf.append(";Comment=");
-                QuotedStringTokenizer.quoteIfNeeded(buf, comment);
+                QuotedStringTokenizer.quoteIfNeeded(buf, comment, delim);
             }
         }
         if (path != null && path.length() > 0)
@@ -1003,25 +1020,24 @@ public class HttpFields
             if (path.trim().startsWith("\""))
                 buf.append(path);
             else
-                QuotedStringTokenizer.quoteIfNeeded(buf,path);
+                QuotedStringTokenizer.quoteIfNeeded(buf,path,delim);
         }
         if (domain != null && domain.length() > 0)
         {
             buf.append(";Domain=");
-            QuotedStringTokenizer.quoteIfNeeded(buf,domain.toLowerCase());
+            QuotedStringTokenizer.quoteIfNeeded(buf,domain.toLowerCase(),delim);
         }
 
         if (maxAge >= 0)
         {
-            if (version == 0)
+        	// Always add the expires param as some browsers still don't handle max-age
-            {
+        	buf.append(";Expires=");
-                buf.append(";Expires=");
+        	if (maxAge == 0)
-                if (maxAge == 0)
+        		buf.append(__01Jan1970);
-                    buf.append(__01Jan1970);
+        	else
-                else
+        		formatCookieDate(buf, System.currentTimeMillis() + 1000L * maxAge);
-                    formatCookieDate(buf, System.currentTimeMillis() + 1000L * maxAge);
+            
-            }
+            if (version >0)
-            else
             {
                 buf.append(";Max-Age=");
                 buf.append(maxAge);
@@ -1039,7 +1055,6 @@ public class HttpFields
 
         // TODO - straight to Buffer?
         name_value_params = buf.toString();
-        put(HttpHeaders.EXPIRES_BUFFER, __01Jan1970_BUFFER);
         
         // look for existing cookie
         Field field = getField(HttpHeaders.SET_COOKIE_BUFFER);
@@ -1059,6 +1074,9 @@ public class HttpFields
         }
         
         add(HttpHeaders.SET_COOKIE_BUFFER, new ByteArrayBuffer(name_value_params));
+        
+        // Expire responses with set-cookie headers so they do not get cached.
+        put(HttpHeaders.EXPIRES_BUFFER, __01Jan1970_BUFFER);
     }
 
     /* -------------------------------------------------------------- */

jetty-http/src/test/java/org/eclipse/jetty/http/HttpFieldsTest.java

@@ -13,6 +13,7 @@
 
 package org.eclipse.jetty.http;
 
+import java.net.CookieHandler;
 import java.util.Enumeration;
 import java.util.HashSet;
 import java.util.Set;
@@ -367,8 +368,19 @@ public class HttpFieldsTest
 
         fields.clear();
         fields.addSetCookie("ev erything","va lue","do main","pa th",1,"co mment",true,true,2);
-        assertEquals("\"ev erything\"=\"va lue\";Version=2;Comment=\"co mment\";Path=\"pa th\";Domain=\"do main\";Max-Age=1;Secure;HttpOnly",fields.getStringField("Set-Cookie"));
+        String setCookie=fields.getStringField("Set-Cookie");
+        assertTrue(setCookie.startsWith("\"ev erything\"=\"va lue\";Version=1;Comment=\"co mment\";Path=\"pa th\";Domain=\"do main\";Expires="));
+        assertTrue(setCookie.endsWith("GMT;Max-Age=1;Secure;HttpOnly"));
 
+        fields.clear();
+        fields.addSetCookie("name","value",null,null,-1,null,false,false,0);
+        setCookie=fields.getStringField("Set-Cookie");
+        assertEquals(-1,setCookie.indexOf("Version="));
+        fields.clear();
+        fields.addSetCookie("name","v a l u e",null,null,-1,null,false,false,0);
+        setCookie=fields.getStringField("Set-Cookie");
+        assertEquals(17,setCookie.indexOf("Version=1"));
+        
         fields.clear();
         fields.addSetCookie("json","{\"services\":[\"cwa\", \"aa\"]}",null,null,-1,null,false,false,-1);
         assertEquals("json=\"{\\\"services\\\":[\\\"cwa\\\", \\\"aa\\\"]}\"",fields.getStringField("Set-Cookie"));
@@ -386,6 +398,12 @@ public class HttpFieldsTest
         Enumeration e=fields.getValues("Set-Cookie");
         assertEquals("name=more;Domain=domain",e.nextElement());
         assertEquals("foo=bob;Domain=domain",e.nextElement());
+        
+        fields=new HttpFields(0);
+        fields.addSetCookie("name","value==",null,null,-1,null,false,false,0);
+        setCookie=fields.getStringField("Set-Cookie");
+        assertEquals("name=value==",setCookie);
+        
     }
 
     private Set<String> enum2set(Enumeration<String> e)

jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java

@@ -147,7 +147,7 @@ public class HttpConnection implements Connection
         HttpBuffers ab = (HttpBuffers)_connector;
         _parser = new HttpParser(ab.getRequestBuffers(), endpoint, new RequestHandler());
         _requestFields = new HttpFields();
-        _responseFields = new HttpFields();
+        _responseFields = new HttpFields(server.getMaxCookieVersion());
         _request = new Request(this);
         _response = new Response(this);
         _generator = new HttpGenerator(ab.getResponseBuffers(), _endp);
@@ -164,7 +164,7 @@ public class HttpConnection implements Connection
         _endp = endpoint;
         _parser = parser;
         _requestFields = new HttpFields();
-        _responseFields = new HttpFields();
+        _responseFields = new HttpFields(server.getMaxCookieVersion());
         _request = request;
         _response = new Response(this);
         _generator = generator;
@@ -1175,13 +1175,13 @@ public class HttpConnection implements Connection
                             else
                             {
                                 _responseFields.put(HttpHeaders.CONTENT_TYPE_BUFFER,
-                                        contentType+";charset="+QuotedStringTokenizer.quote(enc,";= "));
+                                        contentType+";charset="+QuotedStringTokenizer.quoteIfNeeded(enc,";= "));
                             }
                         }
                         else
                         {
                             _responseFields.put(HttpHeaders.CONTENT_TYPE_BUFFER,
-                                    contentType+";charset="+QuotedStringTokenizer.quote(enc,";= "));
+                                    contentType+";charset="+QuotedStringTokenizer.quoteIfNeeded(enc,";= "));
                         }
                     }
                 }

jetty-server/src/main/java/org/eclipse/jetty/server/Response.java

@@ -711,7 +711,7 @@ public class Response implements HttpServletResponse
 
                         if (_contentType==null)
                         {
-                            _contentType = _mimeType+";charset="+QuotedStringTokenizer.quote(_characterEncoding,";= ");
+                            _contentType = _mimeType+";charset="+QuotedStringTokenizer.quoteIfNeeded(_characterEncoding,";= ");
                             _connection.getResponseFields().put(HttpHeaders.CONTENT_TYPE_BUFFER,_contentType);
                         }
                     }
@@ -720,16 +720,16 @@ public class Response implements HttpServletResponse
                         int i1=_contentType.indexOf("charset=",i0);
                         if (i1<0)
                         {
-                            _contentType = _contentType+";charset="+QuotedStringTokenizer.quote(_characterEncoding,";= ");
+                            _contentType = _contentType+";charset="+QuotedStringTokenizer.quoteIfNeeded(_characterEncoding,";= ");
                         }
                         else
                         {
                             int i8=i1+8;
                             int i2=_contentType.indexOf(" ",i8);
                             if (i2<0)
-                                _contentType=_contentType.substring(0,i8)+QuotedStringTokenizer.quote(_characterEncoding,";= ");
+                                _contentType=_contentType.substring(0,i8)+QuotedStringTokenizer.quoteIfNeeded(_characterEncoding,";= ");
                             else
-                                _contentType=_contentType.substring(0,i8)+QuotedStringTokenizer.quote(_characterEncoding,";= ")+_contentType.substring(i2);
+                                _contentType=_contentType.substring(0,i8)+QuotedStringTokenizer.quoteIfNeeded(_characterEncoding,";= ")+_contentType.substring(i2);
                         }
                         _connection.getResponseFields().put(HttpHeaders.CONTENT_TYPE_BUFFER,_contentType);
                     }
@@ -857,12 +857,12 @@ public class Response implements HttpServletResponse
                         }
                         else if (i2<0)
                         {
-                            _contentType=contentType.substring(0,i1)+";charset="+QuotedStringTokenizer.quote(_characterEncoding,";= ");
+                            _contentType=contentType.substring(0,i1)+";charset="+QuotedStringTokenizer.quoteIfNeeded(_characterEncoding,";= ");
                             _connection.getResponseFields().put(HttpHeaders.CONTENT_TYPE_BUFFER,_contentType);
                         }
                         else
                         {
-                            _contentType=contentType.substring(0,i1)+contentType.substring(i2)+";charset="+QuotedStringTokenizer.quote(_characterEncoding,";= ");
+                            _contentType=contentType.substring(0,i1)+contentType.substring(i2)+";charset="+QuotedStringTokenizer.quoteIfNeeded(_characterEncoding,";= ");
                             _connection.getResponseFields().put(HttpHeaders.CONTENT_TYPE_BUFFER,_contentType);
                         }
                     }
@@ -908,7 +908,7 @@ public class Response implements HttpServletResponse
                 else // No encoding in the params.
                 {
                     _cachedMimeType=null;
-                    _contentType=_characterEncoding==null?contentType:contentType+";charset="+QuotedStringTokenizer.quote(_characterEncoding,";= ");
+                    _contentType=_characterEncoding==null?contentType:contentType+";charset="+QuotedStringTokenizer.quoteIfNeeded(_characterEncoding,";= ");
                     _connection.getResponseFields().put(HttpHeaders.CONTENT_TYPE_BUFFER,_contentType);
                 }
             }
@@ -929,13 +929,13 @@ public class Response implements HttpServletResponse
                         }
                         else
                         {
-                            _contentType=_mimeType+";charset="+QuotedStringTokenizer.quote(_characterEncoding,";= ");
+                            _contentType=_mimeType+";charset="+QuotedStringTokenizer.quoteIfNeeded(_characterEncoding,";= ");
                             _connection.getResponseFields().put(HttpHeaders.CONTENT_TYPE_BUFFER,_contentType);
                         }
                     }
                     else
                     {
-                        _contentType=contentType+";charset="+QuotedStringTokenizer.quote(_characterEncoding,";= ");
+                        _contentType=contentType+";charset="+QuotedStringTokenizer.quoteIfNeeded(_characterEncoding,";= ");
                         _connection.getResponseFields().put(HttpHeaders.CONTENT_TYPE_BUFFER,_contentType);
                     }
                 }

jetty-server/src/main/java/org/eclipse/jetty/server/Server.java

@@ -69,7 +69,9 @@ public class Server extends HandlerWrapper implements Attributes
     private boolean _sendDateHeader = false; //send Date: header
     private int _graceful=0;
     private boolean _stopAtShutdown;
+    private int _maxCookieVersion=1;
     
+
     /* ------------------------------------------------------------ */
     public Server()
     {
@@ -433,7 +435,24 @@ public class Server extends HandlerWrapper implements Attributes
     {
         return _sendDateHeader;
     }
-    
+
+    /* ------------------------------------------------------------ */
+    /** Get the maximum cookie version.
+     * @return the maximum set-cookie version sent by this server
+     */
+    public int getMaxCookieVersion()
+    {
+        return _maxCookieVersion;
+    }
+
+    /* ------------------------------------------------------------ */
+    /** Set the maximum cookie version.
+     * @param maxCookieVersion the maximum set-cookie version sent by this server
+     */
+    public void setMaxCookieVersion(int maxCookieVersion)
+    {
+        _maxCookieVersion = maxCookieVersion;
+    }
 
     /* ------------------------------------------------------------ */
     /**

jetty-util/src/main/java/org/eclipse/jetty/util/QuotedStringTokenizer.java

@@ -267,7 +267,7 @@ public class QuotedStringTokenizer
      * @param s The string to quote.
      * @return quoted string
      */
-    public static String quote(String s, String delim)
+    public static String quoteIfNeeded(String s, String delim)
     {
         if (s==null)
             return null;
@@ -413,88 +413,30 @@ public class QuotedStringTokenizer
 
 
     /* ------------------------------------------------------------ */
-    /** Quote a string into a StringBuffer.
+    /** Quote a string into a StringBuffer only if needed.
-     * The characters ", \, \n, \r, \t, \f, \b are escaped.
+     * Quotes are forced if any delim characters are present.
-     * Quotes are forced if any escaped characters are present or there
-     * is a ", ', space, +, =, ; or % character.
      * 
      * @param buf The StringBuffer
      * @param s The String to quote.
+     * @param delim String of characters that must be quoted.
+     * @return true if quoted;
      */
-    public static void quoteIfNeeded(Appendable buf, String s)
+    public static boolean quoteIfNeeded(Appendable buf, String s,String delim)
     {
+        for (int i=0;i<s.length();i++)
+        {
+            char c = s.charAt(i);
+            if (delim.indexOf(c)>=0)
+            {
+            	quote(buf,s);
+            	return true;
+            }
+        }
+    	
         try
         {
-            int e=-1;
+            buf.append(s);
-
+            return false;
-            search: for (int i=0;i<s.length();i++)
-            {
-                char c = s.charAt(i);
-                switch(c)
-                {
-                    case '"':
-                    case '\\':
-                    case '\n':
-                    case '\r':
-                    case '\t':
-                    case '\f':
-                    case '\b':
-                    case '%':
-                    case '+':
-                    case ' ':
-                    case ';':
-                    case '=':
-                        e=i;
-                        buf.append('"');
-                        // TODO when 1.4 support is dropped: buf.append(s,0,e);
-                        for (int j=0;j<e;j++)
-                            buf.append(s.charAt(j));
-                        break search;
-
-                    default:
-                        continue;
-                }
-            }
-
-            if (e<0)
-            {
-                buf.append(s);
-                return;
-            }
-
-            for (int i=e;i<s.length();i++)
-            {
-                char c = s.charAt(i);
-                switch(c)
-                {
-                    case '"':
-                        buf.append("\\\"");
-                        continue;
-                    case '\\':
-                        buf.append("\\\\");
-                        continue;
-                    case '\n':
-                        buf.append("\\n");
-                        continue;
-                    case '\r':
-                        buf.append("\\r");
-                        continue;
-                    case '\t':
-                        buf.append("\\t");
-                        continue;
-                    case '\f':
-                        buf.append("\\f");
-                        continue;
-                    case '\b':
-                        buf.append("\\b");
-                        continue;
-
-                    default:
-                        buf.append(c);
-                        continue;
-                }
-            }
-            buf.append('"');
         }
         catch(IOException e)
         {

jetty-util/src/test/java/org/eclipse/jetty/util/QuotedStringTokenizerTest.java

@@ -108,11 +108,11 @@ public class QuotedStringTokenizerTest
         assertEquals("\"abcefg\\\"\"",buf.toString());
         
         buf.setLength(0);
-        QuotedStringTokenizer.quoteIfNeeded(buf,"abc \n efg");
+        QuotedStringTokenizer.quoteIfNeeded(buf,"abc \n efg","\"\\\n\r\t\f\b%+ ;=");
         assertEquals("\"abc \\n efg\"",buf.toString());
         
         buf.setLength(0);
-        QuotedStringTokenizer.quoteIfNeeded(buf,"abcefg");
+        QuotedStringTokenizer.quoteIfNeeded(buf,"abcefg","\"\\\n\r\t\f\b%+ ;=");
         assertEquals("abcefg",buf.toString());
         
     }
@@ -154,9 +154,9 @@ public class QuotedStringTokenizerTest
     @Test
     public void testQuoteString()
     {
-        assertEquals("abc",QuotedStringTokenizer.quote("abc", " ,"));
+        assertEquals("abc",QuotedStringTokenizer.quoteIfNeeded("abc", " ,"));
-        assertEquals("\"a c\"",QuotedStringTokenizer.quote("a c", " ,"));
+        assertEquals("\"a c\"",QuotedStringTokenizer.quoteIfNeeded("a c", " ,"));
-        assertEquals("\"a'c\"",QuotedStringTokenizer.quote("a'c", " ,"));  
+        assertEquals("\"a'c\"",QuotedStringTokenizer.quoteIfNeeded("a'c", " ,"));  
         assertEquals("\"a\\n\\r\\t\"",QuotedStringTokenizer.quote("a\n\r\t")); 
     }
 

test-jetty-webapp/src/test/java/org/eclipse/jetty/TestServer.java

@@ -13,6 +13,7 @@
 
 package org.eclipse.jetty;
 
+import java.io.File;
 import java.lang.management.ManagementFactory;
 
 import org.eclipse.jetty.jmx.MBeanContainer;
@@ -35,8 +36,7 @@ public class TestServer
 {
     public static void main(String[] args) throws Exception
     {
-        String jetty_home = System.getProperty("jetty.home","../jetty-distribution/target/distribution");
+        String jetty_root = "..";
-        System.setProperty("jetty.home",jetty_home);
 
         Server server = new Server();
         server.setSendDateHeader(true);
@@ -63,10 +63,10 @@ public class TestServer
 
         SslSelectChannelConnector ssl_connector = new SslSelectChannelConnector();
         ssl_connector.setPort(8443);
-        ssl_connector.setKeystore(jetty_home + "/etc/keystore");
+        ssl_connector.setKeystore(jetty_root + "/jetty-server/src/main/config/etc/keystore");
         ssl_connector.setPassword("OBF:1vny1zlo1x8e1vnw1vn61x8g1zlu1vn4");
         ssl_connector.setKeyPassword("OBF:1u2u1wml1z7s1z7a1wnl1u2g");
-        ssl_connector.setTruststore(jetty_home + "/etc/keystore");
+        ssl_connector.setTruststore(jetty_root + "/jetty-server/src/main/config/etc/keystore");
         ssl_connector.setTrustPassword("OBF:1vny1zlo1x8e1vnw1vn61x8g1zlu1vn4");
         server.addConnector(ssl_connector);
 
@@ -82,10 +82,11 @@ public class TestServer
 
         HashLoginService login = new HashLoginService();
         login.setName("Test Realm");
-        login.setConfig(jetty_home + "/etc/realm.properties");
+        login.setConfig(jetty_root + "/test-jetty-webapp/src/main/config/etc/realm.properties");
         server.addBean(login);
 
-        NCSARequestLog requestLog = new NCSARequestLog(jetty_home + "/logs/jetty-yyyy_mm_dd.log");
+        File log=File.createTempFile("jetty-yyyy_mm_dd", "log");
+        NCSARequestLog requestLog = new NCSARequestLog(log.toString());
         requestLog.setExtended(false);
         requestLogHandler.setRequestLog(requestLog);