Fix problem when content-disposition is used with recursive downloading.

diff --git a/src/ChangeLog b/src/ChangeLog
index 55b2be8fb5c669994f546a9a80390c9c0cdac6e0..0e244f8ffbf3af327ecca5964616f49900b679f7 100644 (file)
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,16 @@
+2010-09-24  Dennis, CHENG Renquan <crquan@fedoraproject.org>
+
+       Fix problem when content-disposition is used with recursive downloading.
+       * url.h (url_file_name): Add a new argument `replaced_filename'.
+       * url.c (url_file_name): Likewise.
+       * http.c (parse_content_disposition): Do not add a prefix to the return
+       value.
+       (test_parse_content_disposition): Adjust tests.
+       (gethttp): Pass additional parameter to `url_file_name'.
+       (http_loop): Likewise.
+       * ftp.c (ftp_loop_internal, ftp_get_listing, ftp_retrieve_list)
+       (ftp_loop): Likewise.
+
 2010-09-14  Giuseppe Scrivano  <gscrivano@gnu.org>
 
        * convert.c (local_quote_string): Accept new parameter `no_html_quote'.

diff --git a/src/ftp.c b/src/ftp.c
index 9395369065f7a0f0e71105d2e915a8c531a38a59..d3f6b18ba36eacd62e6c5fec908a74902f7da524 100644 (file)
--- a/src/ftp.c
+++ b/src/ftp.c
@@ -1382,7 +1382,7 @@ ftp_loop_internal (struct url *u, struct fileinfo *f, ccon *con, char **local_fi
   else
     {
       /* URL-derived file.  Consider "-O file" name. */
-      con->target = url_file_name (u);
+      con->target = url_file_name (u, NULL);
       if (!opt.output_document)
         locf = con->target;
       else
@@ -1496,7 +1496,7 @@ ftp_loop_internal (struct url *u, struct fileinfo *f, ccon *con, char **local_fi
             {
               /* Re-determine the file name. */
               xfree_null (con->target);
-              con->target = url_file_name (u);
+              con->target = url_file_name (u, NULL);
               locf = con->target;
             }
           continue;
@@ -1624,7 +1624,7 @@ ftp_get_listing (struct url *u, ccon *con, struct fileinfo **f)
   /* Find the listing file name.  We do it by taking the file name of
      the URL and replacing the last component with the listing file
      name.  */
-  uf = url_file_name (u);
+  uf = url_file_name (u, NULL);
   lf = file_merge (uf, LIST_FILENAME);
   xfree (uf);
   DEBUGP ((_("Using %s as listing tmp file.\n"), quote (lf)));
@@ -1718,7 +1718,7 @@ ftp_retrieve_list (struct url *u, struct fileinfo *f, ccon *con)
       ofile = xstrdup (u->file);
       url_set_file (u, f->name);
 
-      con->target = url_file_name (u);
+      con->target = url_file_name (u, NULL);
       err = RETROK;
 
       dlthis = true;
@@ -2168,7 +2168,7 @@ ftp_loop (struct url *u, char **local_file, int *dt, struct url *proxy,
               char *filename = (opt.output_document
                                 ? xstrdup (opt.output_document)
                                 : (con.target ? xstrdup (con.target)
-                                   : url_file_name (u)));
+                                   : url_file_name (u, NULL)));
               res = ftp_index (filename, u, f);
               if (res == FTPOK && opt.verbose)
                 {

diff --git a/src/http.c b/src/http.c
index 496e64a73dc5f976c3507a4c60eef6bcaac86fe9..0b1d644bb6e3803ef70086bbd77780855a53a715 100644 (file)
--- a/src/http.c
+++ b/src/http.c
@@ -1149,71 +1149,44 @@ append_value_to_filename (char **filename, param_token const * const value)
    false.
 
    The file name is stripped of directory components and must not be
-   empty.  */
+   empty.
+
+   Historically, this function returned filename prefixed with opt.dir_prefix,
+   now that logic is handled by the caller, new code should pay attention,
+   changed by crq, Sep 2010.
+
+*/
 static bool
 parse_content_disposition (const char *hdr, char **filename)
 {
-  *filename = NULL;
   param_token name, value;
+  *filename = NULL;
   while (extract_param (&hdr, &name, &value, ';'))
     {
       int isFilename = BOUNDED_EQUAL_NO_CASE ( name.b, name.e, "filename" );
       if ( isFilename && value.b != NULL)
-       {
-         /* Make the file name begin at the last slash or backslash. */
-         const char *last_slash = memrchr (value.b, '/', value.e - value.b);
-         const char *last_bs = memrchr (value.b, '\\', value.e - value.b);
-         if (last_slash && last_bs)
-           value.b = 1 + MAX (last_slash, last_bs);
-         else if (last_slash || last_bs)
-           value.b = 1 + (last_slash ? last_slash : last_bs);
-         if (value.b == value.e)
-           continue;
-         /* Start with the directory prefix, if specified. */
-         if (opt.dir_prefix)
-           {
-             if (!(*filename))
-               {
-                 int prefix_length = strlen (opt.dir_prefix);
-                 bool add_slash = (opt.dir_prefix[prefix_length - 1] != '/');
-                 int total_length;
-                 
-                 if (add_slash) 
-                   ++prefix_length;
-                 total_length = prefix_length + (value.e - value.b);            
-                 *filename = xmalloc (total_length + 1);
-                 strcpy (*filename, opt.dir_prefix);
-                 if (add_slash) 
-                   (*filename)[prefix_length - 1] = '/';
-                 memcpy (*filename + prefix_length, value.b, (value.e - value.b));
-                 (*filename)[total_length] = '\0';
-               }
-             else
-               {
-                 append_value_to_filename (filename, &value);
-               }  
-           }
-         else
-           {
-             if (*filename)
-               {
-                 append_value_to_filename (filename, &value);
-               }
-             else
-               {
-                 *filename = strdupdelim (value.b, value.e);
-               }
-           }
-       }
+        {
+          /* Make the file name begin at the last slash or backslash. */
+          const char *last_slash = memrchr (value.b, '/', value.e - value.b);
+          const char *last_bs = memrchr (value.b, '\\', value.e - value.b);
+          if (last_slash && last_bs)
+            value.b = 1 + MAX (last_slash, last_bs);
+          else if (last_slash || last_bs)
+            value.b = 1 + (last_slash ? last_slash : last_bs);
+          if (value.b == value.e)
+            continue;
+
+          if (*filename)
+            append_value_to_filename (filename, &value);
+          else
+            *filename = strdupdelim (value.b, value.e);
+        }
     }
+
   if (*filename)
-    {
-      return true;
-    }
+    return true;
   else
-    {
-      return false;
-    }
+    return false;
 }
 
 \f
@@ -2163,15 +2136,23 @@ read_header:
    * hstat.local_file is set by http_loop to the argument of -O. */
   if (!hs->local_file)
     {
+      char *local_file = NULL;
+
       /* Honor Content-Disposition whether possible. */
       if (!opt.content_disposition
           || !resp_header_copy (resp, "Content-Disposition",
                                 hdrval, sizeof (hdrval))
-          || !parse_content_disposition (hdrval, &hs->local_file))
+          || !parse_content_disposition (hdrval, &local_file))
         {
           /* The Content-Disposition header is missing or broken.
            * Choose unique file name according to given URL. */
-          hs->local_file = url_file_name (u);
+          hs->local_file = url_file_name (u, NULL);
+        }
+      else
+        {
+          DEBUGP (("Parsed filename from Content-Disposition: %s\n",
+                  local_file));
+          hs->local_file = url_file_name (u, local_file);
         }
     }
 
@@ -2647,7 +2628,7 @@ http_loop (struct url *u, struct url *original_url, char **newloc,
   else if (!opt.content_disposition)
     {
       hstat.local_file =
-        url_file_name (opt.trustservernames ? u : original_url);
+        url_file_name (opt.trustservernames ? u : original_url, NULL);
       got_name = true;
     }
 
@@ -2685,7 +2666,7 @@ File %s already there; not retrieving.\n\n"),
 
   /* Send preliminary HEAD request if -N is given and we have an existing
    * destination file. */
-  file_name = url_file_name (opt.trustservernames ? u : original_url);
+  file_name = url_file_name (opt.trustservernames ? u : original_url, NULL);
   if (opt.timestamping && (file_exists_p (file_name)
                            || opt.content_disposition))
     send_head_first = true;
@@ -3549,20 +3530,15 @@ test_parse_content_disposition()
   int i;
   struct {
     char *hdrval;
-    char *opt_dir_prefix;
     char *filename;
     bool result;
   } test_array[] = {
-    { "filename=\"file.ext\"", NULL, "file.ext", true },
-    { "filename=\"file.ext\"", "somedir", "somedir/file.ext", true },
-    { "attachment; filename=\"file.ext\"", NULL, "file.ext", true },
-    { "attachment; filename=\"file.ext\"", "somedir", "somedir/file.ext", true },
-    { "attachment; filename=\"file.ext\"; dummy", NULL, "file.ext", true },
-    { "attachment; filename=\"file.ext\"; dummy", "somedir", "somedir/file.ext", true },
-    { "attachment", NULL, NULL, false },
-    { "attachment", "somedir", NULL, false },
-    { "attachement; filename*=UTF-8'en-US'hello.txt", NULL, "hello.txt", true },
-    { "attachement; filename*0=\"hello\"; filename*1=\"world.txt\"", NULL, "helloworld.txt", true },
+    { "filename=\"file.ext\"", "file.ext", true },
+    { "attachment; filename=\"file.ext\"", "file.ext", true },
+    { "attachment; filename=\"file.ext\"; dummy", "file.ext", true },
+    { "attachment", NULL, false },
+    { "attachement; filename*=UTF-8'en-US'hello.txt", "hello.txt", true },
+    { "attachement; filename*0=\"hello\"; filename*1=\"world.txt\"", "helloworld.txt", true },
   };
 
   for (i = 0; i < sizeof(test_array)/sizeof(test_array[0]); ++i)
@@ -3570,7 +3546,6 @@ test_parse_content_disposition()
       char *filename;
       bool res;
 
-      opt.dir_prefix = test_array[i].opt_dir_prefix;
       res = parse_content_disposition (test_array[i].hdrval, &filename);
 
       mu_assert ("test_parse_content_disposition: wrong result",

diff --git a/src/url.c b/src/url.c
index 9135971ddbec368ae0a3e5a819fc20db45715cf5..bcdf750bf3ebc0e32cc18fa2d1b312f0e393037e 100644 (file)
--- a/src/url.c
+++ b/src/url.c
@@ -1499,7 +1499,7 @@ append_dir_structure (const struct url *u, struct growable *dest)
    possible.  Does not create directories on the file system.  */
 
 char *
-url_file_name (const struct url *u)
+url_file_name (const struct url *u, char *replaced_filename)
 {
   struct growable fnres;        /* stands for "file name result" */
 
@@ -1554,18 +1554,29 @@ url_file_name (const struct url *u)
       append_dir_structure (u, &fnres);
     }
 
-  /* Add the file name. */
-  if (fnres.tail)
-    append_char ('/', &fnres);
-  u_file = *u->file ? u->file : index_filename;
-  append_uri_pathel (u_file, u_file + strlen (u_file), false, &fnres);
-
-  /* Append "?query" to the file name. */
-  u_query = u->query && *u->query ? u->query : NULL;
-  if (u_query)
+  if (!replaced_filename)
+    {
+      /* Add the file name. */
+      if (fnres.tail)
+       append_char ('/', &fnres);
+      u_file = *u->file ? u->file : index_filename;
+      append_uri_pathel (u_file, u_file + strlen (u_file), false, &fnres);
+
+      /* Append "?query" to the file name. */
+      u_query = u->query && *u->query ? u->query : NULL;
+      if (u_query)
+       {
+         append_char (FN_QUERY_SEP, &fnres);
+         append_uri_pathel (u_query, u_query + strlen (u_query),
+                            true, &fnres);
+       }
+    }
+  else
     {
-      append_char (FN_QUERY_SEP, &fnres);
-      append_uri_pathel (u_query, u_query + strlen (u_query), true, &fnres);
+      if (fnres.tail)
+       append_char ('/', &fnres);
+      u_file = replaced_filename;
+      append_uri_pathel (u_file, u_file + strlen (u_file), false, &fnres);
     }
 
   /* Zero-terminate the file name. */

diff --git a/src/url.h b/src/url.h
index c9590f51ed501ab2b21e5c2be8a64665252f55c9..7e91ffa3f5ab1d144b5b4aa44ecaf3b416bbe42c 100644 (file)
--- a/src/url.h
+++ b/src/url.h
@@ -99,7 +99,7 @@ int scheme_default_port (enum url_scheme);
 void scheme_disable (enum url_scheme);
 
 char *url_string (const struct url *, enum url_auth_mode);
-char *url_file_name (const struct url *);
+char *url_file_name (const struct url *, char *);
 
 char *uri_merge (const char *, const char *);