From f5f51d3cda8861b3b3a380cc96aaca98e572c377 Mon Sep 17 00:00:00 2001 From: Monty Brandenberg Date: Sat, 7 Jul 2012 19:35:32 -0400 Subject: SH-3185 Fill in some FIXME/TODO cases Also added some comments and changed the callback userdata argument to be an HttpOpRequest rather than a libcurl handle. Less code, less clutter. --- indra/llcorehttp/_httpoprequest.cpp | 62 +++++++++++++++++++++---------------- 1 file changed, 36 insertions(+), 26 deletions(-) (limited to 'indra/llcorehttp/_httpoprequest.cpp') diff --git a/indra/llcorehttp/_httpoprequest.cpp b/indra/llcorehttp/_httpoprequest.cpp index a19b6bd0dc..bec82d8449 100644 --- a/indra/llcorehttp/_httpoprequest.cpp +++ b/indra/llcorehttp/_httpoprequest.cpp @@ -344,6 +344,13 @@ void HttpOpRequest::setupCommon(HttpRequest::policy_t policy_id, } +// Sets all libcurl options and data for a request. +// +// Used both for initial requests and to 'reload' for +// a retry, generally with a different CURL handle. +// Junk may be left around from a failed request and that +// needs to be cleaned out. +// HttpStatus HttpOpRequest::prepareRequest(HttpService * service) { // Scrub transport and result data for retried op case @@ -387,20 +394,29 @@ HttpStatus HttpOpRequest::prepareRequest(HttpService * service) if (ENABLE_LINKSYS_WRT54G_V5_DNS_FIX) { + // The Linksys WRT54G V5 router has an issue with frequent + // DNS lookups from LAN machines. If they happen too often, + // like for every HTTP request, the router gets annoyed after + // about 700 or so requests and starts issuing TCP RSTs to + // new connections. Reuse the DNS lookups for even a few + // seconds and no RSTs. curl_easy_setopt(mCurlHandle, CURLOPT_DNS_CACHE_TIMEOUT, 10); } else { - // *FIXME: Revisit this old DNS timeout setting - may no longer be valid + // *TODO: Revisit this old DNS timeout setting - may no longer be valid + // I don't think this is valid anymore, the Multi shared DNS + // cache is working well. For the case of naked easy handles, + // consider using a shared DNS object. curl_easy_setopt(mCurlHandle, CURLOPT_DNS_CACHE_TIMEOUT, 0); } curl_easy_setopt(mCurlHandle, CURLOPT_AUTOREFERER, 1); curl_easy_setopt(mCurlHandle, CURLOPT_FOLLOWLOCATION, 1); - curl_easy_setopt(mCurlHandle, CURLOPT_MAXREDIRS, DEFAULT_HTTP_REDIRECTS); // *FIXME: parameterize this later + curl_easy_setopt(mCurlHandle, CURLOPT_MAXREDIRS, DEFAULT_HTTP_REDIRECTS); curl_easy_setopt(mCurlHandle, CURLOPT_WRITEFUNCTION, writeCallback); - curl_easy_setopt(mCurlHandle, CURLOPT_WRITEDATA, mCurlHandle); + curl_easy_setopt(mCurlHandle, CURLOPT_WRITEDATA, this); curl_easy_setopt(mCurlHandle, CURLOPT_READFUNCTION, readCallback); - curl_easy_setopt(mCurlHandle, CURLOPT_READDATA, mCurlHandle); + curl_easy_setopt(mCurlHandle, CURLOPT_READDATA, this); curl_easy_setopt(mCurlHandle, CURLOPT_SSL_VERIFYPEER, 1); curl_easy_setopt(mCurlHandle, CURLOPT_SSL_VERIFYHOST, 0); @@ -468,7 +484,9 @@ HttpStatus HttpOpRequest::prepareRequest(HttpService * service) break; default: - // *FIXME: fail out here + LL_ERRS("CoreHttp") << "Invalid HTTP method in request: " + << int(mReqMethod) << ". Can't recover." + << LL_ENDL; break; } @@ -476,7 +494,7 @@ HttpStatus HttpOpRequest::prepareRequest(HttpService * service) if (mTracing >= TRACE_CURL_HEADERS) { curl_easy_setopt(mCurlHandle, CURLOPT_VERBOSE, 1); - curl_easy_setopt(mCurlHandle, CURLOPT_DEBUGDATA, mCurlHandle); + curl_easy_setopt(mCurlHandle, CURLOPT_DEBUGDATA, this); curl_easy_setopt(mCurlHandle, CURLOPT_DEBUGFUNCTION, debugCallback); } @@ -524,7 +542,7 @@ HttpStatus HttpOpRequest::prepareRequest(HttpService * service) if (mProcFlags & (PF_SCAN_RANGE_HEADER | PF_SAVE_HEADERS)) { curl_easy_setopt(mCurlHandle, CURLOPT_HEADERFUNCTION, headerCallback); - curl_easy_setopt(mCurlHandle, CURLOPT_HEADERDATA, mCurlHandle); + curl_easy_setopt(mCurlHandle, CURLOPT_HEADERDATA, this); } if (status) @@ -537,10 +555,7 @@ HttpStatus HttpOpRequest::prepareRequest(HttpService * service) size_t HttpOpRequest::writeCallback(void * data, size_t size, size_t nmemb, void * userdata) { - CURL * handle(static_cast(userdata)); - HttpOpRequest * op(NULL); - curl_easy_getinfo(handle, CURLINFO_PRIVATE, &op); - // *FIXME: check the pointer + HttpOpRequest * op(static_cast(userdata)); if (! op->mReplyBody) { @@ -554,10 +569,7 @@ size_t HttpOpRequest::writeCallback(void * data, size_t size, size_t nmemb, void size_t HttpOpRequest::readCallback(void * data, size_t size, size_t nmemb, void * userdata) { - CURL * handle(static_cast(userdata)); - HttpOpRequest * op(NULL); - curl_easy_getinfo(handle, CURLINFO_PRIVATE, &op); - // *FIXME: check the pointer + HttpOpRequest * op(static_cast(userdata)); if (! op->mReqBody) { @@ -567,7 +579,8 @@ size_t HttpOpRequest::readCallback(void * data, size_t size, size_t nmemb, void const size_t body_size(op->mReqBody->size()); if (body_size <= op->mCurlBodyPos) { - // *FIXME: should probably log this event - unexplained + LL_WARNS("HttpCore") << "Request body position beyond body size. Aborting request." + << LL_ENDL; return 0; } @@ -586,10 +599,7 @@ size_t HttpOpRequest::headerCallback(void * data, size_t size, size_t nmemb, voi static const char con_ran_line[] = "content-range:"; static const size_t con_ran_line_len = sizeof(con_ran_line) - 1; - CURL * handle(static_cast(userdata)); - HttpOpRequest * op(NULL); - curl_easy_getinfo(handle, CURLINFO_PRIVATE, &op); - // *FIXME: check the pointer + HttpOpRequest * op(static_cast(userdata)); const size_t hdr_size(size * nmemb); const char * hdr_data(static_cast(data)); // Not null terminated @@ -609,7 +619,7 @@ size_t HttpOpRequest::headerCallback(void * data, size_t size, size_t nmemb, voi } else if (op->mProcFlags & PF_SCAN_RANGE_HEADER) { - char hdr_buffer[128]; + char hdr_buffer[128]; // Enough for a reasonable header size_t frag_size((std::min)(hdr_size, sizeof(hdr_buffer) - 1)); memcpy(hdr_buffer, hdr_data, frag_size); @@ -638,8 +648,10 @@ size_t HttpOpRequest::headerCallback(void * data, size_t size, size_t nmemb, voi else { // Ignore the unparsable. - // *FIXME: Maybe issue a warning into the log here - ; + LL_INFOS_ONCE("CoreHttp") << "Problem parsing odd Content-Range header: '" + << std::string(hdr_data, frag_size) + << "'. Ignoring." + << LL_ENDL; } } } @@ -668,9 +680,7 @@ size_t HttpOpRequest::headerCallback(void * data, size_t size, size_t nmemb, voi int HttpOpRequest::debugCallback(CURL * handle, curl_infotype info, char * buffer, size_t len, void * userdata) { - HttpOpRequest * op(NULL); - curl_easy_getinfo(handle, CURLINFO_PRIVATE, &op); - // *FIXME: check the pointer + HttpOpRequest * op(static_cast(userdata)); std::string safe_line; std::string tag; -- cgit v1.2.3