Re: lynx-dev more cookie wackiness [revisited]

[brian j pardy]

On Tue, 2 Mar 1999, Klaus Weide wrote:

> On Mon, 1 Mar 1999, brian j pardy wrote:
> 
> > [...]              After skimming the tracelog I was sent, again, in
> > an attempt to get information to send to schwab's webmaster and quote
> > from spec to complain, I noticed a few things:
> 
> I simulated a page sending the same triple of Set-cookie headers (with
> necessary modification of domain value) with a simple CGI script, traced
> lynx, and come to a different conclusion - see below.
> 
> > First, these are the headers that the schwab server is sending to
> > Lynx:
> > 
> > Server: Netscape-Enterprise/3.6^M
> > Date: Sat, 20 Feb 1999 01:19:40 GMT^M
> > Content-type: text/html^M
> > Expires: Thu, 18 Feb 1999 01:19:40 GMT^M
> > Set-cookie: CustInfo=; domain=.schwab.com; path=/; secure^M
> > Set-cookie: CookieTest=YES; domain=.schwab.com; path=/; secure^M
> > Set-cookie: SessionInfo=; domain=.schwab.com; path=/; expires=Mon, 
> > 01-Jul-1996  01:00:00 GMT; secure^M
> > Connection: close^M
> > ^M
> > 
> > When Lynx sends these off to LYCookie.c to process, we see this in
> > the tracelog:
> 
> [ I hope those bogus spaces shown below aren't really there ]

Ah, I think they're artifacts from a select/paste in screen, I didn't 
see anything strange in the tracelog itself.

> > LYSetCookie called with host 'trading41.schwab.com', path '/trading',
> >     and Set-Cookie: 'CustInfo=; domain=.schwab.com; path=/; secure,         
> >     CookieTest=YES; domain=.schwab.com; path=/; secure, SessionInfo=; 
> > domain=.      schwab.com; path=/; expires=Mon, 01-Jul-1996 01:00:00 GMT; 
> > secure'
> > LYmktime: Parsing 'Mon, 01-Jul-1996 01:00:00 GMT'
> > LYProcessSetCookie: attr=value pair: 'CookieTest=YES'
> 
> Remember what you're looking at - this is a trace from 2.8.1, not from
> recent development code.
> 
> There is a problem with 2.8.1, in that its LYCookie.c doesn't have the
> following change (in two places):
> 
>             /* if (!known_attr && value_end > value_start) */
> 
>             /* Is there any reason we don't want to accept cookies with
>              * no value?  This seems to be needed for sites that reset a
>              * cookie by nulling out the value.  If this causes problems,
>              * we can go back to the original behavior above.  - BJP
>              */
>             if (!known_attr) {
> 
> > Lynx is then discarding *ALL* of these cookies, because of that final
> > expires= line, as far as I can tell.  The multiple Set-cookie: headers
> > are being concatenated into a single string in HTTP.c:
> > 
> > HTTP.c::1153:
> > 
> >                       if (TOUPPER(*cp) != 'S') {
> >                           cp++;
> >                       } else if (!strncasecomp(cp, "Set-Cookie:", 11))  {
> >                           char *cp1 = NULL, *cp2 = NULL;
> >                           cp += 11;
> > 
> > It then goes on to make them all one big string if I'm understanding
> > this right.  
> 
> That's right, but it's all as it should be.  LYProcessSetCookies takes
> that long string and parses it into single cookies plus their attributes.
> 
> What actually happens is that in 2.8.1 SessionInfo=; is not recognized
> as the start of a new cookies.  Therefore the expires gets applied to
> the previous (second) cookie.  The first cookie is also discarded, because
> of its empty value in CustInfo=;.

Ah!  That makes much more sense to me.

> OTOH, in 2.8.2dev.recent, CustInfo=; and SessionInfo=; are recognized
> as starting new cookies, so this problem does not occur.  The first and
> third cookies still don't get saved because of the empty value, they
> are dropped later in store_cookie:
>     /*
>      * Don't add the cookie if the value is NULL. - BJP
>      */
>     } else if (co->value[0] == '\0') {
>         CTRACE(tfp, "store_cookie: Value is NULL! Not storing cookie.\n");
>         freeCookie(co);
>         co = NULL;
> 
> I have doubts that even this is right.  Is there anything in any of the
> cookie specs that says a cookie value cannot have length zero?  If there
> is nothing that says cookie values have to be non-empty, we should treat
> a value of "" as any other value, i.e. store such a cookie and send it 
> to the server in requests.

I think I added that bit in response to a different problem, and I just
may agree with you that it's wrong.  I don't remember exactly what it
was for, but since I added something doing nearly the opposite later on,
I think this is wrong.  I'm at work, will check my archives later on.

I'm not aware of anything in the specs forcing non-empty values, but
Fote had some comments in there regarding ignoring empty values, IIRC.

> > I can't see anything in Netscape's cookie spec describing this either
> > way, but I may have missed something.
> 
> Stop taking the Netscape cookie spec too serious - the RFCs / drafts are
> much more useful, and are what the Lynx cookie implementation aims at
> anyway.

RFC2109 was what I was skimming originally, then I saw that it doesn't 
mention the 'expires' attribute, so I went to Netscape's.

> Anyway, the dropping of the 2nd cookie does not occur in 2.8.2dev -
> that's a problem already solved by you, Brian, without knowing it. :) 

Heh.  That's fun to hear :)

Matt, do you have access to recompile Lynx?  If so, maybe you can try
recompiling 2.8.1+SSL with the NULL cookie patch up above:

>             /* if (!known_attr && value_end > value_start) */

This is a couple places in LYCookie.c, no source in front of me right now,
but I can find my old actual diff output later tonight if you'd rather
wait -- I'm not sure if this patch depended on anything else...

-- 
<http://www.psnw.com/~posterkid/keys/> for DSA/ElG-E/RSA keys
DSA 0x0A641AA5:0B1E 37B7 ECCB FC96 B6C6  7242 0A59 F8D5 EFA9 4F81
RSA 0x4E65C321: 42 57 B3 D2 39 8E 74 C3  5E 4D AC 43 25 D2 26 D4