From 1c0f87d82fbc2ae9bf5aa90522819bf82e2d99e5 Mon Sep 17 00:00:00 2001 From: Aleric Inglewood Date: Sat, 28 Sep 2013 20:41:10 +0200 Subject: [PATCH] Let curl follow redirects by default. Turns out that the only responders that want to get the redirect status codes themselves are the ones that already had a redirect_status_ok() exception. --- indra/llmessage/aicurl.cpp | 7 ++++--- indra/llmessage/aicurlthread.cpp | 2 +- indra/llmessage/llhttpclient.h | 9 ++++----- indra/newview/floatervoicelicense.cpp | 2 +- indra/newview/llfloatertos.cpp | 2 +- indra/newview/llmarketplacefunctions.cpp | 1 - indra/newview/llpathfindingmanager.cpp | 1 - indra/newview/lltexturefetch.cpp | 8 ++------ indra/newview/llviewermedia.cpp | 1 - indra/newview/llwebprofile.cpp | 6 ++---- 10 files changed, 15 insertions(+), 24 deletions(-) diff --git a/indra/llmessage/aicurl.cpp b/indra/llmessage/aicurl.cpp index 04b9c5a16..c6d42e1ad 100644 --- a/indra/llmessage/aicurl.cpp +++ b/indra/llmessage/aicurl.cpp @@ -1262,7 +1262,8 @@ std::string CurlEasyRequest::getLowercaseHostname(void) const //----------------------------------------------------------------------------- // BufferedCurlEasyRequest -static int const HTTP_REDIRECTS_DEFAULT = 10; +static int const HTTP_REDIRECTS_DEFAULT = 16; // Singu note: I've seen up to 10 redirects, so setting the limit to 10 is cutting it. + // This limit is only here to avoid a redirect loop (infinite redirections). LLChannelDescriptors const BufferedCurlEasyRequest::sChannels; LLMutex BufferedCurlEasyRequest::sResponderCallbackMutex; @@ -1411,8 +1412,8 @@ void BufferedCurlEasyRequest::prepRequest(AICurlEasyRequest_wat& curl_easy_reque curl_easy_request_w->setHeaderCallback(&curlHeaderCallback, lockobj); bool allow_cookies = headers.hasHeader("Cookie"); - // Allow up to ten redirects. - if (responder->followRedir()) + // Allow up to sixteen redirects. + if (!responder->pass_redirect_status()) { curl_easy_request_w->setopt(CURLOPT_FOLLOWLOCATION, 1); curl_easy_request_w->setopt(CURLOPT_MAXREDIRS, HTTP_REDIRECTS_DEFAULT); diff --git a/indra/llmessage/aicurlthread.cpp b/indra/llmessage/aicurlthread.cpp index 64a6b69fe..5339e9b65 100644 --- a/indra/llmessage/aicurlthread.cpp +++ b/indra/llmessage/aicurlthread.cpp @@ -2062,7 +2062,7 @@ void BufferedCurlEasyRequest::setStatusAndReason(U32 status, std::string const& // Sanity check. If the server replies with a redirect status then we better have that option turned on! if ((status >= 300 && status < 400) && mResponder && !mResponder->redirect_status_ok()) { - llerrs << "Received " << status << " (" << reason << ") for responder \"" << mResponder->getName() << "\" which has no followRedir()!" << llendl; + llerrs << "Received " << status << " (" << reason << ") for responder \"" << mResponder->getName() << "\" which does not allow redirection!" << llendl; } } diff --git a/indra/llmessage/llhttpclient.h b/indra/llmessage/llhttpclient.h index b4ce6facc..507ee2a04 100644 --- a/indra/llmessage/llhttpclient.h +++ b/indra/llmessage/llhttpclient.h @@ -221,12 +221,12 @@ public: // The default is to keep connections open for possible reuse. virtual bool forbidReuse(void) const { return false; } - // A derived class should return true if curl should follow redirections. - // The default is not to follow redirections. - virtual bool followRedir(void) const { return false; } + // A derived class should return true if curl should not follow redirections, but instead pass redirection status codes to the responder. + // The default is to follow redirections and not pass them to the responder. + virtual bool pass_redirect_status(void) const { return false; } // If this function returns false then we generate an error when a redirect status (300..399) is received. - virtual bool redirect_status_ok(void) const { return followRedir(); } + virtual bool redirect_status_ok(void) const { return true; } // Returns the capability type used by this responder. virtual AICapabilityType capability_type(void) const { return cap_other; } @@ -259,7 +259,6 @@ public: class ResponderHeadersOnly : public ResponderBase { private: /*virtual*/ bool needsHeaders(void) const { return true; } - /*virtual*/ bool followRedir(void) const { return true; } protected: // ResponderBase event diff --git a/indra/newview/floatervoicelicense.cpp b/indra/newview/floatervoicelicense.cpp index 101398006..288704d83 100644 --- a/indra/newview/floatervoicelicense.cpp +++ b/indra/newview/floatervoicelicense.cpp @@ -103,7 +103,7 @@ class LLIamHereVoice : public LLHTTPClient::ResponderWithResult }; /*virtual*/ AIHTTPTimeoutPolicy const& getHTTPTimeoutPolicy(void) const { return iamHereVoice_timeout; } - /*virtual*/ bool redirect_status_ok(void) const { return true; } + /*virtual*/ bool pass_redirect_status(void) const { return true; } /*virtual*/ char const* getName(void) const { return "LLIamHereVoice"; } }; diff --git a/indra/newview/llfloatertos.cpp b/indra/newview/llfloatertos.cpp index 8552b881a..eb70ca59f 100644 --- a/indra/newview/llfloatertos.cpp +++ b/indra/newview/llfloatertos.cpp @@ -131,7 +131,7 @@ class LLIamHere : public LLHTTPClient::ResponderWithResult }; /*virtual*/ AIHTTPTimeoutPolicy const& getHTTPTimeoutPolicy(void) const { return iamHere_timeout; } - /*virtual*/ bool redirect_status_ok(void) const { return true; } + /*virtual*/ bool pass_redirect_status(void) const { return true; } /*virtual*/ char const* getName(void) const { return "LLIamHere"; } }; diff --git a/indra/newview/llmarketplacefunctions.cpp b/indra/newview/llmarketplacefunctions.cpp index ed7c774c8..74a8ddbcf 100644 --- a/indra/newview/llmarketplacefunctions.cpp +++ b/indra/newview/llmarketplacefunctions.cpp @@ -201,7 +201,6 @@ namespace LLMarketplaceImport class LLImportGetResponder : public LLHTTPClient::ResponderWithCompleted { public: - /*virtual*/ bool followRedir(void) const { return true; } /*virtual*/ bool needsHeaders(void) const { return true; } /*virtual*/ void completedHeaders(U32 status, std::string const& reason, AIHTTPReceivedHeaders const& headers) diff --git a/indra/newview/llpathfindingmanager.cpp b/indra/newview/llpathfindingmanager.cpp index 7e5bec546..783c728c0 100644 --- a/indra/newview/llpathfindingmanager.cpp +++ b/indra/newview/llpathfindingmanager.cpp @@ -165,7 +165,6 @@ public: /*virtual*/ void result(const LLSD &pContent); /*virtual*/ void error(U32 pStatus, const std::string& pReason); - /*virtual*/ bool followRedir(void) const { return true; } /*virtual*/ AIHTTPTimeoutPolicy const& getHTTPTimeoutPolicy(void) const { return agentStateResponder_timeout; } /*virtual*/ char const* getName(void) const { return "AgentStateResponder"; } diff --git a/indra/newview/lltexturefetch.cpp b/indra/newview/lltexturefetch.cpp index 576e7864c..49798d84e 100644 --- a/indra/newview/lltexturefetch.cpp +++ b/indra/newview/lltexturefetch.cpp @@ -328,13 +328,12 @@ class HTTPGetResponder : public LLHTTPClient::ResponderWithCompleted { LOG_CLASS(HTTPGetResponder); public: - HTTPGetResponder(LLTextureFetch* fetcher, const LLUUID& id, U64 startTime, S32 requestedSize, U32 offset, bool redir) + HTTPGetResponder(LLTextureFetch* fetcher, const LLUUID& id, U64 startTime, S32 requestedSize, U32 offset) : mFetcher(fetcher) , mID(id) , mMetricsStartTime(startTime) , mRequestedSize(requestedSize) , mRequestedOffset(offset) - , mFollowRedir(redir) , mReplyOffset(0) , mReplyLength(0) , mReplyFullLength(0) @@ -449,7 +448,6 @@ public: } } - /*virtual*/ bool followRedir() const { return mFollowRedir; } /*virtual*/ AICapabilityType capability_type(void) const { return cap_texture; } /*virtual*/ AIHTTPTimeoutPolicy const& getHTTPTimeoutPolicy(void) const { return HTTPGetResponder_timeout; } /*virtual*/ char const* getName(void) const { return "HTTPGetResponder"; } @@ -464,8 +462,6 @@ private: U32 mReplyOffset; U32 mReplyLength; U32 mReplyFullLength; - - bool mFollowRedir; }; ////////////////////////////////////////////////////////////////////////////// @@ -1423,7 +1419,7 @@ bool LLTextureFetchWorker::doWork(S32 param) headers.addHeader("Range", llformat("bytes=%d-%d", mRequestedOffset, mRequestedOffset + mRequestedSize - 1)); } LLHTTPClient::request(mUrl, LLHTTPClient::HTTP_GET, NULL, - new HTTPGetResponder(mFetcher, mID, LLTimer::getTotalTime(), mRequestedSize, mRequestedOffset, true), + new HTTPGetResponder(mFetcher, mID, LLTimer::getTotalTime(), mRequestedSize, mRequestedOffset), headers, approved/*,*/ DEBUG_CURLIO_PARAM(debug_off), keep_alive, no_does_authentication, allow_compressed_reply, NULL, 0, NULL); } else diff --git a/indra/newview/llviewermedia.cpp b/indra/newview/llviewermedia.cpp index cdf86c074..6cd1e611f 100644 --- a/indra/newview/llviewermedia.cpp +++ b/indra/newview/llviewermedia.cpp @@ -304,7 +304,6 @@ public: LLViewerMediaWebProfileResponder(std::string host) : mHost(host) { } ~LLViewerMediaWebProfileResponder() { } - /*virtual*/ bool followRedir(void) const { return true; } /*virtual*/ bool needsHeaders(void) const { return true; } /*virtual*/ void completedHeaders(U32 status, std::string const& reason, AIHTTPReceivedHeaders const& headers) diff --git a/indra/newview/llwebprofile.cpp b/indra/newview/llwebprofile.cpp index 7286766c6..75d60f985 100644 --- a/indra/newview/llwebprofile.cpp +++ b/indra/newview/llwebprofile.cpp @@ -120,7 +120,6 @@ public: protected: /*virtual*/ AIHTTPTimeoutPolicy const& getHTTPTimeoutPolicy(void) const { return webProfileResponders_timeout; } - /*virtual*/ bool followRedir(void) const { return true; } /*virtual*/ char const* getName(void) const { return "LLWebProfileResponders::ConfigResponder"; } private: @@ -157,7 +156,6 @@ public: } protected: - /*virtual*/ bool followRedir(void) const { return true; } /*virtual*/ AIHTTPTimeoutPolicy const& getHTTPTimeoutPolicy(void) const { return webProfileResponders_timeout; } /*virtual*/ char const* getName(void) const { return "LLWebProfileResponders::PostImageRedirectResponder"; } @@ -181,7 +179,7 @@ public: // the just uploaded data, which fails // (CURLE_SEND_FAIL_REWIND: Send failed since rewinding of the data stream failed). // Handle it manually. - if (status == 303) + if (status == HTTP_SEE_OTHER) { AIHTTPHeaders headers; headers.addHeader("Accept", "*/*"); @@ -208,7 +206,7 @@ public: } protected: - /*virtual*/ bool redirect_status_ok(void) const { return true; } + /*virtual*/ bool pass_redirect_status(void) const { return true; } /*virtual*/ AIHTTPTimeoutPolicy const& getHTTPTimeoutPolicy(void) const { return webProfileResponders_timeout; } /*virtual*/ char const* getName(void) const { return "LLWebProfileResponders::PostImageResponder"; } };