diff --git a/indra/llcorehttp/_httpoprequest.cpp b/indra/llcorehttp/_httpoprequest.cpp
index 12eed06b1089c55a93654edd443eea407bbdd3c3..207ed8e1e4ca690ccc9d020aac341216cc397216 100644
--- a/indra/llcorehttp/_httpoprequest.cpp
+++ b/indra/llcorehttp/_httpoprequest.cpp
@@ -669,7 +669,7 @@ int HttpOpRequest::debugCallback(CURL * handle, curl_infotype info, char * buffe
std::string safe_line;
std::string tag;
bool logit(false);
- len = (std::min)(len, size_t(256)); // Keep things reasonable in all cases
+ const size_t log_len((std::min)(len, size_t(256))); // Keep things reasonable in all cases
switch (info)
{
@@ -677,7 +677,7 @@ int HttpOpRequest::debugCallback(CURL * handle, curl_infotype info, char * buffe
if (op->mTracing >= HTTP_TRACE_CURL_HEADERS)
{
tag = "TEXT";
- escape_libcurl_debug_data(buffer, len, true, safe_line);
+ escape_libcurl_debug_data(buffer, log_len, true, safe_line);
logit = true;
}
break;
@@ -686,7 +686,7 @@ int HttpOpRequest::debugCallback(CURL * handle, curl_infotype info, char * buffe
if (op->mTracing >= HTTP_TRACE_CURL_HEADERS)
{
tag = "HEADERIN";
- escape_libcurl_debug_data(buffer, len, true, safe_line);
+ escape_libcurl_debug_data(buffer, log_len, true, safe_line);
logit = true;
}
break;
@@ -695,7 +695,7 @@ int HttpOpRequest::debugCallback(CURL * handle, curl_infotype info, char * buffe
if (op->mTracing >= HTTP_TRACE_CURL_HEADERS)
{
tag = "HEADEROUT";
- escape_libcurl_debug_data(buffer, 2 * len, true, safe_line); // Goes out as one line
+ escape_libcurl_debug_data(buffer, log_len, true, safe_line); // Goes out as one line unlike header_in
logit = true;
}
break;
@@ -707,7 +707,7 @@ int HttpOpRequest::debugCallback(CURL * handle, curl_infotype info, char * buffe
logit = true;
if (op->mTracing >= HTTP_TRACE_CURL_BODIES)
{
- escape_libcurl_debug_data(buffer, len, false, safe_line);
+ escape_libcurl_debug_data(buffer, log_len, false, safe_line);
}
else
{
@@ -725,7 +725,7 @@ int HttpOpRequest::debugCallback(CURL * handle, curl_infotype info, char * buffe
logit = true;
if (op->mTracing >= HTTP_TRACE_CURL_BODIES)
{
- escape_libcurl_debug_data(buffer, len, false, safe_line);
+ escape_libcurl_debug_data(buffer, log_len, false, safe_line);
}
else
{
diff --git a/indra/llcorehttp/_httppolicy.cpp b/indra/llcorehttp/_httppolicy.cpp
index 76c1e2243166b98ed0a50e7d4db0ee4ffa251c32..014bd37e2e56d37cd6064fd1c2e36720b2f9c6d0 100644
--- a/indra/llcorehttp/_httppolicy.cpp
+++ b/indra/llcorehttp/_httppolicy.cpp
@@ -4,7 +4,7 @@
*
* $LicenseInfo:firstyear=2012&license=viewerlgpl$
* Second Life Viewer Source Code
- * Copyright (C) 2012, Linden Research, Inc.
+ * Copyright (C) 2012-2013, Linden Research, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -319,33 +319,13 @@ bool HttpPolicy::cancel(HttpHandle handle)
bool HttpPolicy::stageAfterCompletion(HttpOpRequest * op)
{
- static const HttpStatus cant_connect(HttpStatus::EXT_CURL_EASY, CURLE_COULDNT_CONNECT);
- static const HttpStatus cant_res_proxy(HttpStatus::EXT_CURL_EASY, CURLE_COULDNT_RESOLVE_PROXY);
- static const HttpStatus cant_res_host(HttpStatus::EXT_CURL_EASY, CURLE_COULDNT_RESOLVE_HOST);
- static const HttpStatus send_error(HttpStatus::EXT_CURL_EASY, CURLE_SEND_ERROR);
- static const HttpStatus recv_error(HttpStatus::EXT_CURL_EASY, CURLE_RECV_ERROR);
- static const HttpStatus upload_failed(HttpStatus::EXT_CURL_EASY, CURLE_UPLOAD_FAILED);
- static const HttpStatus op_timedout(HttpStatus::EXT_CURL_EASY, CURLE_OPERATION_TIMEDOUT);
- static const HttpStatus post_error(HttpStatus::EXT_CURL_EASY, CURLE_HTTP_POST_ERROR);
-
// Retry or finalize
if (! op->mStatus)
{
- // If this failed, we might want to retry. Have to inspect
- // the status a little more deeply for those reasons worth retrying...
- if (op->mPolicyRetries < op->mPolicyRetryLimit &&
- ((op->mStatus.isHttpStatus() && op->mStatus.mType >= 499 && op->mStatus.mType <= 599) ||
- cant_connect == op->mStatus ||
- cant_res_proxy == op->mStatus ||
- cant_res_host == op->mStatus ||
- send_error == op->mStatus ||
- recv_error == op->mStatus ||
- upload_failed == op->mStatus ||
- op_timedout == op->mStatus ||
- post_error == op->mStatus))
+ // If this failed, we might want to retry.
+ if (op->mPolicyRetries < op->mPolicyRetryLimit && op->mStatus.isRetryable())
{
- // Okay, worth a retry. We include 499 in this test as
- // it's the old 'who knows?' error from many grid services...
+ // Okay, worth a retry.
retryOp(op);
return true; // still active/ready
}
diff --git a/indra/llcorehttp/httpcommon.cpp b/indra/llcorehttp/httpcommon.cpp
index f2fcbf77a3b66f76c4dd472efead2184a18b2edd..0738760763827751ae9678d22d0cf31f39b53ced 100644
--- a/indra/llcorehttp/httpcommon.cpp
+++ b/indra/llcorehttp/httpcommon.cpp
@@ -4,7 +4,7 @@
*
* $LicenseInfo:firstyear=2012&license=viewerlgpl$
* Second Life Viewer Source Code
- * Copyright (C) 2012, Linden Research, Inc.
+ * Copyright (C) 2012-2013, Linden Research, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -116,6 +116,7 @@ std::string HttpStatus::toString() const
{ 415, "Unsupported Media Type" },
{ 416, "Requested range not satisfiable" },
{ 417, "Expectation Failed" },
+ { 499, "Linden Catch-All" },
{ 500, "Internal Server Error" },
{ 501, "Not Implemented" },
{ 502, "Bad Gateway" },
@@ -174,6 +175,37 @@ std::string HttpStatus::toString() const
}
return std::string("Unknown error");
}
+
+
+// Pass true on statuses that might actually be cleared by a
+// retry. Library failures, calling problems, etc. aren't
+// going to be fixed by squirting bits all over the Net.
+bool HttpStatus::isRetryable() const
+{
+ static const HttpStatus cant_connect(HttpStatus::EXT_CURL_EASY, CURLE_COULDNT_CONNECT);
+ static const HttpStatus cant_res_proxy(HttpStatus::EXT_CURL_EASY, CURLE_COULDNT_RESOLVE_PROXY);
+ static const HttpStatus cant_res_host(HttpStatus::EXT_CURL_EASY, CURLE_COULDNT_RESOLVE_HOST);
+ static const HttpStatus send_error(HttpStatus::EXT_CURL_EASY, CURLE_SEND_ERROR);
+ static const HttpStatus recv_error(HttpStatus::EXT_CURL_EASY, CURLE_RECV_ERROR);
+ static const HttpStatus upload_failed(HttpStatus::EXT_CURL_EASY, CURLE_UPLOAD_FAILED);
+ static const HttpStatus op_timedout(HttpStatus::EXT_CURL_EASY, CURLE_OPERATION_TIMEDOUT);
+ static const HttpStatus post_error(HttpStatus::EXT_CURL_EASY, CURLE_HTTP_POST_ERROR);
+ static const HttpStatus partial_file(HttpStatus::EXT_CURL_EASY, CURLE_PARTIAL_FILE);
+ static const HttpStatus inv_cont_range(HttpStatus::LLCORE, HE_INV_CONTENT_RANGE_HDR);
+
+ return ((isHttpStatus() && mType >= 499 && mType <= 599) || // Include special 499 in retryables
+ *this == cant_connect || // Connection reset/endpoint problems
+ *this == cant_res_proxy || // DNS problems
+ *this == cant_res_host || // DNS problems
+ *this == send_error || // General socket problems
+ *this == recv_error || // General socket problems
+ *this == upload_failed || // Transport problem
+ *this == op_timedout || // Timer expired
+ *this == post_error || // Transport problem
+ *this == partial_file || // Data inconsistency in response
+ *this == inv_cont_range); // Short data read disagrees with content-range
+}
+
} // end namespace LLCore
diff --git a/indra/llcorehttp/httpcommon.h b/indra/llcorehttp/httpcommon.h
index c0d4ec5aada659b00a0af8e5f0f743cb0be695b4..41fb5164cfdf3b279728ada93b9ca3aaf82388a0 100644
--- a/indra/llcorehttp/httpcommon.h
+++ b/indra/llcorehttp/httpcommon.h
@@ -4,7 +4,7 @@
*
* $LicenseInfo:firstyear=2012&license=viewerlgpl$
* Second Life Viewer Source Code
- * Copyright (C) 2012, Linden Research, Inc.
+ * Copyright (C) 2012-2013, Linden Research, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -303,6 +303,12 @@ struct HttpStatus
{
return mType >= type_enum_t(100) && mType <= type_enum_t(999);
}
+
+ /// Returns true if the status is one that will be retried
+ /// internally. Provided for external consumption for cases
+ /// where that logic needs to be replicated. Only applies
+ /// to failed statuses, successful statuses will return false.
+ bool isRetryable() const;
}; // end struct HttpStatus
diff --git a/indra/llcorehttp/tests/test_httprequest.hpp b/indra/llcorehttp/tests/test_httprequest.hpp
index 16f39845bb03cb5b811104c9302448f6ecb385fc..ff84b04070ea75a26d8f602d008bcf98d8c3a06c 100644
--- a/indra/llcorehttp/tests/test_httprequest.hpp
+++ b/indra/llcorehttp/tests/test_httprequest.hpp
@@ -2670,10 +2670,15 @@ void HttpRequestTestObjectType::test<22>()
mMemTotal = GetMemTotal();
mHandlerCalls = 0;
+ HttpOptions * options = NULL;
HttpRequest * req = NULL;
try
{
+ // options set
+ options = new HttpOptions();
+ options->setRetries(1); // Partial_File is retryable and can timeout in here
+
// Get singletons created
HttpRequest::createService();
@@ -2699,7 +2704,7 @@ void HttpRequestTestObjectType::test<22>()
url_base + buffer,
0,
25,
- NULL,
+ options,
NULL,
&handler);
ensure("Valid handle returned for ranged request", handle != LLCORE_HTTP_HANDLE_INVALID);
@@ -2707,18 +2712,19 @@ void HttpRequestTestObjectType::test<22>()
// Run the notification pump.
int count(0);
- int limit(10);
+ int limit(30);
while (count++ < limit && mHandlerCalls < test_count)
{
req->update(1000000);
usleep(100000);
}
- ensure("Request executed in reasonable time", count < limit);
- ensure("One handler invocation for each request", mHandlerCalls == test_count);
+ ensure("Request executed in reasonable time - ms1", count < limit);
+ ensure("One handler invocation for each request - ms1", mHandlerCalls == test_count);
// ======================================
// Issue bug2295 GETs that will get a libcurl 18 (PARTIAL_FILE)
// ======================================
+ mHandlerCalls = 0;
mStatus = HttpStatus(HttpStatus::EXT_CURL_EASY, CURLE_PARTIAL_FILE);
static const int test2_count(1);
for (int i(0); i < test2_count; ++i)
@@ -2730,7 +2736,39 @@ void HttpRequestTestObjectType::test<22>()
url_base + buffer,
0,
25,
+ options,
NULL,
+ &handler);
+ ensure("Valid handle returned for ranged request", handle != LLCORE_HTTP_HANDLE_INVALID);
+ }
+
+ // Run the notification pump.
+ count = 0;
+ limit = 30;
+ while (count++ < limit && mHandlerCalls < test2_count)
+ {
+ req->update(1000000);
+ usleep(100000);
+ }
+ ensure("Request executed in reasonable time - ms2", count < limit);
+ ensure("One handler invocation for each request - ms2", mHandlerCalls == test2_count);
+
+ // ======================================
+ // Issue bug2295 GETs that will get an llcorehttp HE_INV_CONTENT_RANGE_HDR status
+ // ======================================
+ mHandlerCalls = 0;
+ mStatus = HttpStatus(HttpStatus::LLCORE, HE_INV_CONTENT_RANGE_HDR);
+ static const int test3_count(1);
+ for (int i(0); i < test3_count; ++i)
+ {
+ char buffer[128];
+ sprintf(buffer, "/bug2295/inv_cont_range/%d/", i);
+ HttpHandle handle = req->requestGetByteRange(HttpRequest::DEFAULT_POLICY_ID,
+ 0U,
+ url_base + buffer,
+ 0,
+ 25,
+ options,
NULL,
&handler);
ensure("Valid handle returned for ranged request", handle != LLCORE_HTTP_HANDLE_INVALID);
@@ -2738,32 +2776,33 @@ void HttpRequestTestObjectType::test<22>()
// Run the notification pump.
count = 0;
- limit = 10;
- while (count++ < limit && mHandlerCalls < (test_count + test2_count))
+ limit = 30;
+ while (count++ < limit && mHandlerCalls < test3_count)
{
req->update(1000000);
usleep(100000);
}
- ensure("Request executed in reasonable time", count < limit);
- ensure("One handler invocation for each request", mHandlerCalls == (test_count + test2_count));
+ ensure("Request executed in reasonable time - ms3", count < limit);
+ ensure("One handler invocation for each request - ms3", mHandlerCalls == test3_count);
// ======================================
// Okay, request a shutdown of the servicing thread
// ======================================
mStatus = HttpStatus();
+ mHandlerCalls = 0;
HttpHandle handle = req->requestStopThread(&handler);
ensure("Valid handle returned for second request", handle != LLCORE_HTTP_HANDLE_INVALID);
// Run the notification pump again
count = 0;
- limit = 10;
- while (count++ < limit && mHandlerCalls < (test_count + test2_count + 1))
+ limit = 20;
+ while (count++ < limit && mHandlerCalls < 1)
{
req->update(1000000);
usleep(100000);
}
- ensure("Second request executed in reasonable time", count < limit);
- ensure("Second handler invocation", mHandlerCalls == (test_count + test2_count + 1));
+ ensure("Shutdown request executed in reasonable time", count < limit);
+ ensure("Shutdown handler invocation", mHandlerCalls == 1);
// See that we actually shutdown the thread
count = 0;
@@ -2773,7 +2812,14 @@ void HttpRequestTestObjectType::test<22>()
usleep(100000);
}
ensure("Thread actually stopped running", HttpService::isStopped());
-
+
+ // release options
+ if (options)
+ {
+ options->release();
+ options = NULL;
+ }
+
// release the request object
delete req;
req = NULL;
@@ -2781,8 +2827,6 @@ void HttpRequestTestObjectType::test<22>()
// Shut down service
HttpRequest::destroyService();
- ensure("4 + 1 handler calls on the way out", (test_count + test2_count + 1) == mHandlerCalls);
-
#if defined(WIN32)
// Can only do this memory test on Windows. On other platforms,
// the LL logging system holds on to memory and produces what looks
diff --git a/indra/llcorehttp/tests/test_llcorehttp_peer.py b/indra/llcorehttp/tests/test_llcorehttp_peer.py
index 7f8f765366c45e4610b00256bc0a7af5d4f8f17d..8796ae57c7ca58c41fbd09a3e2805424611da956 100644
--- a/indra/llcorehttp/tests/test_llcorehttp_peer.py
+++ b/indra/llcorehttp/tests/test_llcorehttp_peer.py
@@ -35,6 +35,10 @@
import select
import getopt
from threading import Thread
+try:
+ from cStringIO import StringIO
+except ImportError:
+ from StringIO import StringIO
from BaseHTTPServer import HTTPServer, BaseHTTPRequestHandler
from SocketServer import ThreadingMixIn
@@ -64,6 +68,7 @@ class TestHTTPRequestHandler(BaseHTTPRequestHandler):
-- '/bug2295/00000018/0/' Generates PARTIAL_FILE (18) error in libcurl.
"Content-Range: bytes 0-75/2983",
"Content-Length: 76"
+ -- '/bug2295/inv_cont_range/0/' Generates HE_INVALID_CONTENT_RANGE error in llcorehttp.
Some combinations make no sense, there's no effort to protect
you from that.
@@ -150,6 +155,7 @@ def answer(self, data, withdata=True):
# appear in the body without actually getting the body.
# Library needs to defend against this case.
#
+ body = None
if "/bug2295/0/" in self.path:
self.send_response(206)
self.send_header("Content-Range", "bytes 0-75/2983")
@@ -164,6 +170,10 @@ def answer(self, data, withdata=True):
self.send_response(206)
self.send_header("Content-Range", "bytes 0-75/2983")
self.send_header("Content-Length", "76")
+ elif "/bug2295/inv_cont_range/0/" in self.path:
+ self.send_response(206)
+ self.send_header("Content-Range", "bytes 0-75/2983")
+ body = "Some text, but not enough."
else:
# Unknown request
self.send_response(400)
@@ -171,7 +181,8 @@ def answer(self, data, withdata=True):
self.reflect_headers()
self.send_header("Content-type", "text/plain")
self.end_headers()
- # No data
+ if body:
+ self.wfile.write(body)
else:
# Normal response path
data = data.copy() # we're going to modify