BUG-2295/MAINT-2624 [FIXED] Crash in HttpOpRequest::stageFromActive w/ Content-Range
Don't rely on a response body being present should a Content-Range header be parsed. Unit tests captured the original crash and confirm the fix.master
Monty Brandenberg
parent
b49f6e1e74
commit
f9850aa5d2
|
|
@ -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
|
||||
|
|
@ -186,9 +186,11 @@ void HttpOpRequest::stageFromActive(HttpService * service)
|
|||
if (mReplyLength)
|
||||
{
|
||||
// If non-zero, we received and processed a Content-Range
|
||||
// header with the response. Verify that what it says
|
||||
// is consistent with the received data.
|
||||
if (mReplyLength != mReplyBody->size())
|
||||
// header with the response. If there is received data
|
||||
// (and there may not be due to protocol violations,
|
||||
// HEAD requests, etc., see BUG-2295) Verify that what it
|
||||
// says is consistent with the received data.
|
||||
if (mReplyBody && mReplyBody->size() && mReplyLength != mReplyBody->size())
|
||||
{
|
||||
// Not as expected, fail the request
|
||||
mStatus = HttpStatus(HttpStatus::LLCORE, HE_INV_CONTENT_RANGE_HDR);
|
||||
|
|
|
|||
|
|
@ -48,8 +48,9 @@ class HttpHeaders;
|
|||
/// individual pieces of the response.
|
||||
///
|
||||
/// Typical usage will have the caller interrogate the object
|
||||
/// and return from the handler callback. Instances are refcounted
|
||||
/// and callers can bump the count and retain the object as needed.
|
||||
/// during the handler callback and then simply returning.
|
||||
/// But instances are refcounted and and callers can add a
|
||||
/// reference and hold onto the object after the callback.
|
||||
///
|
||||
/// Threading: Not intrinsically thread-safe.
|
||||
///
|
||||
|
|
@ -119,6 +120,10 @@ public:
|
|||
/// caller is going to have to make assumptions on receipt of
|
||||
/// a 206 status. The @full value may also be zero in cases of
|
||||
/// parsing problems or a wild-carded length response.
|
||||
///
|
||||
/// These values will not necessarily agree with the data in
|
||||
/// the body itself (if present). The BufferArray object
|
||||
/// is authoritative for actual data length.
|
||||
void getRange(unsigned int * offset, unsigned int * length, unsigned int * full) const
|
||||
{
|
||||
*offset = mReplyOffset;
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
@ -2650,6 +2650,156 @@ void HttpRequestTestObjectType::test<21>()
|
|||
}
|
||||
}
|
||||
|
||||
// BUG-2295 Tests - Content-Range header received but no body
|
||||
template <> template <>
|
||||
void HttpRequestTestObjectType::test<22>()
|
||||
{
|
||||
ScopedCurlInit ready;
|
||||
|
||||
std::string url_base(get_base_url());
|
||||
// std::cerr << "Base: " << url_base << std::endl;
|
||||
|
||||
set_test_name("BUG-2295");
|
||||
|
||||
// Handler can be stack-allocated *if* there are no dangling
|
||||
// references to it after completion of this method.
|
||||
// Create before memory record as the string copy will bump numbers.
|
||||
TestHandler2 handler(this, "handler");
|
||||
|
||||
// record the total amount of dynamically allocated memory
|
||||
mMemTotal = GetMemTotal();
|
||||
mHandlerCalls = 0;
|
||||
|
||||
HttpRequest * req = NULL;
|
||||
|
||||
try
|
||||
{
|
||||
// Get singletons created
|
||||
HttpRequest::createService();
|
||||
|
||||
// Start threading early so that thread memory is invariant
|
||||
// over the test.
|
||||
HttpRequest::startThread();
|
||||
|
||||
// create a new ref counted object with an implicit reference
|
||||
req = new HttpRequest();
|
||||
ensure("Memory allocated on construction", mMemTotal < GetMemTotal());
|
||||
|
||||
// ======================================
|
||||
// Issue bug2295 GETs that will get a 206
|
||||
// ======================================
|
||||
mStatus = HttpStatus(206);
|
||||
static const int test_count(3);
|
||||
for (int i(0); i < test_count; ++i)
|
||||
{
|
||||
char buffer[128];
|
||||
sprintf(buffer, "/bug2295/%d/", i);
|
||||
HttpHandle handle = req->requestGetByteRange(HttpRequest::DEFAULT_POLICY_ID,
|
||||
0U,
|
||||
url_base + buffer,
|
||||
0,
|
||||
25,
|
||||
NULL,
|
||||
NULL,
|
||||
&handler);
|
||||
ensure("Valid handle returned for ranged request", handle != LLCORE_HTTP_HANDLE_INVALID);
|
||||
}
|
||||
|
||||
// Run the notification pump.
|
||||
int count(0);
|
||||
int limit(10);
|
||||
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);
|
||||
|
||||
// ======================================
|
||||
// Issue bug2295 GETs that will get a libcurl 18 (PARTIAL_FILE)
|
||||
// ======================================
|
||||
mStatus = HttpStatus(HttpStatus::EXT_CURL_EASY, CURLE_PARTIAL_FILE);
|
||||
static const int test2_count(1);
|
||||
for (int i(0); i < test2_count; ++i)
|
||||
{
|
||||
char buffer[128];
|
||||
sprintf(buffer, "/bug2295/00000012/%d/", i);
|
||||
HttpHandle handle = req->requestGetByteRange(HttpRequest::DEFAULT_POLICY_ID,
|
||||
0U,
|
||||
url_base + buffer,
|
||||
0,
|
||||
25,
|
||||
NULL,
|
||||
NULL,
|
||||
&handler);
|
||||
ensure("Valid handle returned for ranged request", handle != LLCORE_HTTP_HANDLE_INVALID);
|
||||
}
|
||||
|
||||
// Run the notification pump.
|
||||
count = 0;
|
||||
limit = 10;
|
||||
while (count++ < limit && mHandlerCalls < (test_count + 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));
|
||||
|
||||
// ======================================
|
||||
// Okay, request a shutdown of the servicing thread
|
||||
// ======================================
|
||||
mStatus = HttpStatus();
|
||||
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))
|
||||
{
|
||||
req->update(1000000);
|
||||
usleep(100000);
|
||||
}
|
||||
ensure("Second request executed in reasonable time", count < limit);
|
||||
ensure("Second handler invocation", mHandlerCalls == (test_count + test2_count + 1));
|
||||
|
||||
// See that we actually shutdown the thread
|
||||
count = 0;
|
||||
limit = 10;
|
||||
while (count++ < limit && ! HttpService::isStopped())
|
||||
{
|
||||
usleep(100000);
|
||||
}
|
||||
ensure("Thread actually stopped running", HttpService::isStopped());
|
||||
|
||||
// release the request object
|
||||
delete req;
|
||||
req = NULL;
|
||||
|
||||
// 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
|
||||
// like memory leaks...
|
||||
|
||||
// printf("Old mem: %d, New mem: %d\n", mMemTotal, GetMemTotal());
|
||||
ensure("Memory usage back to that at entry", mMemTotal == GetMemTotal());
|
||||
#endif
|
||||
}
|
||||
catch (...)
|
||||
{
|
||||
stop_thread(req);
|
||||
delete req;
|
||||
HttpRequest::destroyService();
|
||||
throw;
|
||||
}
|
||||
}
|
||||
|
||||
} // end namespace tut
|
||||
|
||||
|
|
|
|||
|
|
@ -9,7 +9,7 @@
|
|||
|
||||
$LicenseInfo:firstyear=2008&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
|
||||
|
|
@ -47,6 +47,26 @@ from testrunner import freeport, run, debug, VERBOSE
|
|||
class TestHTTPRequestHandler(BaseHTTPRequestHandler):
|
||||
"""This subclass of BaseHTTPRequestHandler is to receive and echo
|
||||
LLSD-flavored messages sent by the C++ LLHTTPClient.
|
||||
|
||||
Target URLs are fairly free-form and are assembled by
|
||||
concatinating fragments. Currently defined fragments
|
||||
are:
|
||||
- '/reflect/' Request headers are bounced back to caller
|
||||
after prefixing with 'X-Reflect-'
|
||||
- '/fail/' Body of request can contain LLSD with
|
||||
'reason' string and 'status' integer
|
||||
which will become response header.
|
||||
- '/bug2295/' 206 response, no data in body:
|
||||
-- '/bug2295/0/' "Content-Range: bytes 0-75/2983"
|
||||
-- '/bug2295/1/' "Content-Range: bytes 0-75/*"
|
||||
-- '/bug2295/2/' "Content-Range: bytes 0-75/2983",
|
||||
"Content-Length: 0"
|
||||
-- '/bug2295/00000018/0/' Generates PARTIAL_FILE (18) error in libcurl.
|
||||
"Content-Range: bytes 0-75/2983",
|
||||
"Content-Length: 76"
|
||||
|
||||
Some combinations make no sense, there's no effort to protect
|
||||
you from that.
|
||||
"""
|
||||
def read(self):
|
||||
# The following logic is adapted from the library module
|
||||
|
|
@ -107,22 +127,7 @@ class TestHTTPRequestHandler(BaseHTTPRequestHandler):
|
|||
if "/sleep/" in self.path:
|
||||
time.sleep(30)
|
||||
|
||||
if "fail" not in self.path:
|
||||
data = data.copy() # we're going to modify
|
||||
# Ensure there's a "reply" key in data, even if there wasn't before
|
||||
data["reply"] = data.get("reply", llsd.LLSD("success"))
|
||||
response = llsd.format_xml(data)
|
||||
debug("success: %s", response)
|
||||
self.send_response(200)
|
||||
if "/reflect/" in self.path:
|
||||
self.reflect_headers()
|
||||
self.send_header("Content-type", "application/llsd+xml")
|
||||
self.send_header("Content-Length", str(len(response)))
|
||||
self.send_header("X-LL-Special", "Mememememe");
|
||||
self.end_headers()
|
||||
if withdata:
|
||||
self.wfile.write(response)
|
||||
else: # fail requested
|
||||
if "fail" in self.path:
|
||||
status = data.get("status", 500)
|
||||
# self.responses maps an int status to a (short, long) pair of
|
||||
# strings. We want the longer string. That's why we pass a string
|
||||
|
|
@ -138,6 +143,51 @@ class TestHTTPRequestHandler(BaseHTTPRequestHandler):
|
|||
if "/reflect/" in self.path:
|
||||
self.reflect_headers()
|
||||
self.end_headers()
|
||||
elif "/bug2295/" in self.path:
|
||||
# Test for https://jira.secondlife.com/browse/BUG-2295
|
||||
#
|
||||
# Client can receive a header indicating data should
|
||||
# appear in the body without actually getting the body.
|
||||
# Library needs to defend against this case.
|
||||
#
|
||||
if "/bug2295/0/" in self.path:
|
||||
self.send_response(206)
|
||||
self.send_header("Content-Range", "bytes 0-75/2983")
|
||||
elif "/bug2295/1/" in self.path:
|
||||
self.send_response(206)
|
||||
self.send_header("Content-Range", "bytes 0-75/*")
|
||||
elif "/bug2295/2/" in self.path:
|
||||
self.send_response(206)
|
||||
self.send_header("Content-Range", "bytes 0-75/2983")
|
||||
self.send_header("Content-Length", "0")
|
||||
elif "/bug2295/00000012/0/" in self.path:
|
||||
self.send_response(206)
|
||||
self.send_header("Content-Range", "bytes 0-75/2983")
|
||||
self.send_header("Content-Length", "76")
|
||||
else:
|
||||
# Unknown request
|
||||
self.send_response(400)
|
||||
if "/reflect/" in self.path:
|
||||
self.reflect_headers()
|
||||
self.send_header("Content-type", "text/plain")
|
||||
self.end_headers()
|
||||
# No data
|
||||
else:
|
||||
# Normal response path
|
||||
data = data.copy() # we're going to modify
|
||||
# Ensure there's a "reply" key in data, even if there wasn't before
|
||||
data["reply"] = data.get("reply", llsd.LLSD("success"))
|
||||
response = llsd.format_xml(data)
|
||||
debug("success: %s", response)
|
||||
self.send_response(200)
|
||||
if "/reflect/" in self.path:
|
||||
self.reflect_headers()
|
||||
self.send_header("Content-type", "application/llsd+xml")
|
||||
self.send_header("Content-Length", str(len(response)))
|
||||
self.send_header("X-LL-Special", "Mememememe");
|
||||
self.end_headers()
|
||||
if withdata:
|
||||
self.wfile.write(response)
|
||||
|
||||
def reflect_headers(self):
|
||||
for name in self.headers.keys():
|
||||
|
|
|
|||
Loading…
Reference in New Issue