Re: Bug handling session cookies

2005-06-24 Thread Hrvoje Niksic
Mark Street [EMAIL PROTECTED] writes:

 I'm not sure why this [catering for paths without a leading /] is
 done in the code.

rfc1808 declared that the leading / is not really part of path, but
merely a separator, presumably to be consistent with its treatment
of ;params, ?queries, and #fragments.  The author of the code found it
appealing to disregard common sense and implement rfc1808 semantics.

In most cases the user shouldn't notice the difference, but it has
lead to all kinds of implementation problems with code that assumes
that URL paths naturally begin with /.  Because of that it will be
changed later.

 Note that the forward slash is stripped from prefix, hence never
 matches full_path.  I'm not sure why this is done in the code.

Because PREFIX is the path declared by the cookie, which always begins
with /, and FULL_PATH is the URL path coming from the URL parsing
code, which doesn't begin with a /.  To match them, one must indeed
strip the leading / off PREFIX.

But paths without a slash still caused subtle problems.  For example,
cookies without a path attribute still had to be stored with the
correct cookie-path (with a leading slash).  To account for this, the
invocation of cookie_handle_set_cookie was modified to prepend the /
before the path.  This lead to path_match unexpectedly receiving two
/-prefixed paths and being unable to match them.

The attached patch fixes the problem by:

* Making sure that path consistently gets prepended in all entry
  points to cookie code;

* Removing the special logic from path_match.

With that change your test case seems to work, and so do all the other
tests I could think of.

Please let me know if it works for you, and thanks for the detailed
bug report.


2005-06-24  Hrvoje Niksic  [EMAIL PROTECTED]

* http.c (gethttp): Don't prepend / here.

* cookies.c (cookie_handle_set_cookie): Prepend / to PATH.
(cookie_header): Ditto.

Index: src/http.c
===
--- src/http.c  (revision 1794)
+++ src/http.c  (working copy)
@@ -1706,7 +1706,6 @@
   /* Handle (possibly multiple instances of) the Set-Cookie header. */
   if (opt.cookies)
 {
-  char *pth = NULL;
   int scpos;
   const char *scbeg, *scend;
   /* The jar should have been created by now. */
@@ -1717,15 +1716,8 @@
   ++scpos)
{
  char *set_cookie; BOUNDED_TO_ALLOCA (scbeg, scend, set_cookie);
- if (pth == NULL)
-   {
- /* u-path doesn't begin with /, which cookies.c expects. */
- pth = (char *) alloca (1 + strlen (u-path) + 1);
- pth[0] = '/';
- strcpy (pth + 1, u-path);
-   }
- cookie_handle_set_cookie (wget_cookie_jar, u-host, u-port, pth,
-   set_cookie);
+ cookie_handle_set_cookie (wget_cookie_jar, u-host, u-port,
+   u-path, set_cookie);
}
 }
 
Index: src/cookies.c
===
--- src/cookies.c   (revision 1794)
+++ src/cookies.c   (working copy)
@@ -822,6 +822,17 @@
 {
   return path_matches (path, cookie_path) != 0;
 }
+
+/* Prepend '/' to string S.  S is copied to fresh stack-allocated
+   space and its value is modified to point to the new location.  */
+
+#define PREPEND_SLASH(s) do {  \
+  char *PS_newstr = (char *) alloca (1 + strlen (s) + 1);  \
+  *PS_newstr = '/';\
+  strcpy (PS_newstr + 1, s);   \
+  s = PS_newstr;   \
+} while (0)
+
 
 /* Process the HTTP `Set-Cookie' header.  This results in storing the
cookie or discarding a matching one, or ignoring it completely, all
@@ -835,6 +846,11 @@
   struct cookie *cookie;
   cookies_now = time (NULL);
 
+  /* Wget's paths don't begin with '/' (blame rfc1808), but cookie
+ usage assumes /-prefixed paths.  Until the rest of Wget is fixed,
+ simply prepend slash to PATH.  */
+  PREPEND_SLASH (path);
+
   cookie = parse_set_cookies (set_cookie, update_cookie_field, false);
   if (!cookie)
 goto out;
@@ -977,17 +993,8 @@
 static int
 path_matches (const char *full_path, const char *prefix)
 {
-  int len;
+  int len = strlen (prefix);
 
-  if (*prefix != '/')
-/* Wget's HTTP paths do not begin with '/' (the URL code treats it
-   as a mere separator, inspired by rfc1808), but the '/' is
-   assumed when matching against the cookie stuff.  */
-return 0;
-
-  ++prefix;
-  len = strlen (prefix);
-
   if (0 != strncmp (full_path, prefix, len))
 /* FULL_PATH doesn't begin with PREFIX. */
 return 0;
@@ -1149,6 +1156,7 @@
   int count, i, ocnt;
   char *result;
   int result_size, pos;
+  PREPEND_SLASH (path);/* see cookie_handle_set_cookie */
 
   /* First, find the cookie chains whose domains 

Re: Bug handling session cookies

2005-06-24 Thread Mark Street

Hrvoje,

Many thanks for the explanation and the patch.
Yes, this patch successfully resolves the problem for my particular test
case.

Best regards,

Mark Street.




Re: Bug handling session cookies

2005-06-24 Thread Hrvoje Niksic
Mark Street [EMAIL PROTECTED] writes:

 Many thanks for the explanation and the patch.  Yes, this patch
 successfully resolves the problem for my particular test case.

Thanks for testing it.  It has been applied to the code and will be in
Wget 1.10.1 and later.