diff options
| -rw-r--r-- | indra/llcorehttp/_httpoprequest.cpp | 12 | ||||
| -rw-r--r-- | indra/llcorehttp/_httppolicy.cpp | 28 | ||||
| -rw-r--r-- | indra/llcorehttp/httpcommon.cpp | 34 | ||||
| -rw-r--r-- | indra/llcorehttp/httpcommon.h | 8 | ||||
| -rw-r--r-- | indra/llcorehttp/tests/test_httprequest.hpp | 74 | ||||
| -rw-r--r-- | indra/llcorehttp/tests/test_llcorehttp_peer.py | 13 |
6 files changed, 121 insertions, 48 deletions
diff --git a/indra/llcorehttp/_httpoprequest.cpp b/indra/llcorehttp/_httpoprequest.cpp index 12eed06b10..207ed8e1e4 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 76c1e22431..014bd37e2e 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 f2fcbf77a3..0738760763 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 c0d4ec5aad..41fb5164cf 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 16f39845bb..ff84b04070 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 7f8f765366..8796ae57c7 100644 --- a/indra/llcorehttp/tests/test_llcorehttp_peer.py +++ b/indra/llcorehttp/tests/test_llcorehttp_peer.py @@ -35,6 +35,10 @@ import time 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 @@ class TestHTTPRequestHandler(BaseHTTPRequestHandler): # 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 @@ class TestHTTPRequestHandler(BaseHTTPRequestHandler): 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 @@ class TestHTTPRequestHandler(BaseHTTPRequestHandler): 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 |