Re: Jamey's HTTPUnit changes - Cookie

"Wolfgang Fahl" <[email protected]> Fri, 19 Dec 2008 09:34:19 +1300
Newsgroups gmane.comp.web.httpunit.devel
Organization BITPlan GmbH
Message-ID <[email protected]>
Hi Jamey,

Thank you for your swift reply. After reading your 
explanations and the Article:
http://de.wikipedia.org/wiki/HTTP-Cookie

The answer to your question:
> Your thoughts on proceeding?
I think it would be great to add examples to the 
httpunit Testcases that show where httpunit might not 
behave as expected.

I'v e.g. just added:
        checkAcceptance( 4, true, "www.meterware.com/servlets/special", "meterware.com", null );


under the assumption that a missing leading dot might be 
troublesome. But it runs o.k.

Please give me concrete examples of what doesn't work 
and we can go from there.

Wolfgang

Date sent:      	Wed, 17 Dec 2008 01:44:37 -0800
From:           	"[email protected]" 
<[email protected]>
To:             	[email protected]
Subject:        	Re: Jamey's HTTPUnit changes - Cookie

> Hello again Wolfgang.  I'm pretty cookie ignorant too but I did read 
> what I think were the relevant bits of RFC 2109 and RFC 2965 this 
> evening.  These clarified some items for me which I'll summarize below.
> 
> Assuming a host domain of 'a.b.c.d':
> 
> RFC 2965 is the more strict standard since it accepts only 'a.b.c.d' and 
> '.b.c.d' as matching cookie domain attributes.
> 
> RFC 2109 is more lenient in that it accepts 'a.b.c.d', '.b.c.d', '.c.d', 
> and '.d' as matching cookie domain attributes
> 
> In practice sites (like yahoo.com) commonly don't even adhere to RFC 
> 2109 and omit the leading '.' character in the cookie's domain 
> attribute.  Thus, in order to work with these sites we'd have to allow, 
> in addition to what RFC 2109 allows, 'b.d.c', 'c.d', and 'd' to match
> 
> There are two places in the HTTPUnit code which deal with this logic.  
> Neither handles all the cases above, and they aren't consistent with one 
> another.
> 
> The first is the com.meterware.httpunit.cookies.Cookie.acceptHost method 
> which was the original target of my change.
> This method appears to behave in accordance with RFC 2109 but not 2965 
> as it does not check the value of 
> CookieProperties.isDomainMatchingStrict.  It also does not handle the 
> "yahoo case" of the lack of leading '.' on the domain attribute of the 
> cookie.
> 
> The second location in which this logic is implemented is 
> com.meterware.httpunit.cookies.CookieJar.CookiePress.getDomainAttributeStatus.  
> This method is coded to accept the Yahoo case by prepending a '.' if 
> missing and otherwise correctly adheres to RFC 2109 or 2965 depending on 
> the value of CookieProperties.isDomainMatchingStrict which currently 
> defaults to true.
> 
> Since the behavior of these two methods is currently inconsistent I'm 
> not sure how to correct the code.  My desire would be to remove the 
> duplicate implementation and simply have acceptHost depend on 
> getDomainAttributeStatus ensuring consistency.  The 
> CookieProperties.isDomainMatching strict would continue to toggle 
> between RFC 2965 behavior when true and RFC 2109 behavior when false.  
> An additional CookieProperites.isDomainMatchUndottedSuffix or similar 
> method could be added to, if true, instruct the code to tolerate the 
> Yahoo case above.
> 
> At this point I retract my proposed patch to Cookie.acceptHost as I 
> believe it does not properly capture the intended behavior described in 
> the RFCs and it doesn't address the inconsistent secondary 
> implementation of this logic in the HTTPUnit codebase.
> 
> Your thoughts on proceeding?
> 
> Jamey
> 
> 
> Wolfgang Fahl wrote:
> > Jamey,
> >
> > your suggestions sound logical. AcceptHost is a private method and 
> > never tested directly. mayBeSentToURL is the one being use in 
> > CookieTest.java and there it is sometimes only used indirectly like in 
> > the checkMatching method.
> >
> > Unfortunately I have absolutely no knowledge about Cookies and the 
> > rules for them. For me the only viable starting point of discussion is 
> > concrete examples (test cases) where httpunit fails. From what you are 
> > writing below the isDomainMatchingStrict and the upper/lowercase issue 
> > will probably lead to examples where it can be proven that httpunit 
> > does not behave as excpected by a URL and isDomainMatchingStrict 
> > setting that does not comply to the rules (which I don't know).
> >
> > The testLenienetMatching  test case might be a good starting point:
> >     *public*   *void*  testLenientMatching() *throws*  Exception {
> >         CookieProperties./setDomainMatchingStrict/( *false*  );
> >         checkAcceptance( 1, *true* , 
> > "www.some.meterware.com/servlets/special" , ".meterware.com" , *null*  );
> >         checkAcceptance( 2, *false* , 
> > "www.meterware.com/servlets/special" , ".meterware.com" , 
> > "/servlets/ordinary"  );
> >         checkAcceptance( 3, *true* , 
> > "www.meterware.com/servlets/special" , "www.meterware.com" , *null*  );
> >
> >         CookieProperties./setPathMatchingStrict/( *false*  );
> >         checkAcceptance( 11, *true* , 
> > "www.meterware.com/servlets/special" , ".meterware.com" , 
> > "/servlets/ordinary"  );
> >         checkMatching( 12, *true* , *new*  URL( 
> > "http://www.meterware.com/servlets/sample"  ), "www.meterware.com" , 
> > "/servlets/sample/data"  );
> >     }
> >
> >
> > Would you please add some examples that fail with the current 
> > Subversion trunk and won't fail after the patch below?
> >
> > Thank you
> >
> >
> > Wolfgang
> >
> > Date sent:       Sun, 09 Nov 2008 16:13:34 -0800
> > From:            "[email protected]" <[email protected]>
> > To:              [email protected]
> > Subject:         Jamey's HTTPUnit changes - Cookie
> >
> > > Hey again Wolfgang.  Here's the first change for discussion.  It's the
> > > simpler one regarding cookies so please let me know if you have concerns
> > > about applying this patch.  I wanted to treat it separately from the
> > > recursion issue as it is a separate issue and I don't want this fix
> > > blocked because of the recursion issue.  I don't really have any
> > > comments regarding this issue beyond my original statements copied
> > > below.  I do think that when/if you apply the patch you should consider
> > > whether the acceptPath method (which I've not modified in my patch)
> > > should ignore case in its comparison and whether or not it should also
> > > be declared static asd acceptHost is.
> > >
> > > Thanks!
> > >
> > > Jamey
> > >
> > >
> > > ******************************
> > >
> > > My original issue:
> > >
> > > ******************************
> > >
> > > com.meterware.httpunit.cookies.Cookie:
> > >
> > > Line 250 in the acceptHost method uses the following logic when 
> > validating :
> > >
> > >          return hostPattern.equalsIgnoreCase( hostName ) ||
> > >                 (hostPattern.startsWith( "." ) && hostName.endsWith( 
> > hostPattern ));
> > >
> > > This logic seems to have a couple of problems:
> > > 1) In the second part of the comparison case is NOT ignored.
> > > 2) No consideration is given to the 
> > CookieProperties.isDomainMatchingStrict setting
> > >     - This seems odd since CookieProperties.isPathMatchingStrict is 
> > considered
> > >     in the acceptPath method a few lines earlier and is required to 
> > handle certain
> > >       less than compliant web sites (groups.yahoo.com for example).
> > > 3) Should the acceptPath method maybe be static as the acceptHost 
> > method is?
> > >
> > > ******************************
> > > My proposed patch
> > >
> > > ******************************
> > >
> > > @@ -247,8 +247,9 @@
> > >       * @return true if there is a fit
> > >       */
> > >      private static boolean acceptHost( String hostPattern, String 
> > hostName ) {
> > > -        return hostPattern.equalsIgnoreCase( hostName ) ||
> > > -               (hostPattern.startsWith( "." ) && hostName.endsWith( 
> > hostPattern ));
> > > +        return !CookieProperties.isDomainMatchingStrict() ||
> > > +               hostPattern.equalsIgnoreCase( hostName ) ||
> > > +            (hostPattern.startsWith( "." ) && 
> > hostName.toLowerCase().endsWith(hostPattern.toLowerCase()));
> > >      }
> > >
> > >
> > >
> >
> >
> > *
> > *
> > *BITPlan* - smart solutions
> > Pater-Delp-Str. 1, D-47877 Willich Schiefbahn
> > Tel. +49 2154 811-480, Fax +49 2154 811-481
> > Web: http://www.bitplan.de
> > bitplan GmbH, Willich - HRB 6820 Krefeld, VAT-ID: 10258040548, 
> > Geschäftsführer: Wolfgang Fahl
> >  
> 
> 



BITPlan - smart solutions
Pater-Delp-Str. 1, D-47877 Willich Schiefbahn
Tel. +49 2154 811-480, Fax +49 2154 811-481
Web: http://www.bitplan.de
bitplan GmbH, Willich - HRB 6820 Krefeld, VAT-ID: 10258040548, 
Geschäftsführer: Wolfgang Fahl

------------------------------------------------------------------------------

_______________________________________________
Httpunit-develop mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/httpunit-develop