Subject: [http-state] A question about the parser test data

From: Chris Hanson <[email-masked]>
Date: Sun, 3 Apr 2011 22:08:40 -0700

I've recently built a cookie implementation that's intended to be
(eventually) compliant with draft-23.  As part of my testing, I
gobbled up the test data from https://github.com/abarth/http-state/
and have been running my set-cookie parser on it.

I've been seeing some anomalies between what it specifies and what I
expect based on my reading of the spec.

Are these tests intended to be true for compliant implementations?  Or
are they just true for current browsers, and not necessarily for
compliant implementations?  I suspect the latter but would like
confirmation; otherwise I may need some help understanding why my
reading of the spec is incorrect.

Thanks in advance,
Chris

---

From: Adam Barth <[email-masked]>
Date: Fri, 8 Apr 2011 11:02:41 -0700

The test data isn't up to date with the final changes in the spec.  If
you'd be willing to send me the list of tests that don't match your
reading of the spec, I'll be happy to fix them.

Thanks,
Adam

On Sun, Apr 3, 2011 at 10:08 PM, Chris Hanson <[email-masked]> wrote:
> _______________________________________________
> http-state mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/http-state
>

---

From: Chris Hanson <[email-masked]>
Date: Fri, 8 Apr 2011 15:02:39 -0700

I've attached my notes about the parser tests.  I tried to give enough
detail so you can evaluate whether the data is wrong or my
understanding is wrong; let me know if you need more.

Chris

---

From: Adam Barth <[email-masked]>
Date: Fri, 8 Apr 2011 18:59:28 -0700

Thanks for these notes.  Comments below:

====
My model for the tests is as follows:

* "Set-Cookie:" headers are received in response from
  "http://home.example.org:8888/cookie-parser?TESTNAME".
  Consequently, default domain is "home.example.org" and default path
  is "/".

* "Cookie:" headers are those that would be sent back to the same URL,
  unless overridden by "Location:" header.  "Location:" header, if
  present, is relative to that URL.
====

[abarth] That's not quite right.  They're read from
"http://home.example.org:8888/cookie-parser-result?TESTNAME".  If you
run the Python script in test/run-test-server, that will actually spin
up an HTTP server you can use to test your implementation against.
You can look at run-test for examples of how to automate running all
the tests against your implementation.

====
The following all have issues with parsing of the "Set-Cookie:" header
that should give different behavior:

0021 don't expect "Cookie: x"; cookie-pair no good
NAME0017 expect no cookies; cookie-pair no good
====

[abarth] Fixed.

====
PATH0001 don't expect "Cookie: x=y"; path mismatch
PATH0002 don't expect "Cookie: a=b"; path mismatch
PATH0003 don't expect "Cookie: a=b"; path mismatch
PATH0004 don't expect "Cookie: x=y"; path mismatch
====

[abarth] These should work correctly once you interpret the final URL
as http://home.example.org:8888/cookie-parser-result?TESTNAME.

====
PATH0027 expect "Cookie: foo=bar"; explicit path should be ignored
====

[abarth] Fixed.

====
PATH0029 expect no cookies; path mismatch
====

[abarth] This should work correctly once you interpret the final URL
as http://home.example.org:8888/cookie-parser-result?TESTNAME.
Another possibility is that you're missing this step on page 22:

[[
   5.  Remove any leading or trailing WSP characters from the attribute-
       name string and the attribute-value string.
]]

which is that this test intends to test.

====
These have issues with path matching the cookie path to the URL path:

ATTRIBUTE0023 expect no cookies; path mismatch
====

[abarth] This is likely the "cookie-parser-result" issue again.

====
ORDERING0001 expect "Cookie: key=val0"; path and domain should both match
====

val0 doesn't appear in the final cookie because

Set-Cookie: key=val2; path=/

overwrites it.  Notice that val0 gets the default path (which is / in
this case).  The check for removing old cookies with the same
name/domain/path occurs after the default path gets substituted for a
missing path attribute.

====
DISABLED_DOMAIN0032 and DOMAIN0014 are a bit of a puzzle.
Domain-parsing and host normalization rules don't say anything about
trailing dots.  So, strictly speaking, both tests should send no
cookies due to a domain mismatch.  However, one is expected to send a
cookie and the other not.  Given that a trailing dot is legitimate
syntax for DNS, I would think that ignoring trailing dots would result
in behavior that's different from today's implementations.
====

I've just deleted DISABLED_DOMAIN0032.  It's disabled because it's
actually testing something other than cookies (namely, URL
canonicalization).  DOMAIN0014, however, is a more reasonable test.
The point is that trailing dots in the Domain attribute aren't
canonicalized away (unlike leading dots).

Thanks for your detailed feedback.  I've pushed the fixes to github.

Adam

---

From: Chris Hanson <[email-masked]>
Date: Fri, 8 Apr 2011 23:56:29 -0700

>  Consequently, default domain is "home.example.org" and default path

Actually, I was mistaken (and this is why the ORDERING0001 test fails
for me).  The default path is in fact "", not "/".  From page 21, list
item number 4: 'Output the characters of the uri-path from the first
character up to, but not including, the right-most %2f ("/").'

This seems like it might be an error in the spec.

> name/domain/path occurs after the default path gets substituted for a

And this isn't true because the default path is "", not "/", so the
explicit path of "/" does not overwrite the default path.

---

From: Adam Barth <[email-masked]>
Date: Sat, 9 Apr 2011 01:31:26 -0700

> This seems like it might be an error in the spec.

   1.  Let uri-path be the path portion of the request-uri if such a
       portion exists (and empty otherwise).

^^^ uri-path = /cookie-parser

   2.  If the uri-path is empty or if the first character of the uri-
       path is not a %x2F ("/") character, output %x2F ("/") and skip
       the remaining steps.

^^^ No effect.

   3.  If the uri-path contains only a single %x2F ("/") character,
       output %x2F ("/") and skip the remaining steps.

^^^ Output "/" and skip the remaining steps.

I think I understand your confusion.  You're reading the condition as
being [[if the uri-path is exactly the string "/", ...]].
Unfortunately, that's not the intent of that step.

> explicit path of "/" does not overwrite the default path.

This appears to be the same issue as above.

Adam

---

From: "Remy Lebeau" <[email-masked]>
Date: Thu, 14 Apr 2011 14:35:26 -0700

-----Original Message-----
From: [email-masked] [mailto:[email-masked]] On
Behalf Of Adam Barth
Sent: Saturday, April 09, 2011 1:31 AM
To: Chris Hanson
Cc: http-state
Subject: Re: [http-state] A question about the parser test data

> I think I understand your confusion.  You're reading the condition as
being
> [[if the uri-path is exactly the string "/", ...]]. Unfortunately, that's
not the
> intent of that step.

That is how I originally interpreted it as well.  Re-reading it again, now I
see what it is referring to.  If the uri-path starts with "/" but does not
contain another "/", ie: "/xxx", then the uri-path is referring to a
document "xxx" inside the path "/" and the document is being stripped off.

---

From: Adam Barth <[email-masked]>
Date: Thu, 14 Apr 2011 14:45:39 -0700

> document "xxx" inside the path "/" and the document is being stripped off.

I don't fully understand the RFC publication process, but we might be
able to ask for an editorial improvement in that sentence at this
point.

Adam

---

From: Peter Saint-Andre <[email-masked]>
Date: Thu, 14 Apr 2011 15:47:16 -0600

> able to ask for an editorial improvement in that sentence at this

We still have time for small editorial fixes, during "AUTH48".

Peter

-- 
Peter Saint-Andre
https://stpeter.im/

---

From: Adam Barth <[email-masked]>
Date: Thu, 14 Apr 2011 15:03:37 -0700

> We still have time for small editorial fixes, during "AUTH48".

Here's a wording that might be clearer.  Let me know if you have a
better suggestion.

@@ -741,7 +741,7 @@ time-field      = 1*2DIGIT
             uri-path is not a %x2F ("/") character, output %x2F ("/") and skip
             the remaining steps.</t>

-            <t>If the uri-path contains only a single %x2F ("/") character,
+            <t>If the uri-path contains no more than one %x2F ("/") character,
             output %x2F ("/") and skip the remaining steps.</t>

             <t>Output the characters of the uri-path from the first character

Thanks!
Adam

---

From: Peter Saint-Andre <[email-masked]>
Date: Thu, 14 Apr 2011 16:08:23 -0600

>              <t>Output the characters of the uri-path from the first character

Yes, that's the kind of fix we can make in AUTH48. :) Your proposal
seems fine with me.

Peter

-- 
Peter Saint-Andre
https://stpeter.im/

---

From: Chris Hanson <[email-masked]>
Date: Mon, 18 Apr 2011 15:54:50 -0700

Much better wording; thanks.

Here are some additional issues with the parser test data.  I missed
these in the previous round.

The following tests all parse and return cookies in which either no
equals sign is present, or the name is empty, and send back cookie:
headers with the same property.

0004
0027
CHROMIUM0009
CHROMIUM0010
CHROMIUM0012
CHROMIUM0019
MOZILLA0012
MOZILLA0014
MOZILLA0015
MOZILLA0016
MOZILLA0017
NAME0023
NAME0025
NAME0028
NAME0031
NAME0033

---

From: Adam Barth <[email-masked]>
Date: Mon, 18 Apr 2011 16:06:48 -0700

> headers with the same property.

Thanks.  This is the biggest place where the spec differs from common
browser implementations.  The working group discussed this issue for a
while and decided to ignore those sorts of cookies.  I need to update
the test suite to reflect that decision.  Will do soon.

Thanks,
Adam

---

From: Adam Barth <[email-masked]>
Date: Thu, 21 Apr 2011 15:41:26 -0700

> headers with the same property.

I've fixed the expected results for these tests to match the spec.

Thanks!
Adam