Better follow redirection for HTTPClient

libraries/ESP8266HTTPClient/src/ESP8266HTTPClient.cpp

@@ -423,7 +423,6 @@ void HTTPClient::end(void)
 {
  disconnect(false);
  clear();
- _redirectCount = 0;
 }
 
 /**
@@ -558,8 +557,17 @@ bool HTTPClient::setURL(const String& url)
 /**
  * set true to follow redirects.
  * @param follow
+ * @deprecated
  */
 void HTTPClient::setFollowRedirects(bool follow)
+{
+ _followRedirects = follow ? HTTPC_STRICT_FOLLOW_REDIRECTS : HTTPC_DISABLE_FOLLOW_REDIRECTS;
+}
+/**
+ * set redirect follow mode. See `followRedirects_t` enum for avaliable modes.
+ * @param follow
+ */
+void HTTPClient::setFollowRedirects(followRedirects_t follow)
 {
  _followRedirects = follow;
 }
@@ -652,8 +660,9 @@ int HTTPClient::sendRequest(const char * type, const String& payload)
  */
 int HTTPClient::sendRequest(const char * type, const uint8_t * payload, size_t size)
 {
+ int code;
  bool redirect = false;
- int code = 0;
+ uint16_t redirectCount = 0;
  do {
  // wipe out any existing headers from previous request
  for(size_t i = 0; i < _headerKeysCount; i++) {
@@ -662,8 +671,7 @@ int HTTPClient::sendRequest(const char * type, const uint8_t * payload, size_t s
  }
  }
 
- redirect = false;
- DEBUG_HTTPCLIENT("[HTTP-Client][sendRequest] type: '%s' redirCount: %d\n", type, _redirectCount);
+ DEBUG_HTTPCLIENT("[HTTP-Client][sendRequest] type: '%s' redirCount: %d\n", type, redirectCount);
 
  // connect to server
  if(!connect()) {
@@ -687,9 +695,9 @@ int HTTPClient::sendRequest(const char * type, const uint8_t * payload, size_t s
  int towrite = std::min((int)size, (int)HTTP_TCP_BUFFER_SIZE);
  written = _client->write(p + bytesWritten, towrite);
  if (written < 0) {
- return returnError(HTTPC_ERROR_SEND_PAYLOAD_FAILED);
+  return returnError(HTTPC_ERROR_SEND_PAYLOAD_FAILED);
  } else if (written == 0) {
- return returnError(HTTPC_ERROR_CONNECTION_LOST);
+  return returnError(HTTPC_ERROR_CONNECTION_LOST);
  }
  bytesWritten += written;
  size -= written;
@@ -700,42 +708,67 @@ int HTTPClient::sendRequest(const char * type, const uint8_t * payload, size_t s
  code = handleHeaderResponse();
 
  //
- // We can follow redirects for 301/302/307 for GET and HEAD requests and
- // and we have not exceeded the redirect limit preventing an infinite
- // redirect loop.
- //
+ // Handle redirections as stated in RFC document:
  // https://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html
  //
- if (_followRedirects &&
- (_redirectCount < _redirectLimit) &&
- (_location.length() > 0) &&
- (code == 301 || code == 302 || code == 307) &&
- (!strcmp(type, "GET") || !strcmp(type, "HEAD"))
- ) {
- _redirectCount += 1; // increment the count for redirect.
- redirect = true;
- DEBUG_HTTPCLIENT("[HTTP-Client][sendRequest] following redirect:: '%s' redirCount: %d\n", _location.c_str(), _redirectCount);
- if (!setURL(_location)) {
- // return the redirect instead of handling on failure of setURL()
- redirect = false;
+ // Implementing HTTP_CODE_FOUND as redirection with GET method,
+ // to follow most of existing user agent implementations.
+ //
+ redirect = false;
+ if (
+ _followRedirects != HTTPC_DISABLE_FOLLOW_REDIRECTS && 
+ redirectCount < _redirectLimit &&
+ _location.length() > 0
+ ) {
+ switch (code) {
+ // redirecting using the same method
+ case HTTP_CODE_MOVED_PERMANENTLY:
+ case HTTP_CODE_TEMPORARY_REDIRECT: {
+ if (
+ // allow to force redirections on other methods
+ // (the RFC require user to accept the redirection)
+ _followRedirects == HTTPC_FORCE_FOLLOW_REDIRECTS ||
+ // allow GET and HEAD methods without force
+ !strcmp(type, "GET") || 
+ !strcmp(type, "HEAD")
+ ) {
+ redirectCount += 1;
+ DEBUG_HTTPCLIENT("[HTTP-Client][sendRequest] following redirect (the same method): '%s' redirCount: %d\n", _location.c_str(), redirectCount);
+ if (!setURL(_location)) {
+ DEBUG_HTTPCLIENT("[HTTP-Client][sendRequest] failed setting URL for redirection\n");
+ // no redirection
+ break;
+ }
+ // redirect using the same request method and payload, diffrent URL
+ redirect = true;
+ }
+ break;
+ }
+ // redirecting with method dropped to GET or HEAD
+ // note: it does not need `HTTPC_FORCE_FOLLOW_REDIRECTS` for any method
+ case HTTP_CODE_FOUND:
+ case HTTP_CODE_SEE_OTHER: {
+ redirectCount += 1;
+ DEBUG_HTTPCLIENT("[HTTP-Client][sendRequest] following redirect (dropped to GET/HEAD): '%s' redirCount: %d\n", _location.c_str(), redirectCount);
+ if (!setURL(_location)) {
+ DEBUG_HTTPCLIENT("[HTTP-Client][sendRequest] failed setting URL for redirection\n");
+ // no redirection
+ break;
+ }
+ // redirect after changing method to GET/HEAD and dropping payload
+ type = "GET";
+ payload = nullptr;
+ size = 0;
+ redirect = true;
+ break;
+ }
+
+ default:
+ break;
  }
  }
-
  } while (redirect);
 
- // handle 303 redirect for non GET/HEAD by changing to GET and requesting new url
- if (_followRedirects &&
- (_redirectCount < _redirectLimit) &&
- (_location.length() > 0) &&
- (code == 303) &&
- strcmp(type, "GET") && strcmp(type, "HEAD")
- ) {
- _redirectCount += 1;
- if (setURL(_location)) {
- code = sendRequest("GET");
- }
- }
-
  // handle Server Response (Header)
  return returnError(code);
 }

libraries/ESP8266HTTPClient/src/ESP8266HTTPClient.h

@@ -130,6 +130,19 @@ typedef enum {
  HTTPC_TE_CHUNKED
 } transferEncoding_t;
 
+/**
+ * redirection follow mode.
+ * + `HTTPC_DISABLE_FOLLOW_REDIRECTS` - no redirection will be followed.
+ * + `HTTPC_STRICT_FOLLOW_REDIRECTS` - strict RFC2616, only requests using
+ * GET or HEAD methods will be redirected without user confirmation.
+ * + `HTTPC_FORCE_FOLLOW_REDIRECTS` - all redirections will be followed.
+ */
+typedef enum {
+ HTTPC_DISABLE_FOLLOW_REDIRECTS,
+ HTTPC_STRICT_FOLLOW_REDIRECTS,
+ HTTPC_FORCE_FOLLOW_REDIRECTS
+} followRedirects_t;
+
 #if HTTPCLIENT_1_1_COMPATIBLE
 class TransportTraits;
 typedef std::unique_ptr<TransportTraits> TransportTraitsPtr;
@@ -173,8 +186,12 @@ class HTTPClient
  void setAuthorization(const char * user, const char * password);
  void setAuthorization(const char * auth);
  void setTimeout(uint16_t timeout);
- void setFollowRedirects(bool follow);
+
+ // Redirections
+ void setFollowRedirects(bool follow) __attribute__ ((deprecated));
+ void setFollowRedirects(followRedirects_t follow);
  void setRedirectLimit(uint16_t limit); // max redirects to follow for a single request
+
  bool setURL(const String& url); // handy for handling redirects
  void useHTTP10(bool usehttp10 = true);
 
@@ -252,8 +269,7 @@ class HTTPClient
  int _returnCode = 0;
  int _size = -1;
  bool _canReuse = false;
- bool _followRedirects = false;
- uint16_t _redirectCount = 0;
+ followRedirects_t _followRedirects = HTTPC_DISABLE_FOLLOW_REDIRECTS;
  uint16_t _redirectLimit = 10;
  String _location;
  transferEncoding_t _transferEncoding = HTTPC_TE_IDENTITY;

libraries/ESP8266httpUpdate/src/ESP8266httpUpdate.cpp

@@ -30,12 +30,12 @@ extern "C" uint32_t _FS_start;
 extern "C" uint32_t _FS_end;
 
 ESP8266HTTPUpdate::ESP8266HTTPUpdate(void)
- : _httpClientTimeout(8000), _followRedirects(false), _ledPin(-1)
+ : _httpClientTimeout(8000), _followRedirects(HTTPC_DISABLE_FOLLOW_REDIRECTS), _ledPin(-1)
 {
 }
 
 ESP8266HTTPUpdate::ESP8266HTTPUpdate(int httpClientTimeout)
- : _httpClientTimeout(httpClientTimeout), _followRedirects(false), _ledPin(-1)
+ : _httpClientTimeout(httpClientTimeout), _followRedirects(HTTPC_DISABLE_FOLLOW_REDIRECTS), _ledPin(-1)
 {
 }
 

libraries/ESP8266httpUpdate/src/ESP8266httpUpdate.h

@@ -82,7 +82,20 @@ class ESP8266HTTPUpdate
  _rebootOnUpdate = reboot;
  }
 
- void followRedirects(bool follow)
+ /**
+ * set true to follow redirects.
+ * @param follow
+ * @deprecated Please use `setFollowRedirects(followRedirects_t follow)`
+ */
+ void followRedirects(bool follow) __attribute__ ((deprecated))
+ {
+ _followRedirects = follow ? HTTPC_STRICT_FOLLOW_REDIRECTS : HTTPC_DISABLE_FOLLOW_REDIRECTS;
+ }
+ /**
+ * set redirect follow mode. See `followRedirects_t` enum for avaliable modes.
+ * @param follow
+ */
+ void setFollowRedirects(followRedirects_t follow)
  {
  _followRedirects = follow;
  }
@@ -160,7 +173,7 @@ class ESP8266HTTPUpdate
  bool _closeConnectionsOnUpdate = true;
 private:
  int _httpClientTimeout;
- bool _followRedirects;
+ followRedirects_t _followRedirects;
 
  // Callbacks
  HTTPUpdateStartCB _cbStart;

tests/device/test_sw_http_client/test_sw_http_client.ino

@@ -71,10 +71,10 @@ TEST_CASE("HTTP GET & POST requests", "[HTTPClient]")
  http.end();
  }
  {
- // 301 redirect with follow enabled
+ // GET 301 redirect with strict RFC follow enabled 
  WiFiClient client;
  HTTPClient http;
- http.setFollowRedirects(true);
+ http.setFollowRedirects(HTTPC_STRICT_FOLLOW_REDIRECTS);
  String uri = String("/redirect301?host=")+getenv("SERVER_IP");
  http.begin(client, getenv("SERVER_IP"), 8088, uri.c_str());
  auto httpCode = http.GET();
@@ -83,7 +83,7 @@ TEST_CASE("HTTP GET & POST requests", "[HTTPClient]")
  REQUIRE(payload == "redirect success");
  }
  {
- // 301 redirect with follow disabled
+ // GET 301 redirect with follow disabled
  WiFiClient client;
  HTTPClient http;
  String uri = String("/redirect301?host=")+getenv("SERVER_IP");
@@ -92,10 +92,10 @@ TEST_CASE("HTTP GET & POST requests", "[HTTPClient]")
  REQUIRE(httpCode == 301);
  }
  {
- // 302 redirect with follow enabled
+ // GET 302 redirect with strict RFC follow enabled 
  WiFiClient client;
  HTTPClient http;
- http.setFollowRedirects(true);
+ http.setFollowRedirects(HTTPC_STRICT_FOLLOW_REDIRECTS);
  String uri = String("/redirect302?host=")+getenv("SERVER_IP");
  http.begin(client, getenv("SERVER_IP"), 8088, uri.c_str());
  auto httpCode = http.GET();
@@ -104,7 +104,7 @@ TEST_CASE("HTTP GET & POST requests", "[HTTPClient]")
  REQUIRE(payload == "redirect success");
  }
  {
- // 302 redirect with follow disabled
+ // GET 302 redirect with follow disabled
  WiFiClient client;
  HTTPClient http;
  String uri = String("/redirect302?host=")+getenv("SERVER_IP");
@@ -113,10 +113,10 @@ TEST_CASE("HTTP GET & POST requests", "[HTTPClient]")
  REQUIRE(httpCode == 302);
  }
  {
- // 307 redirect with follow enabled
+ // GET 307 redirect with strict RFC follow enabled
  WiFiClient client;
  HTTPClient http;
- http.setFollowRedirects(true);
+ http.setFollowRedirects(HTTPC_STRICT_FOLLOW_REDIRECTS);
  String uri = String("/redirect307?host=")+getenv("SERVER_IP");
  http.begin(client, getenv("SERVER_IP"), 8088, uri.c_str());
  auto httpCode = http.GET();
@@ -125,7 +125,7 @@ TEST_CASE("HTTP GET & POST requests", "[HTTPClient]")
  REQUIRE(payload == "redirect success");
  }
  {
- // 307 redirect with follow disabled
+ // GET 307 redirect with follow disabled
  WiFiClient client;
  HTTPClient http;
  String uri = String("/redirect307?host=")+getenv("SERVER_IP");
@@ -134,31 +134,33 @@ TEST_CASE("HTTP GET & POST requests", "[HTTPClient]")
  REQUIRE(httpCode == 307);
  }
  {
- // 301 exceeding redirect limit
+ // GET 301 exceeding redirect limit
  WiFiClient client;
  HTTPClient http;
- http.setFollowRedirects(true);
+ http.setFollowRedirects(HTTPC_STRICT_FOLLOW_REDIRECTS);
  http.setRedirectLimit(0);
  String uri = String("/redirect301?host=")+getenv("SERVER_IP");
  http.begin(client, getenv("SERVER_IP"), 8088, uri.c_str());
  auto httpCode = http.GET();
  REQUIRE(httpCode == 301);
  }
  {
- // POST 303 redirect with follow enabled
+ // POST 303 redirect with strict RFC follow enabled
  WiFiClient client;
  HTTPClient http;
- http.setFollowRedirects(true);
+ http.setFollowRedirects(HTTPC_STRICT_FOLLOW_REDIRECTS);
  http.begin(client, getenv("SERVER_IP"), 8088, "/redirect303");
  auto httpCode = http.POST(getenv("SERVER_IP"));
  REQUIRE(httpCode == HTTP_CODE_OK);
  String payload = http.getString();
  REQUIRE(payload == "redirect success");
+ // TODO: need check for dropping: redirection should use GET method
  }
  {
  // POST 303 redirect with follow disabled
  WiFiClient client;
  HTTPClient http;
+ http.setFollowRedirects(HTTPC_DISABLE_FOLLOW_REDIRECTS);
  http.begin(client, getenv("SERVER_IP"), 8088, "/redirect303");
  auto httpCode = http.POST(getenv("SERVER_IP"));
  REQUIRE(httpCode == 303);
@@ -167,6 +169,7 @@ TEST_CASE("HTTP GET & POST requests", "[HTTPClient]")
  // 302 redirect with follow disabled
  WiFiClient client;
  HTTPClient http;
+ http.setFollowRedirects(HTTPC_DISABLE_FOLLOW_REDIRECTS);
  String uri = String("/redirect302?host=")+getenv("SERVER_IP");
  http.begin(client, getenv("SERVER_IP"), 8088, uri.c_str());
  auto httpCode = http.GET();