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
2017-05-23 10:32:18 +02:00 · 2017-05-23 10:32:18 +02:00 · bba59073c5
--- 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
--- 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;
          }

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

--- 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 \
--- a/tests/data/test1261
+++ b/tests/data/test1261
@ -0,0 +1,61 @@
+<testcase>
+<info>
+<keywords>
+HTTP
+HTTP GET
+redirect_url
+followlocation
+--write-out
+</keywords>
+</info>
+
+# Server-side
+<reply>
+<data nocheck="yes">
+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
+</data>
+</reply>
+
+# Client-side
+<client>
+<server>
+http
+</server>
+ <name>
+'redirect_url' with --location and --max-redir
+ </name>
+ <command>
+http://%HOSTIP:%HTTPPORT/we/want/our/1261 -w '%{redirect_url}\n' --location --max-redir 0
+</command>
+</client>
+
+# Verify data after the test has been "shot"
+<verify>
+<strip>
+^User-Agent:.*
+</strip>
+<protocol>
+GET /we/want/our/1261 HTTP/1.1
+Host: %HOSTIP:%HTTPPORT
+Accept: */*
+
+</protocol>
+# CURLE_TOO_MANY_REDIRECTS
+<errorcode>
+47
+</errorcode>
+<stdout>
+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
+</stdout>
+</verify>
+</testcase>