Large changes to the LLCurl::Responder API, as well as pulling in some changes to common libraries from the server codebase:

* Additional error checking in http handlers.
* Uniform log spam for http errors.
* Switch to using constants for http heads and status codes.
* Fixed bugs in incorrectly checking if parsing LLSD xml resulted in an error.
* Reduced spam regarding LLSD parsing errors in the default completedRaw http handler.  It should not longer be necessary to short-circuit completedRaw to avoid spam.
* Ported over a few bug fixes from the server code.
* Switch mode http status codes to use S32 instead of U32.
* Ported LLSD::asStringRef from server code; avoids copying strings all over the place.
* Ported server change to LLSD::asBinary; this always returns a reference now instead of copying the entire binary blob.
* Ported server pretty notation format (and pretty binary format) to llsd serialization.

* The new LLCurl::Responder API no longer has two error handlers to choose from.  Overriding the following methods have been deprecated:
** error - use httpFailure
** errorWithContent - use httpFailure
** result - use httpSuccess
** completed - use httpCompleted
** completedHeader - no longer necessary; call getResponseHeaders() from a completion method to obtain these headers.

* In order to 'catch' a completed http request, override one of these methods:
** httpSuccess - Called for any 2xx status code.
** httpFailure - Called for any non-2xx status code.
** httpComplete - Called for all status codes.  Default implementation is to call either httpSuccess or httpFailure.
* It is recommended to keep these methods protected/private in order to avoid triggering of these methods without using a 'push' method (see below).

* Uniform error handling should followed whenever possible by calling a variant of this during httpFailure:
** llwarns << dumpResponse() << llendl;
* Be sure to include LOG_CLASS(your_class_name) in your class in order for the log entry to give more context.

* In order to 'push' a result into the responder, you should no longer call error, errorWithContent, result, or completed.
* Nor should you directly call httpSuccess/Failure/Completed (unless passing a message up to a parent class).
* Instead, you can set the internal content of a responder and trigger a corresponding method using the following methods:
** successResult - Sets results and calls httpSuccess
** failureResult - Sets results and calls httpFailure
** completedResult - Sets results and calls httpCompleted

* To obtain information about a the response from a reponder method, use the following getters:
** getStatus - HTTP status code
** getReason - Reason string
** getContent - Content (Parsed body LLSD)
** getResponseHeaders - Response Headers (LLSD map)
** getHTTPMethod - HTTP method of the request
** getURL - URL of the request

* It is still possible to override completeRaw if you want to manipulate data directly out of LLPumpIO.

* See indra/llmessage/llcurl.h for more information.

1 files changed, 72 insertions, 65 deletions

diff --git a/indra/llmessage/llhttpclient.cpp b/indra/llmessage/llhttpclient.cpp
index 0c325a68aa..a4a1f02cd3 100644
--- a/indra/llmessage/llhttpclient.cpp
+++ b/indra/llmessage/llhttpclient.cpp
@@ -54,7 +54,7 @@ namespace
 	{
 	public:
 		LLHTTPClientURLAdaptor(LLCurl::ResponderPtr responder)
-			: LLURLRequestComplete(), mResponder(responder), mStatus(499),
+			: LLURLRequestComplete(), mResponder(responder), mStatus(HTTP_INTERNAL_ERROR),
 			  mReason("LLURLRequest complete w/no status")
 		{
 		}
@@ -63,7 +63,7 @@ namespace
 		{
 		}
 
-		virtual void httpStatus(U32 status, const std::string& reason)
+		virtual void httpStatus(S32 status, const std::string& reason)
 		{
 			LLURLRequestComplete::httpStatus(status,reason);
 
@@ -74,30 +74,33 @@ namespace
 		virtual void complete(const LLChannelDescriptors& channels,
 							  const buffer_ptr_t& buffer)
 		{
+			// *TODO: Re-interpret mRequestStatus codes?
+			//        Would like to detect curl errors, such as
+			//        connection errors, write erros, etc.
 			if (mResponder.get())
 			{
-				// Allow clients to parse headers before we attempt to parse
-				// the body and provide completed/result/error calls.
-				mResponder->completedHeader(mStatus, mReason, mHeaderOutput);
-				mResponder->completedRaw(mStatus, mReason, channels, buffer);
+				mResponder->setResult(mStatus, mReason);
+				mResponder->completedRaw(channels, buffer);
 			}
 		}
 		virtual void header(const std::string& header, const std::string& value)
 		{
-			mHeaderOutput[header] = value;
+			if (mResponder.get())
+			{
+				mResponder->setResponseHeader(header, value);
+			}
 		}
 
 	private:
 		LLCurl::ResponderPtr mResponder;
-		U32 mStatus;
+		S32 mStatus;
 		std::string mReason;
-		LLSD mHeaderOutput;
 	};
 	
 	class Injector : public LLIOPipe
 	{
 	public:
-		virtual const char* contentType() = 0;
+		virtual const std::string& contentType() = 0;
 	};
 
 	class LLSDInjector : public Injector
@@ -106,7 +109,7 @@ namespace
 		LLSDInjector(const LLSD& sd) : mSD(sd) {}
 		virtual ~LLSDInjector() {}
 
-		const char* contentType() { return "application/llsd+xml"; }
+		const std::string& contentType() { return HTTP_CONTENT_LLSD_XML; }
 
 		virtual EStatus process_impl(const LLChannelDescriptors& channels,
 			buffer_ptr_t& buffer, bool& eos, LLSD& context, LLPumpIO* pump)
@@ -126,7 +129,7 @@ namespace
 		RawInjector(const U8* data, S32 size) : mData(data), mSize(size) {}
 		virtual ~RawInjector() {delete mData;}
 
-		const char* contentType() { return "application/octet-stream"; }
+		const std::string& contentType() { return HTTP_CONTENT_OCTET_STREAM; }
 
 		virtual EStatus process_impl(const LLChannelDescriptors& channels,
 			buffer_ptr_t& buffer, bool& eos, LLSD& context, LLPumpIO* pump)
@@ -147,7 +150,7 @@ namespace
 		FileInjector(const std::string& filename) : mFilename(filename) {}
 		virtual ~FileInjector() {}
 
-		const char* contentType() { return "application/octet-stream"; }
+		const std::string& contentType() { return HTTP_CONTENT_OCTET_STREAM; }
 
 		virtual EStatus process_impl(const LLChannelDescriptors& channels,
 			buffer_ptr_t& buffer, bool& eos, LLSD& context, LLPumpIO* pump)
@@ -180,7 +183,7 @@ namespace
 		VFileInjector(const LLUUID& uuid, LLAssetType::EType asset_type) : mUUID(uuid), mAssetType(asset_type) {}
 		virtual ~VFileInjector() {}
 
-		const char* contentType() { return "application/octet-stream"; }
+		const std::string& contentType() { return HTTP_CONTENT_OCTET_STREAM; }
 
 		virtual EStatus process_impl(const LLChannelDescriptors& channels,
 			buffer_ptr_t& buffer, bool& eos, LLSD& context, LLPumpIO* pump)
@@ -213,7 +216,7 @@ void LLHTTPClient::setCertVerifyCallback(LLURLRequest::SSLCertVerifyCallback cal
 
 static void request(
 	const std::string& url,
-	LLURLRequest::ERequestAction method,
+	EHTTPMethod method,
 	Injector* body_injector,
 	LLCurl::ResponderPtr responder,
 	const F32 timeout = HTTP_REQUEST_EXPIRY_SECS,
@@ -222,7 +225,11 @@ static void request(
 {
 	if (!LLHTTPClient::hasPump())
 	{
-		responder->completed(U32_MAX, "No pump", LLSD());
+		if (responder)
+		{
+			responder->completeResult(HTTP_INTERNAL_ERROR, "No pump");
+		}
+		delete body_injector;
 		return;
 	}
 	LLPumpIO::chain_t chain;
@@ -230,20 +237,24 @@ static void request(
 	LLURLRequest* req = new LLURLRequest(method, url);
 	if(!req->isValid())//failed
 	{
-		delete req ;
-		return ;
+		if (responder)
+		{
+			responder->completeResult(HTTP_INTERNAL_CURL_ERROR, "Internal Error - curl failure");
+		}
+		delete req;
+		delete body_injector;
+		return;
 	}
 
 	req->setSSLVerifyCallback(LLHTTPClient::getCertVerifyCallback(), (void *)req);
 
 	
-	lldebugs << LLURLRequest::actionAsVerb(method) << " " << url << " "
-		<< headers << llendl;
+	LL_DEBUGS("LLHTTPClient") << httpMethodAsVerb(method) << " " << url << " " << headers << LL_ENDL;
 
 	// Insert custom headers if the caller sent any
 	if (headers.isMap())
 	{
-		if (headers.has("Cookie"))
+		if (headers.has(HTTP_HEADER_COOKIE))
 		{
 			req->allowCookies();
 		}
@@ -253,62 +264,56 @@ static void request(
 
         for (; iter != end; ++iter)
         {
-            std::ostringstream header;
             //if the header is "Pragma" with no value
             //the caller intends to force libcurl to drop
             //the Pragma header it so gratuitously inserts
             //Before inserting the header, force libcurl
             //to not use the proxy (read: llurlrequest.cpp)
-			static const std::string PRAGMA("Pragma");
-			if ((iter->first == PRAGMA) && (iter->second.asString().empty()))
+			if ((iter->first == HTTP_HEADER_PRAGMA) && (iter->second.asString().empty()))
             {
                 req->useProxy(false);
             }
-            header << iter->first << ": " << iter->second.asString() ;
-            lldebugs << "header = " << header.str() << llendl;
-            req->addHeader(header.str().c_str());
+            LL_DEBUGS("LLHTTPClient") << "header = " << iter->first 
+                << ": " << iter->second.asString() << LL_ENDL;
+            req->addHeader(iter->first, iter->second.asString());
         }
     }
 
 	// Check to see if we have already set Accept or not. If no one
 	// set it, set it to application/llsd+xml since that's what we
 	// almost always want.
-	if( method != LLURLRequest::HTTP_PUT && method != LLURLRequest::HTTP_POST )
+	if( method != HTTP_PUT && method != HTTP_POST )
 	{
-		static const std::string ACCEPT("Accept");
-		if(!headers.has(ACCEPT))
+		if(!headers.has(HTTP_HEADER_ACCEPT))
 		{
-			req->addHeader("Accept: application/llsd+xml");
+			req->addHeader(HTTP_HEADER_ACCEPT, HTTP_CONTENT_LLSD_XML);
 		}
 	}
 
 	if (responder)
 	{
 		responder->setURL(url);
+		responder->setHTTPMethod(method);
 	}
 
 	req->setCallback(new LLHTTPClientURLAdaptor(responder));
 
-	if (method == LLURLRequest::HTTP_POST  &&  gMessageSystem)
+	if (method == HTTP_POST  &&  gMessageSystem)
 	{
-		req->addHeader(llformat("X-SecondLife-UDP-Listen-Port: %d",
-								gMessageSystem->mPort).c_str());
+		req->addHeader("X-SecondLife-UDP-Listen-Port", llformat("%d",
+								gMessageSystem->mPort));
    	}
 
-	if (method == LLURLRequest::HTTP_PUT || method == LLURLRequest::HTTP_POST)
+	if (method == HTTP_PUT || method == HTTP_POST)
 	{
-		static const std::string CONTENT_TYPE("Content-Type");
-		if(!headers.has(CONTENT_TYPE))
+		if(!headers.has(HTTP_HEADER_CONTENT_TYPE))
 		{
 			// If the Content-Type header was passed in, it has
 			// already been added as a header through req->addHeader
 			// in the loop above. We defer to the caller's wisdom, but
 			// if they did not specify a Content-Type, then ask the
 			// injector.
-			req->addHeader(
-				llformat(
-					"Content-Type: %s",
-					body_injector->contentType()).c_str());
+			req->addHeader(HTTP_HEADER_CONTENT_TYPE, body_injector->contentType());
 		}
    		chain.push_back(LLIOPipe::ptr_t(body_injector));
 	}
@@ -331,9 +336,9 @@ void LLHTTPClient::getByteRange(
 	if(offset > 0 || bytes > 0)
 	{
 		std::string range = llformat("bytes=%d-%d", offset, offset+bytes-1);
-		headers["Range"] = range;
+		headers[HTTP_HEADER_RANGE] = range;
 	}
-    request(url,LLURLRequest::HTTP_GET, NULL, responder, timeout, headers);
+    request(url,HTTP_GET, NULL, responder, timeout, headers);
 }
 
 void LLHTTPClient::head(
@@ -342,16 +347,16 @@ void LLHTTPClient::head(
 	const LLSD& headers,
 	const F32 timeout)
 {
-	request(url, LLURLRequest::HTTP_HEAD, NULL, responder, timeout, headers);
+	request(url, HTTP_HEAD, NULL, responder, timeout, headers);
 }
 
 void LLHTTPClient::get(const std::string& url, ResponderPtr responder, const LLSD& headers, const F32 timeout)
 {
-	request(url, LLURLRequest::HTTP_GET, NULL, responder, timeout, headers);
+	request(url, HTTP_GET, NULL, responder, timeout, headers);
 }
 void LLHTTPClient::getHeaderOnly(const std::string& url, ResponderPtr responder, const LLSD& headers, const F32 timeout)
 {
-	request(url, LLURLRequest::HTTP_HEAD, NULL, responder, timeout, headers);
+	request(url, HTTP_HEAD, NULL, responder, timeout, headers);
 }
 void LLHTTPClient::getHeaderOnly(const std::string& url, ResponderPtr responder, const F32 timeout)
 {
@@ -392,7 +397,7 @@ public:
 		return content;
 	}
 
-	std::string asString()
+	const std::string& asString()
 	{
 		return mBuffer;
 	}
@@ -421,7 +426,7 @@ private:
   */
 static LLSD blocking_request(
 	const std::string& url,
-	LLURLRequest::ERequestAction method,
+	EHTTPMethod method,
 	const LLSD& body,
 	const LLSD& headers = LLSD(),
 	const F32 timeout = 5
@@ -464,11 +469,11 @@ static LLSD blocking_request(
 	}
 	
 	// * Setup specific method / "verb" for the URI (currently only GET and POST supported + poppy)
-	if (method == LLURLRequest::HTTP_GET)
+	if (method == HTTP_GET)
 	{
 		curl_easy_setopt(curlp, CURLOPT_HTTPGET, 1);
 	}
-	else if (method == LLURLRequest::HTTP_POST)
+	else if (method == HTTP_POST)
 	{
 		curl_easy_setopt(curlp, CURLOPT_POST, 1);
 		//serialize to ostr then copy to str - need to because ostr ptr is unstable :(
@@ -477,18 +482,20 @@ static LLSD blocking_request(
 		body_str = ostr.str();
 		curl_easy_setopt(curlp, CURLOPT_POSTFIELDS, body_str.c_str());
 		//copied from PHP libs, correct?
-		headers_list = curl_slist_append(headers_list, "Content-Type: application/llsd+xml");
+		headers_list = curl_slist_append(headers_list, 
+				llformat("%s: %s", HTTP_HEADER_CONTENT_TYPE.c_str(), HTTP_CONTENT_LLSD_XML.c_str()).c_str());
 
 		// copied from llurlrequest.cpp
 		// it appears that apache2.2.3 or django in etch is busted. If
 		// we do not clear the expect header, we get a 500. May be
 		// limited to django/mod_wsgi.
-		headers_list = curl_slist_append(headers_list, "Expect:");
+		headers_list = curl_slist_append(headers_list, llformat("%s:", HTTP_HEADER_EXPECT.c_str()).c_str());
 	}
 	
 	// * Do the action using curl, handle results
 	lldebugs << "HTTP body: " << body_str << llendl;
-	headers_list = curl_slist_append(headers_list, "Accept: application/llsd+xml");
+	headers_list = curl_slist_append(headers_list,
+				llformat("%s: %s", HTTP_HEADER_ACCEPT.c_str(), HTTP_CONTENT_LLSD_XML.c_str()).c_str());
 	CURLcode curl_result = curl_easy_setopt(curlp, CURLOPT_HTTPHEADER, headers_list);
 	if ( curl_result != CURLE_OK )
 	{
@@ -497,11 +504,11 @@ static LLSD blocking_request(
 
 	LLSD response = LLSD::emptyMap();
 	S32 curl_success = curl_easy_perform(curlp);
-	S32 http_status = 499;
+	S32 http_status = HTTP_INTERNAL_ERROR;
 	curl_easy_getinfo(curlp, CURLINFO_RESPONSE_CODE, &http_status);
 	response["status"] = http_status;
 	// if we get a non-404 and it's not a 200 OR maybe it is but you have error bits,
-	if ( http_status != 404 && (http_status != 200 || curl_success != 0) )
+	if ( http_status != HTTP_NOT_FOUND && (http_status != HTTP_OK || curl_success != 0) )
 	{
 		// We expect 404s, don't spam for them.
 		llwarns << "CURL REQ URL: " << url << llendl;
@@ -531,12 +538,12 @@ static LLSD blocking_request(
 
 LLSD LLHTTPClient::blockingGet(const std::string& url)
 {
-	return blocking_request(url, LLURLRequest::HTTP_GET, LLSD());
+	return blocking_request(url, HTTP_GET, LLSD());
 }
 
 LLSD LLHTTPClient::blockingPost(const std::string& url, const LLSD& body)
 {
-	return blocking_request(url, LLURLRequest::HTTP_POST, body);
+	return blocking_request(url, HTTP_POST, body);
 }
 
 void LLHTTPClient::put(
@@ -546,7 +553,7 @@ void LLHTTPClient::put(
 	const LLSD& headers,
 	const F32 timeout)
 {
-	request(url, LLURLRequest::HTTP_PUT, new LLSDInjector(body), responder, timeout, headers);
+	request(url, HTTP_PUT, new LLSDInjector(body), responder, timeout, headers);
 }
 
 void LLHTTPClient::post(
@@ -556,7 +563,7 @@ void LLHTTPClient::post(
 	const LLSD& headers,
 	const F32 timeout)
 {
-	request(url, LLURLRequest::HTTP_POST, new LLSDInjector(body), responder, timeout, headers);
+	request(url, HTTP_POST, new LLSDInjector(body), responder, timeout, headers);
 }
 
 void LLHTTPClient::postRaw(
@@ -567,7 +574,7 @@ void LLHTTPClient::postRaw(
 	const LLSD& headers,
 	const F32 timeout)
 {
-	request(url, LLURLRequest::HTTP_POST, new RawInjector(data, size), responder, timeout, headers);
+	request(url, HTTP_POST, new RawInjector(data, size), responder, timeout, headers);
 }
 
 void LLHTTPClient::postFile(
@@ -577,7 +584,7 @@ void LLHTTPClient::postFile(
 	const LLSD& headers,
 	const F32 timeout)
 {
-	request(url, LLURLRequest::HTTP_POST, new FileInjector(filename), responder, timeout, headers);
+	request(url, HTTP_POST, new FileInjector(filename), responder, timeout, headers);
 }
 
 void LLHTTPClient::postFile(
@@ -588,7 +595,7 @@ void LLHTTPClient::postFile(
 	const LLSD& headers,
 	const F32 timeout)
 {
-	request(url, LLURLRequest::HTTP_POST, new VFileInjector(uuid, asset_type), responder, timeout, headers);
+	request(url, HTTP_POST, new VFileInjector(uuid, asset_type), responder, timeout, headers);
 }
 
 // static
@@ -598,7 +605,7 @@ void LLHTTPClient::del(
 	const LLSD& headers,
 	const F32 timeout)
 {
-	request(url, LLURLRequest::HTTP_DELETE, NULL, responder, timeout, headers);
+	request(url, HTTP_DELETE, NULL, responder, timeout, headers);
 }
 
 // static
@@ -610,8 +617,8 @@ void LLHTTPClient::move(
 	const F32 timeout)
 {
 	LLSD headers = hdrs;
-	headers["Destination"] = destination;
-	request(url, LLURLRequest::HTTP_MOVE, NULL, responder, timeout, headers);
+	headers[HTTP_HEADER_DESTINATION] = destination;
+	request(url, HTTP_MOVE, NULL, responder, timeout, headers);
 }