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.

2 files changed, 218 insertions, 18 deletions

diff --git a/indra/llcorehttp/tests/test_httprequest.hpp b/indra/llcorehttp/tests/test_httprequest.hpp
index e5488cf941..16f39845bb 100644
--- a/indra/llcorehttp/tests/test_httprequest.hpp
+++ b/indra/llcorehttp/tests/test_httprequest.hpp
@@ -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
 
diff --git a/indra/llcorehttp/tests/test_llcorehttp_peer.py b/indra/llcorehttp/tests/test_llcorehttp_peer.py
index 75a3c39ef2..7f8f765366 100644
--- a/indra/llcorehttp/tests/test_llcorehttp_peer.py
+++ b/indra/llcorehttp/tests/test_llcorehttp_peer.py
@@ -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():