You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2008/03/18 18:04:35 UTC

svn commit: r638442 - /tomcat/tc6.0.x/trunk/STATUS.txt

Author: markt
Date: Tue Mar 18 10:04:22 2008
New Revision: 638442

URL: http://svn.apache.org/viewvc?rev=638442&view=rev
Log:
Withdraw previous alternative proposal and replace with an improved patch that addresses the concerns raised.

Modified:
    tomcat/tc6.0.x/trunk/STATUS.txt

Modified: tomcat/tc6.0.x/trunk/STATUS.txt
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/STATUS.txt?rev=638442&r1=638441&r2=638442&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/STATUS.txt (original)
+++ tomcat/tc6.0.x/trunk/STATUS.txt Tue Mar 18 10:04:22 2008
@@ -105,10 +105,11 @@
             elements of the cookie header.
   -1:
 
-* As an alternative to the above patch, I propose:
-  http://people.apache.org/~markt/patches/2008-03-15-cookie-path.patch
-  +1: markt, remm, jfclere
-  -1: maybeQuote2 should not be applied to the path, since / is a valid value without quotes
+* As an alternative to the above patch, I propose an improved patch that
+  addresses the concerns with the previous alternative:
+  http://people.apache.org/~markt/patches/2008-03-18-cookie-path.patch
+  +1: markt
+  -1:
 
 * Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=44562
   http://svn.apache.org/viewvc?rev=635294&view=rev (prior code clean up)



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


Re: svn commit: r638442 - /tomcat/tc6.0.x/trunk/STATUS.txt

Posted by Remy Maucherat <re...@apache.org>.
On Tue, 2008-03-18 at 16:05 -0600, Filip Hanik - Dev Lists wrote:
> Mark Thomas wrote:
> > Filip Hanik - Dev Lists wrote:
> >> this wont work either. for the value of the cookie, it must be what 
> >> it was before. tspecials2. those are the correct characters for the 
> >> "value" of the cookie, but they are not correct for the "path"
> >
> > I'm not convinced of this yet. I am doing some more testing but I 
> > think things are being complicated by some potential bugs in the new 
> > parsing code. Now we have the test cases, I'll look at adding some 
> > more tests and go from there.
> don't think there are any bugs in the parsing code, it's just in the 
> autoswitch, that just need to autoswitch on the value.

There seem to be a lot of complaints about the cookies, and I don't see
nearly as much about the reader problem. So both issues should be
addressed.

Rémy



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


Re: svn commit: r638442 - /tomcat/tc6.0.x/trunk/STATUS.txt

Posted by Mark Thomas <ma...@apache.org>.
Filip Hanik - Dev Lists wrote:
> Mark Thomas wrote:
>> Filip Hanik - Dev Lists wrote:
>>> Mark Thomas wrote:
>>>> Filip Hanik - Dev Lists wrote:
>>>>> this wont work either. for the value of the cookie, it must be what 
>>>>> it was before. tspecials2. those are the correct characters for the 
>>>>> "value" of the cookie, but they are not correct for the "path"
>>>>
>>>> I'm not convinced of this yet. I am doing some more testing but I 
>>>> think things are being complicated by some potential bugs in the new 
>>>> parsing code. Now we have the test cases, I'll look at adding some 
>>>> more tests and go from there.
>>> don't think there are any bugs in the parsing code, it's just in the 
>>> autoswitch, that just need to autoswitch on the value.
>>
>> Right now it doesn't appear to parse the following header correctly:
>> Cookie: a=(
> problem is that v0 cookies don't specify what values are valid, instead 
> they recommend using some sort of encoding for characters that might 
> interfer with the HTTP spec.
> and according to 2616, ( is a separator
> 
>       token          = 1*<any CHAR except CTLs or separators>
>       separators     = "(" | ")" | "<" | ">" | "@"
>                      | "," | ";" | ":" | "\" | <">
>                      | "/" | "[" | "]" | "?" | "="
>                      | "{" | "}" | SP | HT

Thanks for that. That covers most of the parsing stuff I was seeing. The 
only exception is that we are treating '\'' as a separator but it doesn't 
appear to be in any of the specs. Not something to fix in this patch, but 
we probably should remove it from the list.

Mark

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


Re: svn commit: r638442 - /tomcat/tc6.0.x/trunk/STATUS.txt

Posted by Filip Hanik - Dev Lists <de...@hanik.com>.
Mark Thomas wrote:
> Filip Hanik - Dev Lists wrote:
>> Mark Thomas wrote:
>>> Filip Hanik - Dev Lists wrote:
>>>> this wont work either. for the value of the cookie, it must be what 
>>>> it was before. tspecials2. those are the correct characters for the 
>>>> "value" of the cookie, but they are not correct for the "path"
>>>
>>> I'm not convinced of this yet. I am doing some more testing but I 
>>> think things are being complicated by some potential bugs in the new 
>>> parsing code. Now we have the test cases, I'll look at adding some 
>>> more tests and go from there.
>> don't think there are any bugs in the parsing code, it's just in the 
>> autoswitch, that just need to autoswitch on the value.
>
> Right now it doesn't appear to parse the following header correctly:
> Cookie: a=(
problem is that v0 cookies don't specify what values are valid, instead 
they recommend using some sort of encoding for characters that might 
interfer with the HTTP spec.
and according to 2616, ( is a separator

       token          = 1*<any CHAR except CTLs or separators>
       separators     = "(" | ")" | "<" | ">" | "@"
                      | "," | ";" | ":" | "\" | <">
                      | "/" | "[" | "]" | "?" | "="
                      | "{" | "}" | SP | HT


>
> which is a valid v0 cookie. This is interfering with the testing I am 
> trying to do.
>
> I am still working my way through the RFC2965 to see why such a long 
> list of separators is being used. Any pointers appreciated.
wrong RFC, 2109 is for v1 cookies, and v0 is a small document from netscape
>
> Mark
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>
>


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


Re: svn commit: r638442 - /tomcat/tc6.0.x/trunk/STATUS.txt

Posted by Mark Thomas <ma...@apache.org>.
Filip Hanik - Dev Lists wrote:
> Mark Thomas wrote:
>> Filip Hanik - Dev Lists wrote:
>>> this wont work either. for the value of the cookie, it must be what 
>>> it was before. tspecials2. those are the correct characters for the 
>>> "value" of the cookie, but they are not correct for the "path"
>>
>> I'm not convinced of this yet. I am doing some more testing but I 
>> think things are being complicated by some potential bugs in the new 
>> parsing code. Now we have the test cases, I'll look at adding some 
>> more tests and go from there.
> don't think there are any bugs in the parsing code, it's just in the 
> autoswitch, that just need to autoswitch on the value.

Right now it doesn't appear to parse the following header correctly:
Cookie: a=(

which is a valid v0 cookie. This is interfering with the testing I am 
trying to do.

I am still working my way through the RFC2965 to see why such a long list 
of separators is being used. Any pointers appreciated.

Mark


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


Re: svn commit: r638442 - /tomcat/tc6.0.x/trunk/STATUS.txt

Posted by Filip Hanik - Dev Lists <de...@hanik.com>.
Mark Thomas wrote:
> Filip Hanik - Dev Lists wrote:
>> this wont work either. for the value of the cookie, it must be what 
>> it was before. tspecials2. those are the correct characters for the 
>> "value" of the cookie, but they are not correct for the "path"
>
> I'm not convinced of this yet. I am doing some more testing but I 
> think things are being complicated by some potential bugs in the new 
> parsing code. Now we have the test cases, I'll look at adding some 
> more tests and go from there.
don't think there are any bugs in the parsing code, it's just in the 
autoswitch, that just need to autoswitch on the value.
>
> I don't see a huge rush to fix this and I'd like to take the time to 
> understand what is going on with the parsing code and if what I think 
> are bugs are really bugs or my misunderstanding.
I do see a little bit of rush, considering we have a 6.0.16/5.5.26 with 
a broken Reader out there. The broken cookies out there are due to 
invalid cookies, but we are providing the autoswitch to mitigate users 
that don't want to, or are unable to fix the invalid v0 cookies.

Filip
>
> I'll withdraw/modify/stand by my proposal when I have done some 
> further work.
>
> Mark
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>
>


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


Re: svn commit: r638442 - /tomcat/tc6.0.x/trunk/STATUS.txt

Posted by Mark Thomas <ma...@apache.org>.
Filip Hanik - Dev Lists wrote:
> this wont work either. for the value of the cookie, it must be what it 
> was before. tspecials2. those are the correct characters for the "value" 
> of the cookie, but they are not correct for the "path"

I'm not convinced of this yet. I am doing some more testing but I think 
things are being complicated by some potential bugs in the new parsing 
code. Now we have the test cases, I'll look at adding some more tests and 
go from there.

I don't see a huge rush to fix this and I'd like to take the time to 
understand what is going on with the parsing code and if what I think are 
bugs are really bugs or my misunderstanding.

I'll withdraw/modify/stand by my proposal when I have done some further work.

Mark


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


Re: svn commit: r638442 - /tomcat/tc6.0.x/trunk/STATUS.txt

Posted by Filip Hanik - Dev Lists <de...@hanik.com>.
this wont work either. for the value of the cookie, it must be what it 
was before. tspecials2. those are the correct characters for the "value" 
of the cookie, but they are not correct for the "path"

the problem is how browsers work (or not work for that matter). for 
example, browsers will accept a quoted value for the "value", but not 
for the "path".

the safest bet, is to remove the whole version switch, and be spec 
compliant, and let the users make sure they have the correct values.

if we need to pursue the path of auto switching, which I think is not a 
good idea given the problems that come up, then switch on the value 
only. I'd leave the path and domain unescaped and unquoted for v0 
cookies, and don't even try to do anything.

http://svn.apache.org/viewvc?view=rev&revision=634513

Filip


markt@apache.org wrote:
> Author: markt
> Date: Tue Mar 18 10:04:22 2008
> New Revision: 638442
>
> URL: http://svn.apache.org/viewvc?rev=638442&view=rev
> Log:
> Withdraw previous alternative proposal and replace with an improved patch that addresses the concerns raised.
>
> Modified:
>     tomcat/tc6.0.x/trunk/STATUS.txt
>
> Modified: tomcat/tc6.0.x/trunk/STATUS.txt
> URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/STATUS.txt?rev=638442&r1=638441&r2=638442&view=diff
> ==============================================================================
> --- tomcat/tc6.0.x/trunk/STATUS.txt (original)
> +++ tomcat/tc6.0.x/trunk/STATUS.txt Tue Mar 18 10:04:22 2008
> @@ -105,10 +105,11 @@
>              elements of the cookie header.
>    -1:
>  
> -* As an alternative to the above patch, I propose:
> -  http://people.apache.org/~markt/patches/2008-03-15-cookie-path.patch
> -  +1: markt, remm, jfclere
> -  -1: maybeQuote2 should not be applied to the path, since / is a valid value without quotes
> +* As an alternative to the above patch, I propose an improved patch that
> +  addresses the concerns with the previous alternative:
> +  http://people.apache.org/~markt/patches/2008-03-18-cookie-path.patch
> +  +1: markt
> +  -1:
>  
>  * Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=44562
>    http://svn.apache.org/viewvc?rev=635294&view=rev (prior code clean up)
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>
>
>   


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