svn commit: r831116 - in /tomcat/trunk: java/javax/servlet/http/Cookie.java webapps/docs/config/systemprops.xml

[markt-2]

Author: markt
Date: Thu Oct 29 22:14:26 2009
New Revision: 831116

URL: http://svn.apache.org/viewvc?rev=831116&view=rev
Log:
Apply Konstantin's review comments

Modified:
    tomcat/trunk/java/javax/servlet/http/Cookie.java
    tomcat/trunk/webapps/docs/config/systemprops.xml

Modified: tomcat/trunk/java/javax/servlet/http/Cookie.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/javax/servlet/http/Cookie.java?rev=831116&r1=831115&r2=831116&view=diff
==============================================================================
--- tomcat/trunk/java/javax/servlet/http/Cookie.java (original)
+++ tomcat/trunk/java/javax/servlet/http/Cookie.java Thu Oct 29 22:14:26 2009
@@ -487,8 +487,9 @@
     // private static final String tspecials = "()<>@,;:\\\"/[]?={} \t";
 
     private static final String tspecials = ",; ";
-    private static final String tspecials2 = "()<>@,;:\\\"/[]?={} \t";
-    private static final String tspecials2NoSlash = "()<>@,;:\\\"/[]?={} \t";
+    private static final String tspecials2NoSlash = "()<>@,;:\\\"[]?={} \t";
+    private static final String tspecials2WithSlash = tspecials2NoSlash + "/";
+    private static final String tspecials2;
     
     /**
      * If set to true, we parse cookies strictly according to the servlet,
@@ -504,9 +505,10 @@
     private static final boolean FWD_SLASH_IS_SEPARATOR;
 
     /**
-     * If set to false, we don't use the IE6/7 Max-Age/Expires work around.
-     * Default is usually true. If STRICT_SERVLET_COMPLIANCE==true then default
-     * is false. Explicitly setting always takes priority.
+     * If set to true, enforce the cookie naming rules in the spec that require
+     * no separators in the cookie name. Default is usually false. If
+     * STRICT_SERVLET_COMPLIANCE==true then default is true. Explicitly setting
+     * always takes priority.
      */
     private static final boolean STRICT_NAMING;
 
@@ -525,8 +527,14 @@
                 Boolean.valueOf(fwdSlashIsSeparator).booleanValue();
         }
 
+        if (FWD_SLASH_IS_SEPARATOR) {
+            tspecials2 = tspecials2WithSlash;
+        } else {
+            tspecials2 = tspecials2NoSlash;
+        }
+        
         String strictNaming = System.getProperty(
-                "javax.servlet.http.Cookie.STRICT_NAMING");
+                "org.apache.tomcat.util.http.ServerCookie.STRICT_NAMING");
         if (strictNaming == null) {
             STRICT_NAMING = STRICT_SERVLET_COMPLIANCE;
         } else {
@@ -555,13 +563,8 @@
         for (int i = 0; i < len; i++) {
             char c = value.charAt(i);
 
-            if (c < 0x20 ||
-                    c >= 0x7f ||
-                    (!STRICT_NAMING && tspecials.indexOf(c) != -1) ||
-                    (STRICT_NAMING && !FWD_SLASH_IS_SEPARATOR &&
-                            tspecials2NoSlash.indexOf(c) != -1) ||
-                    (STRICT_NAMING && FWD_SLASH_IS_SEPARATOR &&
-                            tspecials2.indexOf(c) != -1)) {
+            if (c < 0x20 || c >= 0x7f ||
+                    (STRICT_NAMING && tspecials2.indexOf(c) != -1)) {
                 return false;
             }
         }

Modified: tomcat/trunk/webapps/docs/config/systemprops.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/config/systemprops.xml?rev=831116&r1=831115&r2=831116&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/config/systemprops.xml (original)
+++ tomcat/trunk/webapps/docs/config/systemprops.xml Thu Oct 29 22:14:26 2009
@@ -296,7 +296,7 @@
     </property>
 
     <property
-    name="javax.servlet.http.Cookie. STRICT_NAMING">
+    name="org.apache.tomcat.util.http. ServerCookie.STRICT_NAMING">
       <p> If this is true then the requirements of the Servlet specification
       that Cookie names must adhere to RFC2109 (no use of separators) will be
       enforced. If not specified, the default value will be used. If



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@...
For additional commands, e-mail: dev-help@...

[Konstantin Kolinko]

2009/10/30  <markt@...>:

> Author: markt
> Date: Thu Oct 29 22:14:26 2009
> New Revision: 831116
>
> URL: http://svn.apache.org/viewvc?rev=831116&view=rev
> Log:
> Apply Konstantin's review comments
>
> Modified:
>    tomcat/trunk/java/javax/servlet/http/Cookie.java
>    tomcat/trunk/webapps/docs/config/systemprops.xml
>

Now it is OK for me.

Thank you.

Best regards,
Konstantin Kolinko

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@...
For additional commands, e-mail: dev-help@...

[Filip Hanik - Dev Lists]

The EG group is already receiving reports on the change of breaking apps
cookie code is simply such, that when you touch it, you break something.
This seems to be the rule rather than the exception :)

---------------------8<----------------------

just a heads up: We are already receiving reports that this change:

-    private static final String tspecials = ",; ";
+    private static final String tspecials = "/()<>@,;:\\\"[]?={} \t";


is breaking several existing apps.

We may have to revert it.



On 10/29/2009 06:46 PM, Konstantin Kolinko wrote:

> 2009/10/30<markt@...>:
>    
>> Author: markt
>> Date: Thu Oct 29 22:14:26 2009
>> New Revision: 831116
>>
>> URL: http://svn.apache.org/viewvc?rev=831116&view=rev
>> Log:
>> Apply Konstantin's review comments
>>
>> Modified:
>>     tomcat/trunk/java/javax/servlet/http/Cookie.java
>>     tomcat/trunk/webapps/docs/config/systemprops.xml
>>
>>      
> Now it is OK for me.
>
> Thank you.
>
> Best regards,
> Konstantin Kolinko
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@...
> For additional commands, e-mail: dev-help@...
>
>
>    

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@...
For additional commands, e-mail: dev-help@...

[markt-2]

Filip Hanik - Dev Lists wrote:

> The EG group is already receiving reports on the change of breaking apps
> cookie code is simply such, that when you touch it, you break something.
> This seems to be the rule rather than the exception :)
>
> ---------------------8<----------------------
>
> just a heads up: We are already receiving reports that this change:
>
> -    private static final String tspecials = ",; ";
> +    private static final String tspecials = "/()<>@,;:\\\"[]?={} \t";
>
>
> is breaking several existing apps.
>
> We may have to revert it.

That doesn't surprise me at all.

The options and associated defaults in Tomcat 7 should protect our users against
this and if they do hit it, there are options available to disable it.

Mark

>
>
>
> On 10/29/2009 06:46 PM, Konstantin Kolinko wrote:
>> 2009/10/30<markt@...>:
>>  
>>> Author: markt
>>> Date: Thu Oct 29 22:14:26 2009
>>> New Revision: 831116
>>>
>>> URL: http://svn.apache.org/viewvc?rev=831116&view=rev
>>> Log:
>>> Apply Konstantin's review comments
>>>
>>> Modified:
>>>     tomcat/trunk/java/javax/servlet/http/Cookie.java
>>>     tomcat/trunk/webapps/docs/config/systemprops.xml
>>>
>>>      
>> Now it is OK for me.
>>
>> Thank you.
>>
>> Best regards,
>> Konstantin Kolinko
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@...
>> For additional commands, e-mail: dev-help@...
>>
>>
>>    
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@...
> For additional commands, e-mail: dev-help@...
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@...
For additional commands, e-mail: dev-help@...