Fine tune of private (libcwd) assertion.

It turns out that it's possible to receive data before an upload
finished when the server sends HTTP_BAD_REQUEST in the middle of the
upload (this happens to me when I try to upload a 6000x6000 image to my
profile feed, with is a 44MB file). So, in that case the finished upload
detection did not fail and we shouldn't assert.
This commit is contained in:
Aleric Inglewood
2013-01-11 01:28:46 +01:00
parent 5daf817e7e
commit 741a160913
3 changed files with 32 additions and 12 deletions

View File

@@ -1276,7 +1276,7 @@ static int const HTTP_REDIRECTS_DEFAULT = 10;
LLChannelDescriptors const BufferedCurlEasyRequest::sChannels;
BufferedCurlEasyRequest::BufferedCurlEasyRequest() : mRequestTransferedBytes(0), mResponseTransferedBytes(0), mBufferEventsTarget(NULL)
BufferedCurlEasyRequest::BufferedCurlEasyRequest() : mRequestTransferedBytes(0), mResponseTransferedBytes(0), mBufferEventsTarget(NULL), mStatus(HTTP_INTERNAL_ERROR)
{
AICurlInterface::Stats::BufferedCurlEasyRequest_count++;
}
@@ -1336,8 +1336,13 @@ void BufferedCurlEasyRequest::resetState(void)
// Call base class implementation.
CurlEasyRequest::resetState();
// Reset local variables.
mOutput.reset();
mInput.reset();
mRequestTransferedBytes = 0;
mResponseTransferedBytes = 0;
mBufferEventsTarget = NULL;
mStatus = HTTP_INTERNAL_ERROR;
}
void BufferedCurlEasyRequest::print_diagnostics(CURLcode code)

View File

@@ -88,7 +88,7 @@ class HTTPTimeout : public LLRefCount {
bool data_sent(size_t n);
// Called when data is received. Returns true if transfer timed out.
bool data_received(size_t n);
bool data_received(size_t n/*,*/ ASSERT_ONLY_COMMA(bool upload_error_status = false));
// Called immediately before done() after curl finished, with code.
void done(AICurlEasyRequest_wat const& curlEasyRequest_w, CURLcode code);
@@ -498,6 +498,9 @@ class BufferedCurlEasyRequest : public CurlEasyRequest {
// Return pointer to the ThreadSafe (wrapped) version of this object.
ThreadSafeBufferedCurlEasyRequest* get_lockobj(void);
ThreadSafeBufferedCurlEasyRequest const* get_lockobj(void) const;
// Return true when an error code was received that can occur before the upload finished.
// So far the only such error I've seen is HTTP_BAD_REQUEST.
bool upload_error_status(void) const { return mStatus == HTTP_BAD_REQUEST /*&& mStatus != HTTP_INTERNAL_ERROR*/; }
// Return true when prepRequest was already called and the object has not been
// invalidated as a result of calling timed_out().

View File

@@ -1871,7 +1871,13 @@ void HTTPTimeout::upload_finished(void)
// queued--><--DNS lookup + connect + send headers-->[<--send body (if any)-->]<--replydelay--><--receive headers + body--><--done
// ^ ^ ^ ^ ^ ^ ^ ^
// | | | | | | | |
bool HTTPTimeout::data_received(size_t n)
bool HTTPTimeout::data_received(size_t n/*,*/
#ifdef CWDEBUG
ASSERT_ONLY_COMMA(bool upload_error_status)
#else
ASSERT_ONLY_COMMA(bool)
#endif
)
{
// The HTTP header of the reply is the first thing we receive.
if (mNothingReceivedYet && n > 0)
@@ -1882,7 +1888,8 @@ bool HTTPTimeout::data_received(size_t n)
// because in that case it is impossible to detect the difference between connecting and waiting for a reply without
// using CURLOPT_DEBUGFUNCTION. Note that mDebugIsHeadOrGetMethod is only valid when the debug channel 'curlio' is on,
// because it is set in the debug callback function.
Debug(llassert(AICurlEasyRequest_wat(*mLockObj)->mDebugIsHeadOrGetMethod || !dc::curlio.is_on()));
// This is also normal if we received a HTTP header with an error status, since that can interrupt our upload.
Debug(llassert(upload_error_status || AICurlEasyRequest_wat(*mLockObj)->mDebugIsHeadOrGetMethod || !dc::curlio.is_on()));
// 'Upload finished' detection failed, generate it now.
upload_finished();
}
@@ -2395,23 +2402,18 @@ size_t BufferedCurlEasyRequest::curlHeaderCallback(char* data, size_t size, size
char const* const header_line = static_cast<char const*>(data);
size_t const header_len = size * nmemb;
if (self_w->httptimeout()->data_received(header_len)) // Update timeout administration.
{
// Transfer timed out. Return 0 which will abort with error CURLE_WRITE_ERROR.
return 0;
}
if (!header_len)
{
return header_len;
}
std::string header(header_line, header_len);
bool done = false;
if (!LLStringUtil::_isASCII(header))
{
return header_len;
done = true;
}
// Per HTTP spec the first header line must be the status line.
if (header.substr(0, 5) == "HTTP/")
else if (header.substr(0, 5) == "HTTP/")
{
std::string::iterator const begin = header.begin();
std::string::iterator const end = header.end();
@@ -2445,6 +2447,16 @@ size_t BufferedCurlEasyRequest::curlHeaderCallback(char* data, size_t size, size
}
self_w->received_HTTP_header();
self_w->setStatusAndReason(status, reason);
done = true;
}
// Update timeout administration. This must be done after the status is already known.
if (self_w->httptimeout()->data_received(header_len/*,*/ ASSERT_ONLY_COMMA(self_w->upload_error_status())))
{
// Transfer timed out. Return 0 which will abort with error CURLE_WRITE_ERROR.
return 0;
}
if (done)
{
return header_len;
}