--remote-header-name doesn't work when a scheme is missing from a URL #760
Comments
nodakai
changed the title
|
I don't think it is related to the missing scheme, I think it is the redirect that causes it. Hm, but why does the scheme-using one work then... ? |
|
We don't honor the content disposition filename unless the scheme is explicit and http or https. How about we relax it a little: diff --git a/src/tool_operate.c b/src/tool_operate.c
index c8bf12b..94a9658 100644
--- a/src/tool_operate.c
+++ b/src/tool_operate.c
@@ -1290,7 +1290,10 @@ static CURLcode operate_do(struct GlobalConfig *global,
if(config->content_disposition
&& (urlnode->flags & GETOUT_USEREMOTE)
&& (checkprefix("http://", this_url) ||
- checkprefix("https://", this_url)))
+ checkprefix("https://", this_url) ||
+ (!strstr(this_url, "://") &&
+ (!config->proto_default ||
+ checkprefix("http", config->proto_default)))))
hdrcbdata.honor_cd_filename = TRUE;
else
hdrcbdata.honor_cd_filename = FALSE; |
|
I completely forgot that piece of logic. I'm thinking perhaps we can skip the protocol check completely. There are (or at least these once existed) for example users who'd get FTP:// URLs over a HTTP proxy that can also get HTTP headers like this. To me, it seems quite harmless to just remove that check and have all protocols get this ability. Thoughts? |
|
born in a884ffe. If we remove the scheme check could there be a vulnerability if some other protocol sends a content disposition header with the filename? |
|
I don't see how any other protocol would, and we only check headers if asked for it with the specific command line option so it seems like a very small risk that would ever happen. |
|
I say please land that patch and close this issue! |
|
I landed your patch with a test that verifies it. Thanks! |
|
|
|
It was your patch, I hope you like it! =) It is still a small risk AFAICS because of libcurl's protocol "guessing" logic based on the host name prefix. If you specify "curl ftp.funet.fi", it will speak FTP... |
|
Oops I forgot about that, I guess I'll eat my words. How about this: diff --git a/src/tool_cb_hdr.c b/src/tool_cb_hdr.c
index 5be02aa..f7d8355 100644
--- a/src/tool_cb_hdr.c
+++ b/src/tool_cb_hdr.c
@@ -48,6 +48,7 @@ size_t tool_header_cb(void *ptr, size_t size, size_t nmemb, vo
const char *str = ptr;
const size_t cb = size * nmemb;
const char *end = (char*)ptr + cb;
+ char *url = NULL;
/*
* Once that libcurl has called back tool_header_cb() the returned value
@@ -88,7 +89,9 @@ size_t tool_header_cb(void *ptr, size_t size, size_t nmemb, vo
*/
if(hdrcbdata->honor_cd_filename &&
- (cb > 20) && checkprefix("Content-disposition:", str)) {
+ (cb > 20) && checkprefix("Content-disposition:", str) &&
+ !curl_easy_getinfo(outs->config->easy, CURLINFO_EFFECTIVE_URL, &url) &&
+ url && (checkprefix("http://", url) || checkprefix("https://", url))) {
const char *p = str + 20;
/* look for the 'filename=' parameter
diff --git a/src/tool_operate.c b/src/tool_operate.c
index ab29c00..5f49efb 100644
--- a/src/tool_operate.c
+++ b/src/tool_operate.c
@@ -1295,12 +1295,7 @@ static CURLcode operate_do(struct GlobalConfig *global,
my_setopt_flags(curl, CURLOPT_REDIR_PROTOCOLS, config->proto_redir);
if(config->content_disposition
- && (urlnode->flags & GETOUT_USEREMOTE)
- && (checkprefix("http://", this_url) ||
- checkprefix("https://", this_url) ||
- (!strstr(this_url, "://") &&
- (!config->proto_default ||
- checkprefix("http", config->proto_default)))))
+ && (urlnode->flags & GETOUT_USEREMOTE))
hdrcbdata.honor_cd_filename = TRUE;
else
hdrcbdata.honor_cd_filename = FALSE; |
|
Oh, clever. Should work just fine indeed! |
- Move the existing scheme check from tool_operate. In the case of --remote-header-name we want to parse Content-disposition for a filename, but only if the scheme is http or https. A recent adjustment 0dc4d8e was made to account for schemeless URLs however it's not 100% accurate. To remedy that I've moved the scheme check to the header callback, since at that point the library has already determined the scheme. Bug: #760 Reported-by: Kai Noda
|
Thanks, landed in b9728bc. |
I did this
I expected the following
I expected to successfully download
rustup-setup.exeaccording tobut I got the following error:
curl/libcurl version
operating system
The text was updated successfully, but these errors were encountered: