From bba59073c52e6dd00ddc18e0e40d1f7dfc1c9315 Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Tue, 23 May 2017 10:32:18 +0200 Subject: redirect: store the "would redirect to" URL when max redirs is reached Test 1261 added to verify. Reported-by: Lloyd Fournier Fixes #1489 Closes #1497 --- docs/cmdline-opts/write-out.d | 6 ++--- lib/multi.c | 11 ++------ lib/transfer.c | 52 ++++++++++++++++++------------------ tests/data/Makefile.inc | 2 +- tests/data/test1261 | 61 +++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 94 insertions(+), 38 deletions(-) create mode 100644 tests/data/test1261 diff --git a/docs/cmdline-opts/write-out.d b/docs/cmdline-opts/write-out.d index 394151f31..3747845cc 100644 --- a/docs/cmdline-opts/write-out.d +++ b/docs/cmdline-opts/write-out.d @@ -65,9 +65,9 @@ The result of the HTTPS proxy's SSL peer certificate verification that was requested. 0 means the verification was successful. (Added in 7.52.0) .TP .B redirect_url -When an HTTP request was made without --location to follow redirects, this -variable will show the actual URL a redirect \fIwould\fP take you to. (Added -in 7.18.2) +When an HTTP request was made without --location to follow redirects (or when +--max-redir is met), this variable will show the actual URL a redirect +\fIwould\fP have gone to. (Added in 7.18.2) .TP .B remote_ip The remote IP address of the most recently done connection - can be either diff --git a/lib/multi.c b/lib/multi.c index 09be44396..4b2872743 100644 --- a/lib/multi.c +++ b/lib/multi.c @@ -1715,20 +1715,18 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi, else { /* Follow failed */ result = drc; - free(newurl); } } else { /* done didn't return OK or SEND_ERROR */ result = drc; - free(newurl); } } else { /* Have error handler disconnect conn if we can't retry */ stream_error = TRUE; - free(newurl); } + free(newurl); } else { /* failure detected */ @@ -1963,9 +1961,6 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi, if(!result) { multistate(data, CURLM_STATE_CONNECT); rc = CURLM_CALL_MULTI_PERFORM; - newurl = NULL; /* handed over the memory ownership to - Curl_follow(), make sure we don't free() it - here */ } } } @@ -1979,9 +1974,7 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi, newurl = data->req.location; data->req.location = NULL; result = Curl_follow(data, newurl, FOLLOW_FAKE); - if(!result) - newurl = NULL; /* allocation was handed over Curl_follow() */ - else + if(result) stream_error = TRUE; } diff --git a/lib/transfer.c b/lib/transfer.c index 73497f79f..799fd4da8 100644 --- a/lib/transfer.c +++ b/lib/transfer.c @@ -1624,9 +1624,7 @@ static char *concat_url(const char *base, const char *relurl) * as given by the remote server and set up the new URL to request. */ CURLcode Curl_follow(struct Curl_easy *data, - char *newurl, /* this 'newurl' is the Location: string, - and it must be malloc()ed before passed - here */ + char *newurl, /* the Location: string */ followtype type) /* see transfer.h */ { #ifdef CURL_DISABLE_HTTP @@ -1639,33 +1637,36 @@ CURLcode Curl_follow(struct Curl_easy *data, /* Location: redirect */ bool disallowport = FALSE; + bool reachedmax = FALSE; if(type == FOLLOW_REDIR) { if((data->set.maxredirs != -1) && - (data->set.followlocation >= data->set.maxredirs)) { - failf(data, "Maximum (%ld) redirects followed", data->set.maxredirs); - return CURLE_TOO_MANY_REDIRECTS; + (data->set.followlocation >= data->set.maxredirs)) { + reachedmax = TRUE; + type = FOLLOW_FAKE; /* switch to fake to store the would-be-redirected + to URL */ } + else { + /* mark the next request as a followed location: */ + data->state.this_is_a_follow = TRUE; - /* mark the next request as a followed location: */ - data->state.this_is_a_follow = TRUE; + data->set.followlocation++; /* count location-followers */ - data->set.followlocation++; /* count location-followers */ + if(data->set.http_auto_referer) { + /* We are asked to automatically set the previous URL as the referer + when we get the next URL. We pick the ->url field, which may or may + not be 100% correct */ - if(data->set.http_auto_referer) { - /* We are asked to automatically set the previous URL as the referer - when we get the next URL. We pick the ->url field, which may or may - not be 100% correct */ + if(data->change.referer_alloc) { + Curl_safefree(data->change.referer); + data->change.referer_alloc = FALSE; + } - if(data->change.referer_alloc) { - Curl_safefree(data->change.referer); - data->change.referer_alloc = FALSE; + data->change.referer = strdup(data->change.url); + if(!data->change.referer) + return CURLE_OUT_OF_MEMORY; + data->change.referer_alloc = TRUE; /* yes, free this later */ } - - data->change.referer = strdup(data->change.url); - if(!data->change.referer) - return CURLE_OUT_OF_MEMORY; - data->change.referer_alloc = TRUE; /* yes, free this later */ } } @@ -1677,7 +1678,6 @@ CURLcode Curl_follow(struct Curl_easy *data, char *absolute = concat_url(data->change.url, newurl); if(!absolute) return CURLE_OUT_OF_MEMORY; - free(newurl); newurl = absolute; } else { @@ -1693,8 +1693,6 @@ CURLcode Curl_follow(struct Curl_easy *data, if(!newest) return CURLE_OUT_OF_MEMORY; strcpy_url(newest, newurl); /* create a space-free URL */ - - free(newurl); /* that was no good */ newurl = newest; /* use this instead now */ } @@ -1703,6 +1701,11 @@ CURLcode Curl_follow(struct Curl_easy *data, /* we're only figuring out the new url if we would've followed locations but now we're done so we can get out! */ data->info.wouldredirect = newurl; + + if(reachedmax) { + failf(data, "Maximum (%ld) redirects followed", data->set.maxredirs); + return CURLE_TOO_MANY_REDIRECTS; + } return CURLE_OK; } @@ -1716,7 +1719,6 @@ CURLcode Curl_follow(struct Curl_easy *data, data->change.url = newurl; data->change.url_alloc = TRUE; - newurl = NULL; /* don't free! */ infof(data, "Issue another request to this URL: '%s'\n", data->change.url); diff --git a/tests/data/Makefile.inc b/tests/data/Makefile.inc index 089f598c6..17d2b7945 100644 --- a/tests/data/Makefile.inc +++ b/tests/data/Makefile.inc @@ -129,7 +129,7 @@ test1228 test1229 test1230 test1231 test1232 test1233 test1234 test1235 \ test1236 test1237 test1238 test1239 test1240 test1241 test1242 test1243 \ test1244 test1245 test1246 test1247 test1248 test1249 test1250 test1251 \ test1252 test1253 test1254 test1255 test1256 test1257 test1258 test1259 \ -test1260 \ +test1260 test1261 \ \ test1280 test1281 test1282 test1283 test1284 test1285 test1286 test1287 \ test1288 \ diff --git a/tests/data/test1261 b/tests/data/test1261 new file mode 100644 index 000000000..7f887994e --- /dev/null +++ b/tests/data/test1261 @@ -0,0 +1,61 @@ + + + +HTTP +HTTP GET +redirect_url +followlocation +--write-out + + + +# Server-side + + +HTTP/1.1 301 This is a weirdo text message swsclose +Location: data/10290002.txt?coolsite=yes +Content-Length: 62 +Connection: close + +This server reply is for testing a simple Location: following + + + +# Client-side + + +http + + +'redirect_url' with --location and --max-redir + + +http://%HOSTIP:%HTTPPORT/we/want/our/1261 -w '%{redirect_url}\n' --location --max-redir 0 + + + +# Verify data after the test has been "shot" + + +^User-Agent:.* + + +GET /we/want/our/1261 HTTP/1.1 +Host: %HOSTIP:%HTTPPORT +Accept: */* + + +# CURLE_TOO_MANY_REDIRECTS + +47 + + +HTTP/1.1 301 This is a weirdo text message swsclose +Location: data/10290002.txt?coolsite=yes +Content-Length: 62 +Connection: close + +http://%HOSTIP:%HTTPPORT/we/want/our/data/10290002.txt?coolsite=yes + + + -- cgit v1.2.3