From 03db925988ed5defc2e925d1834059b3aed46ee9 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 10 May 2008 04:50:49 +0000 Subject: [PATCH] 2008-05-10 Tatsuhiro Tsujikawa Pool connection when a server returns 4xx, 5xx responses. * src/HttpNullDownloadCommand.cc * src/HttpNullDownloadCommand.h * src/HttpResponse.cc * src/HttpResponse.h * src/HttpResponseCommand.cc * src/HttpResponseCommand.h * test/HttpResponseTest.cc --- ChangeLog | 11 +++++++ src/HttpNullDownloadCommand.cc | 59 ++++++++++++++++++++++++++-------- src/HttpNullDownloadCommand.h | 2 ++ src/HttpResponse.cc | 19 ++++++----- src/HttpResponse.h | 5 +++ src/HttpResponseCommand.cc | 35 ++++++++------------ src/HttpResponseCommand.h | 2 +- test/HttpResponseTest.cc | 31 ++++++++---------- 8 files changed, 103 insertions(+), 61 deletions(-) diff --git a/ChangeLog b/ChangeLog index be5aa7ed..116f727b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,14 @@ +2008-05-10 Tatsuhiro Tsujikawa + + Pool connection when a server returns 4xx, 5xx responses. + * src/HttpNullDownloadCommand.cc + * src/HttpNullDownloadCommand.h + * src/HttpResponse.cc + * src/HttpResponse.h + * src/HttpResponseCommand.cc + * src/HttpResponseCommand.h + * test/HttpResponseTest.cc + 2008-05-10 Tatsuhiro Tsujikawa Print usage when no URL is specifed or bad command-line option is diff --git a/src/HttpNullDownloadCommand.cc b/src/HttpNullDownloadCommand.cc index a8f5098f..4ff448a9 100644 --- a/src/HttpNullDownloadCommand.cc +++ b/src/HttpNullDownloadCommand.cc @@ -44,6 +44,9 @@ #include "Logger.h" #include "HttpRequest.h" #include "Segment.h" +#include "Util.h" +#include "StringFormat.h" +#include "DlAbortEx.h" namespace aria2 { @@ -72,21 +75,30 @@ void HttpNullDownloadCommand::setTransferDecoder bool HttpNullDownloadCommand::executeInternal() { + if(_totalLength == 0 && _transferDecoder.isNull()) { + return processResponse(); + } const size_t BUFSIZE = 16*1024; unsigned char buf[BUFSIZE]; size_t bufSize = BUFSIZE; - socket->readData(buf, bufSize); - if(_transferDecoder.isNull()) { - _receivedBytes += bufSize; - } else { - // _receivedBytes is not updated if transferEncoding is set. - size_t infbufSize = 16*1024; - unsigned char infbuf[infbufSize]; - _transferDecoder->inflate(infbuf, infbufSize, buf, bufSize); - } - if(_totalLength != 0 && bufSize == 0) { - throw DlRetryEx(EX_GOT_EOF); + try { + socket->readData(buf, bufSize); + + if(_transferDecoder.isNull()) { + _receivedBytes += bufSize; + } else { + // _receivedBytes is not updated if transferEncoding is set. + size_t infbufSize = 16*1024; + unsigned char infbuf[infbufSize]; + _transferDecoder->inflate(infbuf, infbufSize, buf, bufSize); + } + if(_totalLength != 0 && bufSize == 0) { + throw DlRetryEx(EX_GOT_EOF); + } + } catch(RecoverableException& e) { + logger->debug(EX_EXCEPTION_CAUGHT, e); + return processResponse(); } if(bufSize == 0) { @@ -103,13 +115,32 @@ bool HttpNullDownloadCommand::executeInternal() socket->getPeerInfo(peerInfo); e->poolSocket(peerInfo.first, peerInfo.second, socket); } - _httpResponse->processRedirect(); - logger->info(MSG_REDIRECT, cuid, _httpResponse->getRedirectURI().c_str()); - return prepareForRetry(0); + return processResponse(); } else { e->commands.push_back(this); return false; } } +bool HttpNullDownloadCommand::processResponse() +{ + if(_httpResponse->isRedirect()) { + _httpResponse->processRedirect(); + logger->info(MSG_REDIRECT, cuid, _httpResponse->getRedirectURI().c_str()); + return prepareForRetry(0); + } else if(_httpResponse->hasRetryAfter()) { + return prepareForRetry(_httpResponse->getRetryAfter()); + } else if(_httpResponse->getResponseStatus() >= "400") { + if(_httpResponse->getResponseStatus() == "401") { + throw DlAbortEx(EX_AUTH_FAILED); + }else if(_httpResponse->getResponseStatus() == "404") { + throw DlAbortEx(MSG_RESOURCE_NOT_FOUND); + } else { + throw DlAbortEx(StringFormat(EX_BAD_STATUS, Util::parseUInt(_httpResponse->getResponseStatus())).str()); + } + } else { + return prepareForRetry(0); + } +} + } // namespace aria2 diff --git a/src/HttpNullDownloadCommand.h b/src/HttpNullDownloadCommand.h index 8860ee4a..dd9678a3 100644 --- a/src/HttpNullDownloadCommand.h +++ b/src/HttpNullDownloadCommand.h @@ -54,6 +54,8 @@ private: uint64_t _totalLength; uint64_t _receivedBytes; + + bool processResponse(); protected: virtual bool executeInternal(); public: diff --git a/src/HttpResponse.cc b/src/HttpResponse.cc index 2b14ea45..265cc9eb 100644 --- a/src/HttpResponse.cc +++ b/src/HttpResponse.cc @@ -59,15 +59,8 @@ HttpResponse::~HttpResponse() {} void HttpResponse::validateResponse() const { const std::string& status = getResponseStatus(); - if(status == "401") { - throw DlAbortEx(EX_AUTH_FAILED); - } - if(status == "404") { - throw DlAbortEx(MSG_RESOURCE_NOT_FOUND); - } if(status >= "400") { - throw DlAbortEx - (StringFormat(EX_BAD_STATUS, Util::parseUInt(status)).str()); + return; } if(status >= "300") { if(!httpHeader->defined("Location")) { @@ -207,4 +200,14 @@ const std::string& HttpResponse::getResponseStatus() const return httpHeader->getResponseStatus(); } +bool HttpResponse::hasRetryAfter() const +{ + return httpHeader->defined("Retry-After"); +} + +time_t HttpResponse::getRetryAfter() const +{ + return httpHeader->getFirstAsUInt("Retry-After"); +} + } // namespace aria2 diff --git a/src/HttpResponse.h b/src/HttpResponse.h index 04ea1153..3aeac22a 100644 --- a/src/HttpResponse.h +++ b/src/HttpResponse.h @@ -37,6 +37,7 @@ #include "common.h" #include "SharedHandle.h" +#include "a2time.h" #include namespace aria2 { @@ -104,6 +105,10 @@ public: { this->cuid = cuid; } + + bool hasRetryAfter() const; + + time_t getRetryAfter() const; }; typedef SharedHandle HttpResponseHandle; diff --git a/src/HttpResponseCommand.cc b/src/HttpResponseCommand.cc index 55c2a4fb..efe070da 100644 --- a/src/HttpResponseCommand.cc +++ b/src/HttpResponseCommand.cc @@ -87,27 +87,11 @@ bool HttpResponseCommand::executeInternal() // check HTTP status number httpResponse->validateResponse(); httpResponse->retrieveCookie(); - // check whether Location header exists. If it does, update request object - // with redirected URL. - if(httpResponse->isRedirect()) { - // To reuse a connection, a response body must be received. - if(req->supportsPersistentConnection() && - (httpResponse->getEntityLength() > 0 || - httpResponse->isTransferEncodingSpecified())) { - return handleRedirect(httpResponse); - } else { - // Response body is 0 length or a response header shows that a persistent - // connection is not enabled. - if(req->supportsPersistentConnection()) { - std::pair peerInfo; - socket->getPeerInfo(peerInfo); - e->poolSocket(peerInfo.first, peerInfo.second, socket); - } - httpResponse->processRedirect(); - logger->info(MSG_REDIRECT, cuid, httpResponse->getRedirectURI().c_str()); - return prepareForRetry(0); - } + + if(httpResponse->getResponseStatus() >= "300") { + return skipResponseBody(httpResponse); } + if(!_requestGroup->isSingleHostMultiConnectionEnabled()) { _requestGroup->removeURIWhoseHostnameIs(_requestGroup->searchServerHost(cuid)->getHostname()); } @@ -209,13 +193,22 @@ static SharedHandle getTransferEncoding return enc; } -bool HttpResponseCommand::handleRedirect +bool HttpResponseCommand::skipResponseBody (const SharedHandle& httpResponse) { SharedHandle enc(getTransferEncoding(httpResponse)); HttpNullDownloadCommand* command = new HttpNullDownloadCommand (cuid, req, _requestGroup, httpConnection, httpResponse, e, socket); command->setTransferDecoder(enc); + + // If the response body is zero-length, set command's status to real time + // so that avoid read check blocking + if(httpResponse->getEntityLength() == 0 && + !httpResponse->isTransferEncodingSpecified()) { + command->setStatusRealtime(); + e->setNoWait(true); + } + e->commands.push_back(command); return true; } diff --git a/src/HttpResponseCommand.h b/src/HttpResponseCommand.h index 19940264..0a30d08f 100644 --- a/src/HttpResponseCommand.h +++ b/src/HttpResponseCommand.h @@ -50,7 +50,7 @@ private: bool handleDefaultEncoding(const SharedHandle& httpResponse); bool handleOtherEncoding(const SharedHandle& httpResponse); - bool handleRedirect(const SharedHandle& httpResponse); + bool skipResponseBody(const SharedHandle& httpResponse); HttpDownloadCommand* createHttpDownloadCommand(const SharedHandle& httpResponse); protected: diff --git a/test/HttpResponseTest.cc b/test/HttpResponseTest.cc index 4d52cdbe..2308ea24 100644 --- a/test/HttpResponseTest.cc +++ b/test/HttpResponseTest.cc @@ -33,6 +33,7 @@ class HttpResponseTest : public CppUnit::TestFixture { CPPUNIT_TEST(testValidateResponse_good_range); CPPUNIT_TEST(testValidateResponse_bad_range); CPPUNIT_TEST(testValidateResponse_chunked); + CPPUNIT_TEST(testHasRetryAfter); CPPUNIT_TEST_SUITE_END(); private: @@ -57,6 +58,7 @@ public: void testValidateResponse_good_range(); void testValidateResponse_bad_range(); void testValidateResponse_chunked(); + void testHasRetryAfter(); }; @@ -241,25 +243,9 @@ void HttpResponseTest::testGetTransferDecoder() void HttpResponseTest::testValidateResponse() { HttpResponse httpResponse; - SharedHandle httpHeader(new HttpHeader()); - httpHeader->setResponseStatus("401"); httpResponse.setHttpHeader(httpHeader); - try { - httpResponse.validateResponse(); - CPPUNIT_FAIL("exception must be thrown."); - } catch(Exception& e) { - } - - httpHeader->setResponseStatus("505"); - - try { - httpResponse.validateResponse(); - CPPUNIT_FAIL("exception must be thrown."); - } catch(Exception& e) { - } - httpHeader->setResponseStatus("304"); try { @@ -269,7 +255,6 @@ void HttpResponseTest::testValidateResponse() } httpHeader->put("Location", "http://localhost/archives/aria2-1.0.0.tar.bz2"); - try { httpResponse.validateResponse(); } catch(Exception& e) { @@ -352,4 +337,16 @@ void HttpResponseTest::testValidateResponse_chunked() } } +void HttpResponseTest::testHasRetryAfter() +{ + HttpResponse httpResponse; + SharedHandle httpHeader(new HttpHeader()); + httpResponse.setHttpHeader(httpHeader); + + httpHeader->put("Retry-After", "60"); + + CPPUNIT_ASSERT(httpResponse.hasRetryAfter()); + CPPUNIT_ASSERT_EQUAL((time_t)60, httpResponse.getRetryAfter()); +} + } // namespace aria2