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

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'.

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)
                 {

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. */
+          /* 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_slash = memrchr (value.b, '/', value.e - value.b);
-	  const char *last_bs = memrchr (value.b, '\\', value.e - value.b);
+          const char *last_bs = memrchr (value.b, '\\', value.e - value.b);
-	  if (last_slash && last_bs)
+          if (last_slash && last_bs)
-	    value.b = 1 + MAX (last_slash, last_bs);
+            value.b = 1 + MAX (last_slash, last_bs);
-	  else if (last_slash || last_bs)
+          else if (last_slash || last_bs)
-	    value.b = 1 + (last_slash ? last_slash : last_bs);
+            value.b = 1 + (last_slash ? last_slash : last_bs);
-	  if (value.b == value.e)
+          if (value.b == value.e)
-	    continue;
+            continue;
-	  /* Start with the directory prefix, if specified. */
+
-	  if (opt.dir_prefix)
+          if (*filename)
-	    {
+            append_value_to_filename (filename, &value);
-	      if (!(*filename))
+          else
-		{
+            *filename = strdupdelim (value.b, value.e);
-		  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);
-		}
-	    }
-	}
     }
+
   if (*filename)
-    {
+    return true;
-      return true;
-    }
   else
-    {
+    return false;
-      return false;
-    }
 }
 
 
@@ -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\"", "file.ext", true },
-    { "filename=\"file.ext\"", "somedir", "somedir/file.ext", true },
+    { "attachment; filename=\"file.ext\"", "file.ext", true },
-    { "attachment; filename=\"file.ext\"", NULL, "file.ext", true },
+    { "attachment; filename=\"file.ext\"; dummy", "file.ext", true },
-    { "attachment; filename=\"file.ext\"", "somedir", "somedir/file.ext", true },
+    { "attachment", NULL, false },
-    { "attachment; filename=\"file.ext\"; dummy", NULL, "file.ext", true },
+    { "attachement; filename*=UTF-8'en-US'hello.txt", "hello.txt", true },
-    { "attachment; filename=\"file.ext\"; dummy", "somedir", "somedir/file.ext", true },
+    { "attachement; filename*0=\"hello\"; filename*1=\"world.txt\"", "helloworld.txt", 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 },
   };
 
   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",

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 (!replaced_filename)
-  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);
+      /* Add the file name. */
-      append_uri_pathel (u_query, u_query + strlen (u_query), true, &fnres);
+      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
+    {
+      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. */

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 *);