]> git.ipfire.org Git - thirdparty/curl.git/commitdiff
tool_cb_hdr: with -J, use the redirect name as a backup
authorDaniel Stenberg <daniel@haxx.se>
Sun, 25 Jan 2026 15:35:53 +0000 (16:35 +0100)
committerDaniel Stenberg <daniel@haxx.se>
Mon, 26 Jan 2026 11:53:03 +0000 (12:53 +0100)
The -J / --remote-header-name logic now records the file name part used
in the redirects so that it can use the last one as a name if no
Content-Disposition header arrives.

Add tests to verify:

1641: -J with a redirect and extract the CD contents in the second
response

1642: -J with a redirect but no Content-Disposition, use the name from
the Location: header

1643: -J with two redirects, using the last file name and also use
queries and fragments to verify them stripped off

Closes #20430

docs/TODO.md
docs/cmdline-opts/remote-header-name.md
src/tool_cb_hdr.c
src/tool_sdecls.h
tests/data/Makefile.am
tests/data/test1641 [new file with mode: 0644]
tests/data/test1642 [new file with mode: 0644]
tests/data/test1643 [new file with mode: 0644]

index 7c7c065baf5aaf49b1edd05f5125af3747a88119..6946f453e0a96a2ae8dc745445e7dbc39a6dec33 100644 (file)
@@ -819,25 +819,6 @@ already transferred before the retry.
 
 See [curl issue 1084](https://github.com/curl/curl/issues/1084)
 
-## consider filename from the redirected URL with `-O` ?
-
-When a user gives a URL and uses `-O`, and curl follows a redirect to a new
-URL, the filename is not extracted and used from the newly redirected-to URL
-even if the new URL may have a much more sensible filename.
-
-This is clearly documented and helps for security since there is no surprise
-to users which filename that might get overwritten, but maybe a new option
-could allow for this or maybe `-J` should imply such a treatment as well as
-`-J` already allows for the server to decide what filename to use so it
-already provides the "may overwrite any file" risk.
-
-This is extra tricky if the original URL has no filename part at all since
-then the current code path does error out with an error message, and we cannot
-*know* already at that point if curl is redirected to a URL that has a
-filename...
-
-See [curl issue 1241](https://github.com/curl/curl/issues/1241)
-
 ## retry on network is unreachable
 
 The `--retry` option retries transfers on *transient failures*. We later added
index 88c2808a3fecd6b4cce05b590e3ff4ea3fbb5764..52ae98b01cffbe63a4357604ee20efbf5e17a3df 100644 (file)
@@ -34,6 +34,9 @@ this option may provide you with rather unexpected filenames.
 This feature uses the name from the `filename` field, it does not yet support
 the `filename*` field (filenames with explicit character sets).
 
+Starting in 8.19.0, curl falls back and uses the filename extracted from the
+last redirect header if no `Content-Disposition:` header provides a filename.
+
 **WARNING**: Exercise judicious use of this option, especially on Windows. A
 rogue server could send you the name of a DLL or other file that could be
 loaded automatically by Windows or some third party software.
index 876a599049971f98761afdf2633b275e1c99f324..eb35c5c383937b39d9b447cb7ce2444b62b7e602 100644 (file)
@@ -148,30 +148,40 @@ locdone:
 /*
  * Copies a filename part and returns an ALLOCATED data buffer.
  */
-static char *parse_filename(const char *ptr, size_t len)
+static char *parse_filename(const char *ptr, size_t len, char stop)
 {
   char *copy;
   char *p;
   char *q;
-  char stop = '\0';
 
   copy = memdup0(ptr, len);
   if(!copy)
     return NULL;
 
   p = copy;
-  if(*p == '\'' || *p == '"') {
-    /* store the starting quote */
-    stop = *p;
-    p++;
-  }
-  else
-    stop = ';';
+  if(stop) {
+    /* a Content-Disposition: header */
+    if(*p == '\'' || *p == '"') {
+      /* store the starting quote */
+      stop = *p;
+      p++;
+    }
 
-  /* scan for the end letter and stop there */
-  q = strchr(p, stop);
-  if(q)
-    *q = '\0';
+    /* scan for the end letter and stop there */
+    q = strchr(p, stop);
+    if(q)
+      *q = '\0';
+  }
+  else {
+    /* this is a Location: header, so we need to trim off any queries and
+       fragments present */
+    q = strchr(p, '?'); /* trim off query, if present */
+    if(q)
+      *q = '\0';
+    q = strchr(p, '#'); /* trim off fragment, if present */
+    if(q)
+      *q = '\0';
+  }
 
   /* if the filename contains a path, only use filename portion */
   q = strrchr(p, '/');
@@ -283,12 +293,44 @@ static size_t save_etag(const char *etag_h, const char *endp,
  * Content-Disposition header specifying a filename property.
  */
 static size_t content_disposition(const char *str, const char *end,
-                                  size_t cb, struct per_transfer *per)
+                                  size_t cb, struct per_transfer *per,
+                                  long response) /* response code */
 {
   struct HdrCbData *hdrcbdata = &per->hdrcbdata;
   struct OutStruct *outs = &per->outs;
 
-  if((cb > 20) && checkprefix("Content-disposition:", str)) {
+  if((cb > 9) && checkprefix("Location:", str) && (response/100 == 3)) {
+    /* Get the name off the location header as a temporary measure in case
+       there is no Content-Disposition */
+    const char *p = &str[9];
+    curlx_str_passblanks(&p);
+    if(p < end) { /* as a precaution */
+      char *filename = parse_filename(p, cb - (p - str), 0);
+      if(filename) {
+        if(outs->stream) {
+          /* indication of problem, get out! */
+          curlx_free(filename);
+          return CURL_WRITEFUNC_ERROR;
+        }
+        if(outs->alloc_filename)
+          curlx_free(outs->filename);
+
+        if(per->config->output_dir) {
+          outs->filename = curl_maprintf("%s/%s", per->config->output_dir,
+                                         filename);
+          curlx_free(filename);
+          if(!outs->filename)
+            return CURL_WRITEFUNC_ERROR;
+        }
+        else
+          outs->filename = filename;
+        outs->alloc_filename = TRUE;
+        outs->is_cd_filename = TRUE; /* set to avoid clobbering existing files
+                                        by default */
+      }
+    }
+  }
+  else if((cb > 20) && checkprefix("Content-disposition:", str)) {
     const char *p = str + 20;
     /* look for the 'filename=' parameter (encoded filenames (*=) are not
        supported) */
@@ -312,14 +354,17 @@ static size_t content_disposition(const char *str, const char *end,
       }
       p += 9;
 
+      curlx_str_passblanks(&p);
       len = cb - (size_t)(p - str);
-      filename = parse_filename(p, len);
+      filename = parse_filename(p, len, ';');
       if(filename) {
         if(outs->stream) {
           /* indication of problem, get out! */
           curlx_free(filename);
           return CURL_WRITEFUNC_ERROR;
         }
+        if(outs->alloc_filename)
+          curlx_free(outs->filename);
 
         if(per->config->output_dir) {
           outs->filename = curl_maprintf("%s/%s", per->config->output_dir,
@@ -440,13 +485,13 @@ size_t tool_header_cb(char *ptr, size_t size, size_t nmemb, void *userdata)
         return rc;
     }
 
-    /* Parse the content-disposition header. When honor_cd_filename is true
-       other headers may be stored until the content-disposition header is
-       reached, at which point the saved headers can be written. That means
-       the content_disposition() may return an rc when it has saved a
-       different header for writing later. */
+    /* Parse the content-disposition and location headers. When
+       honor_cd_filename is true, other headers may be stored until the
+       content-disposition header is reached, at which point the saved headers
+       can be written. That means the content_disposition() may return an rc
+       when it has saved a different header for writing later. */
     else if(hdrcbdata->honor_cd_filename) {
-      size_t rc = content_disposition(str, end, cb, per);
+      size_t rc = content_disposition(str, end, cb, per, response);
       if(rc)
         return rc;
     }
index abee355d170dd741e1326320776a80a55bfa9709..9058233a688e183df4edc2d69812d513ebdd4c5a 100644 (file)
@@ -36,7 +36,8 @@
  * dynamically allocated and 'belongs' to this OutStruct, otherwise FALSE.
  *
  * 'is_cd_filename' member is TRUE when string pointed by 'filename' has been
- * set using a server-specified Content-Disposition filename, otherwise FALSE.
+ * set using a server-specified Content-Disposition or Location filename,
+ * otherwise FALSE.
  *
  * 'regular_file' member is TRUE when output goes to a regular file, this also
  * implies that output is 'seekable' and 'appendable' and also that member
index 7de9f5f5958ca7bbc0e3ef63ed3f90c941c0eccf..d7115003bfaaa8da56bd7bcb04d2058c12829a50 100644 (file)
@@ -218,7 +218,7 @@ test1620 test1621 test1622 test1623 \
 \
 test1630 test1631 test1632 test1633 test1634 test1635 test1636 \
 \
-test1640 \
+test1640 test1641 test1642 test1643 \
 \
 test1650 test1651 test1652 test1653 test1654 test1655 test1656 test1657 \
 test1658 \
diff --git a/tests/data/test1641 b/tests/data/test1641
new file mode 100644 (file)
index 0000000..a71a59b
--- /dev/null
@@ -0,0 +1,62 @@
+<?xml version="1.0" encoding="US-ASCII"?>
+<testcase>
+<info>
+<keywords>
+HTTP
+HTTP GET
+-J
+</keywords>
+</info>
+
+<reply>
+<data nocheck="yes">
+HTTP/1.1 301 OK
+Location: %TESTNUMBER0002
+
+</data>
+<data2 nocheck="yes">
+HTTP/1.1 200 OK
+Date: Tue, 09 Nov 2010 14:49:00 GMT
+Server: test-server/fake
+Content-Length: 6
+Connection: close
+Content-Type: text/html
+Content-Disposition: filename=name%TESTNUMBER; charset=funny; option=strange
+
+12345
+</data2>
+</reply>
+
+# Client-side
+<client>
+<server>
+http
+</server>
+<name>
+HTTP GET with -J, a redirect and Content-Disposition in the second response
+</name>
+<command option="no-output,no-include">
+http://%HOSTIP:%HTTPPORT/%TESTNUMBER -J -L -O --output-dir %LOGDIR
+</command>
+</client>
+
+# Verify data after the test has been "shot"
+<verify>
+<protocol crlf="headers">
+GET /%TESTNUMBER HTTP/1.1
+Host: %HOSTIP:%HTTPPORT
+User-Agent: curl/%VERSION
+Accept: */*
+
+GET /%TESTNUMBER0002 HTTP/1.1
+Host: %HOSTIP:%HTTPPORT
+User-Agent: curl/%VERSION
+Accept: */*
+
+</protocol>
+<file name="%LOGDIR/name%TESTNUMBER">
+12345
+</file>
+
+</verify>
+</testcase>
diff --git a/tests/data/test1642 b/tests/data/test1642
new file mode 100644 (file)
index 0000000..81f0ae8
--- /dev/null
@@ -0,0 +1,61 @@
+<?xml version="1.0" encoding="US-ASCII"?>
+<testcase>
+<info>
+<keywords>
+HTTP
+HTTP GET
+-J
+</keywords>
+</info>
+
+<reply>
+<data nocheck="yes">
+HTTP/1.1 301 OK
+Location: go/here/%TESTNUMBER0002
+
+</data>
+<data2 nocheck="yes">
+HTTP/1.1 200 OK
+Date: Tue, 09 Nov 2010 14:49:00 GMT
+Server: test-server/fake
+Content-Length: 6
+Connection: close
+Content-Type: text/html
+
+12345
+</data2>
+</reply>
+
+# Client-side
+<client>
+<server>
+http
+</server>
+<name>
+HTTP GET with -J, redirect, no Content-Disposition use the Location: name
+</name>
+<command option="no-output,no-include">
+http://%HOSTIP:%HTTPPORT/%TESTNUMBER -J -L -O --output-dir %LOGDIR
+</command>
+</client>
+
+# Verify data after the test has been "shot"
+<verify>
+<protocol crlf="headers">
+GET /%TESTNUMBER HTTP/1.1
+Host: %HOSTIP:%HTTPPORT
+User-Agent: curl/%VERSION
+Accept: */*
+
+GET /go/here/%TESTNUMBER0002 HTTP/1.1
+Host: %HOSTIP:%HTTPPORT
+User-Agent: curl/%VERSION
+Accept: */*
+
+</protocol>
+<file name="%LOGDIR/%TESTNUMBER0002">
+12345
+</file>
+
+</verify>
+</testcase>
diff --git a/tests/data/test1643 b/tests/data/test1643
new file mode 100644 (file)
index 0000000..17c3683
--- /dev/null
@@ -0,0 +1,71 @@
+<?xml version="1.0" encoding="US-ASCII"?>
+<testcase>
+<info>
+<keywords>
+HTTP
+HTTP GET
+-J
+</keywords>
+</info>
+
+<reply>
+<data nocheck="yes">
+HTTP/1.1 301 OK
+Location: go/here/%TESTNUMBER0002#first
+
+</data>
+<data2 nocheck="yes">
+HTTP/1.1 301 OK
+Location: too/%TESTNUMBER0003?fooo#second/%TESTNUMBER0003
+
+</data2>
+<data3 nocheck="yes">
+HTTP/1.1 200 OK
+Date: Tue, 09 Nov 2010 14:49:00 GMT
+Server: test-server/fake
+Content-Length: 6
+Connection: close
+Content-Type: text/html
+
+12345
+</data3>
+</reply>
+
+# Client-side
+<client>
+<server>
+http
+</server>
+<name>
+HTTP -J, two redirects, use the last Location: name
+</name>
+<command option="no-output,no-include">
+http://%HOSTIP:%HTTPPORT/%TESTNUMBER -J -L -O --output-dir %LOGDIR
+</command>
+</client>
+
+# Verify data after the test has been "shot"
+<verify>
+<protocol crlf="headers">
+GET /%TESTNUMBER HTTP/1.1
+Host: %HOSTIP:%HTTPPORT
+User-Agent: curl/%VERSION
+Accept: */*
+
+GET /go/here/%TESTNUMBER0002 HTTP/1.1
+Host: %HOSTIP:%HTTPPORT
+User-Agent: curl/%VERSION
+Accept: */*
+
+GET /go/here/too/%TESTNUMBER0003?fooo HTTP/1.1
+Host: %HOSTIP:%HTTPPORT
+User-Agent: curl/%VERSION
+Accept: */*
+
+</protocol>
+<file name="%LOGDIR/%TESTNUMBER0003">
+12345
+</file>
+
+</verify>
+</testcase>