livie
/
phoenix-firestorm
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

BUG-2295/MAINT-2624 unexpected crash around Content-Range: header processing 

Not certain what the source of the short data is with one resident but I'm
going to make these problems retryable as they are transport-related.  Lift
the retry detection into a method that should be reusable by others interested
in determining what is retryable.  Trace output handling on the libcurl debug
callback was attrocious.  Some unsafe length handling on my part was protected
by a second layer of defense.  Made that correct and more useful by logging
actual data sizes during trace.
master
Monty Brandenberg 2013-05-06 12:12:05 -04:00
parent f9850aa5d2
commit f5e8457e4e
6 changed files with 122 additions and 49 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
indra/llcorehttp/_httpoprequest.cpp
View File

@ -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
{

28
indra/llcorehttp/_httppolicy.cpp
View File

@ -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
}

34
indra/llcorehttp/httpcommon.cpp
View File

@ -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

8
indra/llcorehttp/httpcommon.h
View File

@ -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

76
indra/llcorehttp/tests/test_httprequest.hpp
View File

@ -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,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);
@ -2738,32 +2744,65 @@ 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 < test2_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 - 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);
}
// Run the notification pump.
count = 0;
limit = 30;
while (count++ < limit && mHandlerCalls < test3_count)
{
req->update(1000000);
usleep(100000);
}
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

13
indra/llcorehttp/tests/test_llcorehttp_peer.py
View File

@ -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