Skip to content

Commit d91f1da

Browse files
AgainPsychoXAgainPsychoX
authored
Better follow redirection for HTTPClient (#7157)
* Add way to force follow redirections in `HTTPClient` * Follow other client implementations about `HTTP_CODE_FOUND`; Small rewrite of `sendRequest` function of `HTTPClient` * Better names for follow redirection modes in `HTTPClient` Also changed a bit order of the enums (0 element to be DISABLED) * Rewrite `sendRequest` to remove recursion Also got rid of unnecessary `redirectCount` field. Now redirect counting and limiting is handled in `sendRequest` directly. * Use new `setFollowRedirects` of `HTTPClient` instead deprecated one. * More explanatory comment for `followRedirects_t` in HTTPClient
1 parent 1127a09 commit d91f1da

File tree

5 files changed

+125
-56
lines changed

5 files changed

+125
-56
lines changed

Diff for: ‎libraries/ESP8266HTTPClient/src/ESP8266HTTPClient.cpp

+69-36
Original file line numberDiff line numberDiff line change
@@ -423,7 +423,6 @@ void HTTPClient::end(void)
423423
{
424424
disconnect(false);
425425
clear();
426-
_redirectCount = 0;
427426
}
428427

429428
/**
@@ -558,8 +557,17 @@ bool HTTPClient::setURL(const String& url)
558557
/**
559558
* set true to follow redirects.
560559
* @param follow
560+
* @deprecated
561561
*/
562562
void HTTPClient::setFollowRedirects(bool follow)
563+
{
564+
_followRedirects = follow ? HTTPC_STRICT_FOLLOW_REDIRECTS : HTTPC_DISABLE_FOLLOW_REDIRECTS;
565+
}
566+
/**
567+
* set redirect follow mode. See `followRedirects_t` enum for avaliable modes.
568+
* @param follow
569+
*/
570+
void HTTPClient::setFollowRedirects(followRedirects_t follow)
563571
{
564572
_followRedirects = follow;
565573
}
@@ -652,8 +660,9 @@ int HTTPClient::sendRequest(const char * type, const String& payload)
652660
*/
653661
int HTTPClient::sendRequest(const char * type, const uint8_t * payload, size_t size)
654662
{
663+
int code;
655664
bool redirect = false;
656-
int code = 0;
665+
uint16_t redirectCount = 0;
657666
do {
658667
// wipe out any existing headers from previous request
659668
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
662671
}
663672
}
664673

665-
redirect = false;
666-
DEBUG_HTTPCLIENT("[HTTP-Client][sendRequest] type: '%s' redirCount: %d\n", type, _redirectCount);
674+
DEBUG_HTTPCLIENT("[HTTP-Client][sendRequest] type: '%s' redirCount: %d\n", type, redirectCount);
667675

668676
// connect to server
669677
if(!connect()) {
@@ -687,9 +695,9 @@ int HTTPClient::sendRequest(const char * type, const uint8_t * payload, size_t s
687695
int towrite = std::min((int)size, (int)HTTP_TCP_BUFFER_SIZE);
688696
written = _client->write(p + bytesWritten, towrite);
689697
if (written < 0) {
690-
return returnError(HTTPC_ERROR_SEND_PAYLOAD_FAILED);
698+
return returnError(HTTPC_ERROR_SEND_PAYLOAD_FAILED);
691699
} else if (written == 0) {
692-
return returnError(HTTPC_ERROR_CONNECTION_LOST);
700+
return returnError(HTTPC_ERROR_CONNECTION_LOST);
693701
}
694702
bytesWritten += written;
695703
size -= written;
@@ -700,42 +708,67 @@ int HTTPClient::sendRequest(const char * type, const uint8_t * payload, size_t s
700708
code = handleHeaderResponse();
701709

702710
//
703-
// We can follow redirects for 301/302/307 for GET and HEAD requests and
704-
// and we have not exceeded the redirect limit preventing an infinite
705-
// redirect loop.
706-
//
711+
// Handle redirections as stated in RFC document:
707712
// https://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html
708713
//
709-
if (_followRedirects &&
710-
(_redirectCount < _redirectLimit) &&
711-
(_location.length() > 0) &&
712-
(code == 301 || code == 302 || code == 307) &&
713-
(!strcmp(type, "GET") || !strcmp(type, "HEAD"))
714-
) {
715-
_redirectCount += 1; // increment the count for redirect.
716-
redirect = true;
717-
DEBUG_HTTPCLIENT("[HTTP-Client][sendRequest] following redirect:: '%s' redirCount: %d\n", _location.c_str(), _redirectCount);
718-
if (!setURL(_location)) {
719-
// return the redirect instead of handling on failure of setURL()
720-
redirect = false;
714+
// Implementing HTTP_CODE_FOUND as redirection with GET method,
715+
// to follow most of existing user agent implementations.
716+
//
717+
redirect = false;
718+
if (
719+
_followRedirects != HTTPC_DISABLE_FOLLOW_REDIRECTS &&
720+
redirectCount < _redirectLimit &&
721+
_location.length() > 0
722+
) {
723+
switch (code) {
724+
// redirecting using the same method
725+
case HTTP_CODE_MOVED_PERMANENTLY:
726+
case HTTP_CODE_TEMPORARY_REDIRECT: {
727+
if (
728+
// allow to force redirections on other methods
729+
// (the RFC require user to accept the redirection)
730+
_followRedirects == HTTPC_FORCE_FOLLOW_REDIRECTS ||
731+
// allow GET and HEAD methods without force
732+
!strcmp(type, "GET") ||
733+
!strcmp(type, "HEAD")
734+
) {
735+
redirectCount += 1;
736+
DEBUG_HTTPCLIENT("[HTTP-Client][sendRequest] following redirect (the same method): '%s' redirCount: %d\n", _location.c_str(), redirectCount);
737+
if (!setURL(_location)) {
738+
DEBUG_HTTPCLIENT("[HTTP-Client][sendRequest] failed setting URL for redirection\n");
739+
// no redirection
740+
break;
741+
}
742+
// redirect using the same request method and payload, diffrent URL
743+
redirect = true;
744+
}
745+
break;
746+
}
747+
// redirecting with method dropped to GET or HEAD
748+
// note: it does not need `HTTPC_FORCE_FOLLOW_REDIRECTS` for any method
749+
case HTTP_CODE_FOUND:
750+
case HTTP_CODE_SEE_OTHER: {
751+
redirectCount += 1;
752+
DEBUG_HTTPCLIENT("[HTTP-Client][sendRequest] following redirect (dropped to GET/HEAD): '%s' redirCount: %d\n", _location.c_str(), redirectCount);
753+
if (!setURL(_location)) {
754+
DEBUG_HTTPCLIENT("[HTTP-Client][sendRequest] failed setting URL for redirection\n");
755+
// no redirection
756+
break;
757+
}
758+
// redirect after changing method to GET/HEAD and dropping payload
759+
type = "GET";
760+
payload = nullptr;
761+
size = 0;
762+
redirect = true;
763+
break;
764+
}
765+
766+
default:
767+
break;
721768
}
722769
}
723-
724770
} while (redirect);
725771

726-
// handle 303 redirect for non GET/HEAD by changing to GET and requesting new url
727-
if (_followRedirects &&
728-
(_redirectCount < _redirectLimit) &&
729-
(_location.length() > 0) &&
730-
(code == 303) &&
731-
strcmp(type, "GET") && strcmp(type, "HEAD")
732-
) {
733-
_redirectCount += 1;
734-
if (setURL(_location)) {
735-
code = sendRequest("GET");
736-
}
737-
}
738-
739772
// handle Server Response (Header)
740773
return returnError(code);
741774
}

Diff for: ‎libraries/ESP8266HTTPClient/src/ESP8266HTTPClient.h

+23-3
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,23 @@ typedef enum {
130130
HTTPC_TE_CHUNKED
131131
} transferEncoding_t;
132132

133+
/**
134+
* redirection follow mode.
135+
* + `HTTPC_DISABLE_FOLLOW_REDIRECTS` - no redirection will be followed.
136+
* + `HTTPC_STRICT_FOLLOW_REDIRECTS` - strict RFC2616, only requests using
137+
* GET or HEAD methods will be redirected (using the same method),
138+
* since the RFC requires end-user confirmation in other cases.
139+
* + `HTTPC_FORCE_FOLLOW_REDIRECTS` - all redirections will be followed,
140+
* regardless of a used method. New request will use the same method,
141+
* and they will include the same body data and the same headers.
142+
* In the sense of the RFC, it's just like every redirection is confirmed.
143+
*/
144+
typedef enum {
145+
HTTPC_DISABLE_FOLLOW_REDIRECTS,
146+
HTTPC_STRICT_FOLLOW_REDIRECTS,
147+
HTTPC_FORCE_FOLLOW_REDIRECTS
148+
} followRedirects_t;
149+
133150
#if HTTPCLIENT_1_1_COMPATIBLE
134151
class TransportTraits;
135152
typedef std::unique_ptr<TransportTraits> TransportTraitsPtr;
@@ -173,8 +190,12 @@ class HTTPClient
173190
void setAuthorization(const char * user, const char * password);
174191
void setAuthorization(const char * auth);
175192
void setTimeout(uint16_t timeout);
176-
void setFollowRedirects(bool follow);
193+
194+
// Redirections
195+
void setFollowRedirects(bool follow) __attribute__ ((deprecated));
196+
void setFollowRedirects(followRedirects_t follow);
177197
void setRedirectLimit(uint16_t limit); // max redirects to follow for a single request
198+
178199
bool setURL(const String& url); // handy for handling redirects
179200
void useHTTP10(bool usehttp10 = true);
180201

@@ -252,8 +273,7 @@ class HTTPClient
252273
int _returnCode = 0;
253274
int _size = -1;
254275
bool _canReuse = false;
255-
bool _followRedirects = false;
256-
uint16_t _redirectCount = 0;
276+
followRedirects_t _followRedirects = HTTPC_DISABLE_FOLLOW_REDIRECTS;
257277
uint16_t _redirectLimit = 10;
258278
String _location;
259279
transferEncoding_t _transferEncoding = HTTPC_TE_IDENTITY;

Diff for: ‎libraries/ESP8266httpUpdate/src/ESP8266httpUpdate.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,12 @@ extern "C" uint32_t _FS_start;
3030
extern "C" uint32_t _FS_end;
3131

3232
ESP8266HTTPUpdate::ESP8266HTTPUpdate(void)
33-
: _httpClientTimeout(8000), _followRedirects(false), _ledPin(-1)
33+
: _httpClientTimeout(8000), _followRedirects(HTTPC_DISABLE_FOLLOW_REDIRECTS), _ledPin(-1)
3434
{
3535
}
3636

3737
ESP8266HTTPUpdate::ESP8266HTTPUpdate(int httpClientTimeout)
38-
: _httpClientTimeout(httpClientTimeout), _followRedirects(false), _ledPin(-1)
38+
: _httpClientTimeout(httpClientTimeout), _followRedirects(HTTPC_DISABLE_FOLLOW_REDIRECTS), _ledPin(-1)
3939
{
4040
}
4141

Diff for: ‎libraries/ESP8266httpUpdate/src/ESP8266httpUpdate.h

+15-2
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,20 @@ class ESP8266HTTPUpdate
8282
_rebootOnUpdate = reboot;
8383
}
8484

85-
void followRedirects(bool follow)
85+
/**
86+
* set true to follow redirects.
87+
* @param follow
88+
* @deprecated Please use `setFollowRedirects(followRedirects_t follow)`
89+
*/
90+
void followRedirects(bool follow) __attribute__ ((deprecated))
91+
{
92+
_followRedirects = follow ? HTTPC_STRICT_FOLLOW_REDIRECTS : HTTPC_DISABLE_FOLLOW_REDIRECTS;
93+
}
94+
/**
95+
* set redirect follow mode. See `followRedirects_t` enum for avaliable modes.
96+
* @param follow
97+
*/
98+
void setFollowRedirects(followRedirects_t follow)
8699
{
87100
_followRedirects = follow;
88101
}
@@ -160,7 +173,7 @@ class ESP8266HTTPUpdate
160173
bool _closeConnectionsOnUpdate = true;
161174
private:
162175
int _httpClientTimeout;
163-
bool _followRedirects;
176+
followRedirects_t _followRedirects;
164177

165178
// Callbacks
166179
HTTPUpdateStartCB _cbStart;

Diff for: ‎tests/device/test_sw_http_client/test_sw_http_client.ino

+16-13
Original file line numberDiff line numberDiff line change
@@ -71,10 +71,10 @@ TEST_CASE("HTTP GET & POST requests", "[HTTPClient]")
7171
http.end();
7272
}
7373
{
74-
// 301 redirect with follow enabled
74+
// GET 301 redirect with strict RFC follow enabled
7575
WiFiClient client;
7676
HTTPClient http;
77-
http.setFollowRedirects(true);
77+
http.setFollowRedirects(HTTPC_STRICT_FOLLOW_REDIRECTS);
7878
String uri = String("/redirect301?host=")+getenv("SERVER_IP");
7979
http.begin(client, getenv("SERVER_IP"), 8088, uri.c_str());
8080
auto httpCode = http.GET();
@@ -83,7 +83,7 @@ TEST_CASE("HTTP GET & POST requests", "[HTTPClient]")
8383
REQUIRE(payload == "redirect success");
8484
}
8585
{
86-
// 301 redirect with follow disabled
86+
// GET 301 redirect with follow disabled
8787
WiFiClient client;
8888
HTTPClient http;
8989
String uri = String("/redirect301?host=")+getenv("SERVER_IP");
@@ -92,10 +92,10 @@ TEST_CASE("HTTP GET & POST requests", "[HTTPClient]")
9292
REQUIRE(httpCode == 301);
9393
}
9494
{
95-
// 302 redirect with follow enabled
95+
// GET 302 redirect with strict RFC follow enabled
9696
WiFiClient client;
9797
HTTPClient http;
98-
http.setFollowRedirects(true);
98+
http.setFollowRedirects(HTTPC_STRICT_FOLLOW_REDIRECTS);
9999
String uri = String("/redirect302?host=")+getenv("SERVER_IP");
100100
http.begin(client, getenv("SERVER_IP"), 8088, uri.c_str());
101101
auto httpCode = http.GET();
@@ -104,7 +104,7 @@ TEST_CASE("HTTP GET & POST requests", "[HTTPClient]")
104104
REQUIRE(payload == "redirect success");
105105
}
106106
{
107-
// 302 redirect with follow disabled
107+
// GET 302 redirect with follow disabled
108108
WiFiClient client;
109109
HTTPClient http;
110110
String uri = String("/redirect302?host=")+getenv("SERVER_IP");
@@ -113,10 +113,10 @@ TEST_CASE("HTTP GET & POST requests", "[HTTPClient]")
113113
REQUIRE(httpCode == 302);
114114
}
115115
{
116-
// 307 redirect with follow enabled
116+
// GET 307 redirect with strict RFC follow enabled
117117
WiFiClient client;
118118
HTTPClient http;
119-
http.setFollowRedirects(true);
119+
http.setFollowRedirects(HTTPC_STRICT_FOLLOW_REDIRECTS);
120120
String uri = String("/redirect307?host=")+getenv("SERVER_IP");
121121
http.begin(client, getenv("SERVER_IP"), 8088, uri.c_str());
122122
auto httpCode = http.GET();
@@ -125,7 +125,7 @@ TEST_CASE("HTTP GET & POST requests", "[HTTPClient]")
125125
REQUIRE(payload == "redirect success");
126126
}
127127
{
128-
// 307 redirect with follow disabled
128+
// GET 307 redirect with follow disabled
129129
WiFiClient client;
130130
HTTPClient http;
131131
String uri = String("/redirect307?host=")+getenv("SERVER_IP");
@@ -134,31 +134,33 @@ TEST_CASE("HTTP GET & POST requests", "[HTTPClient]")
134134
REQUIRE(httpCode == 307);
135135
}
136136
{
137-
// 301 exceeding redirect limit
137+
// GET 301 exceeding redirect limit
138138
WiFiClient client;
139139
HTTPClient http;
140-
http.setFollowRedirects(true);
140+
http.setFollowRedirects(HTTPC_STRICT_FOLLOW_REDIRECTS);
141141
http.setRedirectLimit(0);
142142
String uri = String("/redirect301?host=")+getenv("SERVER_IP");
143143
http.begin(client, getenv("SERVER_IP"), 8088, uri.c_str());
144144
auto httpCode = http.GET();
145145
REQUIRE(httpCode == 301);
146146
}
147147
{
148-
// POST 303 redirect with follow enabled
148+
// POST 303 redirect with strict RFC follow enabled
149149
WiFiClient client;
150150
HTTPClient http;
151-
http.setFollowRedirects(true);
151+
http.setFollowRedirects(HTTPC_STRICT_FOLLOW_REDIRECTS);
152152
http.begin(client, getenv("SERVER_IP"), 8088, "/redirect303");
153153
auto httpCode = http.POST(getenv("SERVER_IP"));
154154
REQUIRE(httpCode == HTTP_CODE_OK);
155155
String payload = http.getString();
156156
REQUIRE(payload == "redirect success");
157+
// TODO: need check for dropping: redirection should use GET method
157158
}
158159
{
159160
// POST 303 redirect with follow disabled
160161
WiFiClient client;
161162
HTTPClient http;
163+
http.setFollowRedirects(HTTPC_DISABLE_FOLLOW_REDIRECTS);
162164
http.begin(client, getenv("SERVER_IP"), 8088, "/redirect303");
163165
auto httpCode = http.POST(getenv("SERVER_IP"));
164166
REQUIRE(httpCode == 303);
@@ -167,6 +169,7 @@ TEST_CASE("HTTP GET & POST requests", "[HTTPClient]")
167169
// 302 redirect with follow disabled
168170
WiFiClient client;
169171
HTTPClient http;
172+
http.setFollowRedirects(HTTPC_DISABLE_FOLLOW_REDIRECTS);
170173
String uri = String("/redirect302?host=")+getenv("SERVER_IP");
171174
http.begin(client, getenv("SERVER_IP"), 8088, uri.c_str());
172175
auto httpCode = http.GET();

0 commit comments

Comments
 (0)