diff --git a/ChangeLog b/ChangeLog index 48a20075..440dd9e5 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,19 @@ +2008-04-22 Tatsuhiro Tsujikawa + + Rewritten HTTP keep-alive and pipelining routine. + * src/AbstractCommand.cc + * src/HttpConnection.cc + * src/HttpDownloadCommand.cc + * src/HttpRequest.cc + * src/HttpRequestCommand.cc + * src/HttpResponseCommand.cc + * src/Request.cc + * src/Request.h + * src/RequestGroup.cc + * test/HttpRequestTest.cc + * test/HttpResponseTest.cc + * test/RequestTest.cc + 2008-04-21 Tatsuhiro Tsujikawa Added missing setUserHeaders call diff --git a/src/AbstractCommand.cc b/src/AbstractCommand.cc index e808e0db..4bd8d388 100644 --- a/src/AbstractCommand.cc +++ b/src/AbstractCommand.cc @@ -120,7 +120,7 @@ bool AbstractCommand::execute() { if(!_requestGroup->getPieceStorage().isNull()) { _segments = _requestGroup->getSegmentMan()->getInFlightSegment(cuid); size_t maxSegments; - if(req->isKeepAlive() && e->option->get(PREF_ENABLE_HTTP_PIPELINING) == V_TRUE) { + if(req->isPipeliningEnabled()) { maxSegments = e->option->getAsInt(PREF_MAX_HTTP_PIPELINING); } else { maxSegments = 1; diff --git a/src/HttpConnection.cc b/src/HttpConnection.cc index 02a9e600..08b3c01a 100644 --- a/src/HttpConnection.cc +++ b/src/HttpConnection.cc @@ -47,6 +47,7 @@ #include "HttpHeader.h" #include "Logger.h" #include "Socket.h" +#include "Option.h" #include namespace aria2 { @@ -136,7 +137,9 @@ HttpResponseHandle HttpConnection::receiveResponse() SharedHandle httpHeader = proc->getHttpResponseHeader(); if(Util::toLower(httpHeader->getFirst("Connection")).find("close") != std::string::npos) { - entry->getHttpRequest()->getRequest()->setKeepAlive(false); + entry->getHttpRequest()->getRequest()->supportsPersistentConnection(false); + } else { + entry->getHttpRequest()->getRequest()->supportsPersistentConnection(true); } HttpResponseHandle httpResponse(new HttpResponse()); diff --git a/src/HttpDownloadCommand.cc b/src/HttpDownloadCommand.cc index 23379ca1..104d0ae1 100644 --- a/src/HttpDownloadCommand.cc +++ b/src/HttpDownloadCommand.cc @@ -56,7 +56,7 @@ HttpDownloadCommand::HttpDownloadCommand(int cuid, HttpDownloadCommand::~HttpDownloadCommand() {} bool HttpDownloadCommand::prepareForNextSegment() { - if(!_requestGroup->downloadFinished() && req->isKeepAlive()) { + if(req->isPipeliningEnabled() && !_requestGroup->downloadFinished()) { Command* command = new HttpRequestCommand(cuid, req, _requestGroup, _httpConnection, e, socket); e->commands.push_back(command); return true; diff --git a/src/HttpRequest.cc b/src/HttpRequest.cc index da2db386..caf467a5 100644 --- a/src/HttpRequest.cc +++ b/src/HttpRequest.cc @@ -91,7 +91,7 @@ off_t HttpRequest::getEndByte() const if(segment.isNull() || request.isNull()) { return 0; } else { - if(request->isKeepAlive()) { + if(request->isPipeliningEnabled()) { return segment->getPosition()+segment->getLength()-1; } else { return 0; @@ -150,20 +150,20 @@ std::string HttpRequest::createRequest() const "Host: "+getHostText(getHost(), getPort())+"\r\n"+ "Pragma: no-cache\r\n"+ "Cache-Control: no-cache\r\n"; - if(!request->isKeepAlive()) { + if(!request->isKeepAliveEnabled() && !request->isPipeliningEnabled()) { requestLine += "Connection: close\r\n"; } - if(!segment.isNull() && segment->getLength() > 0 && - (request->isKeepAlive() || getStartByte() > 0)) { + if(!segment.isNull() && segment->getLength() > 0 && + (request->isPipeliningEnabled() || getStartByte() > 0)) { requestLine += "Range: bytes="+Util::itos(getStartByte()); requestLine += "-"; - if(request->isKeepAlive()) { + if(request->isPipeliningEnabled()) { requestLine += Util::itos(getEndByte()); } requestLine += "\r\n"; } if(proxyEnabled) { - if(request->isKeepAlive()) { + if(request->isKeepAliveEnabled() || request->isPipeliningEnabled()) { requestLine += "Proxy-Connection: Keep-Alive\r\n"; } else { requestLine += "Proxy-Connection: close\r\n"; @@ -208,7 +208,7 @@ std::string HttpRequest::createProxyRequest() const std::string(" HTTP/1.1\r\n")+ "User-Agent: "+userAgent+"\r\n"+ "Host: "+getHost()+":"+Util::uitos(getPort())+"\r\n"; - if(request->isKeepAlive()) { + if(request->isKeepAliveEnabled() || request->isPipeliningEnabled()) { requestLine += "Proxy-Connection: Keep-Alive\r\n"; }else { requestLine += "Proxy-Connection: close\r\n"; diff --git a/src/HttpRequestCommand.cc b/src/HttpRequestCommand.cc index 4f5bc131..cc76a9be 100644 --- a/src/HttpRequestCommand.cc +++ b/src/HttpRequestCommand.cc @@ -47,12 +47,13 @@ namespace aria2 { -HttpRequestCommand::HttpRequestCommand(int cuid, - const RequestHandle& req, - RequestGroup* requestGroup, - const HttpConnectionHandle& httpConnection, - DownloadEngine* e, - const SocketHandle& s) +HttpRequestCommand::HttpRequestCommand +(int cuid, + const RequestHandle& req, + RequestGroup* requestGroup, + const HttpConnectionHandle& httpConnection, + DownloadEngine* e, + const SocketHandle& s) :AbstractCommand(cuid, req, requestGroup, e, s), _httpConnection(httpConnection) { @@ -62,44 +63,41 @@ HttpRequestCommand::HttpRequestCommand(int cuid, HttpRequestCommand::~HttpRequestCommand() {} +static SharedHandle +createHttpRequest(const SharedHandle& req, + const SharedHandle& segment, + uint64_t totalLength, + const Option* option) +{ + HttpRequestHandle httpRequest(new HttpRequest()); + httpRequest->setUserAgent(option->get(PREF_USER_AGENT)); + httpRequest->setRequest(req); + httpRequest->setSegment(segment); + httpRequest->setEntityLength(totalLength); + httpRequest->setUserHeaders(option->get(PREF_HEADER)); + httpRequest->configure(option); + + return httpRequest; +} + bool HttpRequestCommand::executeInternal() { socket->setBlockingMode(); if(req->getProtocol() == "https") { socket->initiateSecureConnection(); } - if(e->option->get(PREF_ENABLE_HTTP_PIPELINING) == V_TRUE) { - req->setKeepAlive(true); - } else if(e->option->get(PREF_ENABLE_HTTP_KEEP_ALIVE) == V_TRUE && - !_requestGroup->getSegmentMan().isNull() && - _requestGroup->getSegmentMan()->countFreePieceFrom(_segments.front()->getIndex()+1) <= 4) { - // TODO Do we need to consider the case where content-length is unknown? - // TODO parameterize the value which enables keep-alive, '4' - req->setKeepAlive(true); - } else { - req->setKeepAlive(false); - } if(_segments.empty()) { - HttpRequestHandle httpRequest(new HttpRequest()); - httpRequest->setUserAgent(e->option->get(PREF_USER_AGENT)); - httpRequest->setRequest(req); - httpRequest->setEntityLength(_requestGroup->getTotalLength()); - httpRequest->setUserHeaders(e->option->get(PREF_HEADER)); - httpRequest->configure(e->option); - + HttpRequestHandle httpRequest + (createHttpRequest(req, SharedHandle(), + _requestGroup->getTotalLength(), e->option)); _httpConnection->sendRequest(httpRequest); } else { for(Segments::iterator itr = _segments.begin(); itr != _segments.end(); ++itr) { - SegmentHandle segment = *itr; + const SegmentHandle& segment = *itr; if(!_httpConnection->isIssued(segment)) { - HttpRequestHandle httpRequest(new HttpRequest()); - httpRequest->setUserAgent(e->option->get(PREF_USER_AGENT)); - httpRequest->setRequest(req); - httpRequest->setSegment(segment); - httpRequest->setEntityLength(_requestGroup->getTotalLength()); - httpRequest->setUserHeaders(e->option->get(PREF_HEADER)); - httpRequest->configure(e->option); - + HttpRequestHandle httpRequest + (createHttpRequest(req, segment, + _requestGroup->getTotalLength(), e->option)); _httpConnection->sendRequest(httpRequest); } } diff --git a/src/HttpResponseCommand.cc b/src/HttpResponseCommand.cc index 4e0f2870..997b17fc 100644 --- a/src/HttpResponseCommand.cc +++ b/src/HttpResponseCommand.cc @@ -164,8 +164,6 @@ bool HttpResponseCommand::handleOtherEncoding(const HttpResponseHandle& httpResp if(httpRequest->getMethod() == Request::METHOD_HEAD) { return true; } - // disable keep-alive - req->setKeepAlive(false); _requestGroup->initPieceStorage(); _requestGroup->shouldCancelDownloadForSafety(); _requestGroup->getPieceStorage()->getDiskAdaptor()->initAndOpenFile(); diff --git a/src/Request.cc b/src/Request.cc index f9b2a5bb..bc25f8af 100644 --- a/src/Request.cc +++ b/src/Request.cc @@ -46,8 +46,14 @@ const std::string Request::METHOD_GET = "get"; const std::string Request::METHOD_HEAD = "head"; -Request::Request():port(0), tryCount(0), keepAlive(false), method(METHOD_GET), - cookieBox(CookieBoxFactorySingletonHolder::instance()->createNewInstance()) {} +Request::Request(): + port(0), tryCount(0), + _supportsPersistentConnection(false), + _keepAliveHint(false), + _pipeliningHint(false), + method(METHOD_GET), + cookieBox(CookieBoxFactorySingletonHolder::instance()->createNewInstance()) +{} Request::~Request() {} @@ -63,7 +69,7 @@ bool Request::resetUrl() { bool Request::redirectUrl(const std::string& url) { previousUrl = ""; - keepAlive = false; + _supportsPersistentConnection = false; return parseUrl(url); } diff --git a/src/Request.h b/src/Request.h index 544babd4..9319b8da 100644 --- a/src/Request.h +++ b/src/Request.h @@ -53,14 +53,6 @@ namespace aria2 { class CookieBox; class Request { -public: - enum TRACKER_EVENT { - AUTO, - STARTED, - STOPPED, - COMPLETED, - AFTER_COMPLETED - }; private: std::string url; std::string currentUrl; @@ -78,8 +70,14 @@ private: std::string dir; std::string file; unsigned int tryCount; - TRACKER_EVENT trackerEvent; - bool keepAlive; + + // whether or not the server supports persistent connection + bool _supportsPersistentConnection; + // enable keep-alive if possible. + bool _keepAliveHint; + // enable pipelining if possible. + bool _pipeliningHint; + std::string method; std::string _username; @@ -121,10 +119,36 @@ public: uint16_t getPort() const { return port; } std::string getDir() const { return dir; } std::string getFile() const { return file;} - bool isKeepAlive() const { return keepAlive; } - void setKeepAlive(bool keepAlive) { this->keepAlive = keepAlive; } - void setTrackerEvent(TRACKER_EVENT event) { trackerEvent = event; } - TRACKER_EVENT getTrackerEvent() const { return trackerEvent; } + + void supportsPersistentConnection(bool f) + { + _supportsPersistentConnection = f; + } + + bool supportsPersistentConnection() + { + return _supportsPersistentConnection; + } + + bool isKeepAliveEnabled() const + { + return _supportsPersistentConnection && _keepAliveHint; + } + + void setKeepAliveHint(bool keepAliveHint) + { + _keepAliveHint = keepAliveHint; + } + + bool isPipeliningEnabled() + { + return _supportsPersistentConnection && _pipeliningHint; + } + + void setPipeliningHint(bool pipeliningHint) + { + _pipeliningHint = pipeliningHint; + } void setMethod(const std::string& method) { this->method = method; diff --git a/src/RequestGroup.cc b/src/RequestGroup.cc index 7f8c33bf..18ed9c23 100644 --- a/src/RequestGroup.cc +++ b/src/RequestGroup.cc @@ -457,6 +457,18 @@ Commands RequestGroup::createNextCommand(DownloadEngine* e, unsigned int numComm _spentUris.push_back(uri); req->setReferer(_option->get(PREF_REFERER)); req->setMethod(method); + + if(_option->getAsBool(PREF_ENABLE_HTTP_KEEP_ALIVE)) { + req->setKeepAliveHint(true); + } + if(_option->getAsBool(PREF_ENABLE_HTTP_PIPELINING)) { + req->setPipeliningHint(true); + } + + if(req->getProtocol() == "http" || req->getProtocol() == "https") { + req->supportsPersistentConnection(true); + } + Command* command = InitiateConnectionCommandFactory::createInitiateConnectionCommand(CUIDCounterSingletonHolder::instance()->newID(), req, this, e); ServerHostHandle sv(new ServerHost(command->getCuid(), req->getHost())); registerServerHost(sv); diff --git a/test/HttpRequestTest.cc b/test/HttpRequestTest.cc index a2ffb558..d514f3d4 100644 --- a/test/HttpRequestTest.cc +++ b/test/HttpRequestTest.cc @@ -74,7 +74,8 @@ void HttpRequestTest::testGetEndByte() CPPUNIT_ASSERT_EQUAL(0LL, httpRequest.getEndByte()); SharedHandle request(new Request()); - request->setKeepAlive(true); + request->supportsPersistentConnection(true); + request->setPipeliningHint(true); httpRequest.setRequest(request); @@ -82,7 +83,7 @@ void HttpRequestTest::testGetEndByte() httpRequest.getEndByte()); - request->setKeepAlive(false); + request->setPipeliningHint(false); CPPUNIT_ASSERT_EQUAL(0LL, httpRequest.getEndByte()); } @@ -105,6 +106,8 @@ void HttpRequestTest::testCreateRequest() SingletonHolder >::instance(authConfigFactory); SharedHandle request(new Request()); + request->supportsPersistentConnection(true); + request->setUrl("http://localhost:8080/archives/aria2-1.0.0.tar.bz2"); p.reset(new Piece(0, 1024)); @@ -114,7 +117,8 @@ void HttpRequestTest::testCreateRequest() httpRequest.setRequest(request); httpRequest.setSegment(segment); - request->setKeepAlive(true); + // remove "Connection: close" and add end byte range + request->setPipeliningHint(true); std::string expectedText = "GET /archives/aria2-1.0.0.tar.bz2 HTTP/1.1\r\n" "User-Agent: aria2\r\n" @@ -127,7 +131,7 @@ void HttpRequestTest::testCreateRequest() CPPUNIT_ASSERT_EQUAL(expectedText, httpRequest.createRequest()); - request->setKeepAlive(false); + request->setPipeliningHint(false); expectedText = "GET /archives/aria2-1.0.0.tar.bz2 HTTP/1.1\r\n" "User-Agent: aria2\r\n" @@ -156,7 +160,7 @@ void HttpRequestTest::testCreateRequest() CPPUNIT_ASSERT_EQUAL(expectedText, httpRequest.createRequest()); - request->setKeepAlive(true); + request->setPipeliningHint(true); expectedText = "GET /archives/aria2-1.0.0.tar.bz2 HTTP/1.1\r\n" "User-Agent: aria2\r\n" @@ -169,8 +173,7 @@ void HttpRequestTest::testCreateRequest() CPPUNIT_ASSERT_EQUAL(expectedText, httpRequest.createRequest()); - request->setKeepAlive(false); - + // redirection clears persistent connection falg request->redirectUrl("http://localhost:8080/archives/download/aria2-1.0.0.tar.bz2"); expectedText = "GET /archives/download/aria2-1.0.0.tar.bz2 HTTP/1.1\r\n" @@ -184,7 +187,26 @@ void HttpRequestTest::testCreateRequest() "\r\n"; CPPUNIT_ASSERT_EQUAL(expectedText, httpRequest.createRequest()); - + + request->supportsPersistentConnection(true); + request->setPipeliningHint(false); + + // this only removes "Connection: close". + request->setKeepAliveHint(true); + + expectedText = "GET /archives/download/aria2-1.0.0.tar.bz2 HTTP/1.1\r\n" + "User-Agent: aria2\r\n" + "Accept: */*\r\n" + "Host: localhost:8080\r\n" + "Pragma: no-cache\r\n" + "Cache-Control: no-cache\r\n" + "Range: bytes=1048576-\r\n" + "\r\n"; + + CPPUNIT_ASSERT_EQUAL(expectedText, httpRequest.createRequest()); + + request->setKeepAliveHint(false); + request->resetUrl(); p.reset(new Piece(0, 1024*1024)); @@ -259,7 +281,7 @@ void HttpRequestTest::testCreateRequest() CPPUNIT_ASSERT_EQUAL(expectedText, httpRequest.createRequest()); - request->setKeepAlive(true); + request->setPipeliningHint(true); expectedText = "GET http://localhost:8080/archives/aria2-1.0.0.tar.bz2 HTTP/1.1\r\n" "User-Agent: aria2\r\n" @@ -275,7 +297,7 @@ void HttpRequestTest::testCreateRequest() CPPUNIT_ASSERT_EQUAL(expectedText, httpRequest.createRequest()); - request->setKeepAlive(false); + request->setPipeliningHint(false); option.put(PREF_HTTP_PROXY_AUTH_ENABLED, V_FALSE); @@ -447,6 +469,8 @@ void HttpRequestTest::testCreateProxyRequest() httpRequest.setRequest(request); httpRequest.setSegment(segment); + request->supportsPersistentConnection(true); + std::string expectedText = "CONNECT localhost:80 HTTP/1.1\r\n" "User-Agent: aria2\r\n" "Host: localhost:80\r\n" @@ -455,7 +479,8 @@ void HttpRequestTest::testCreateProxyRequest() CPPUNIT_ASSERT_EQUAL(expectedText, httpRequest.createProxyRequest()); - request->setKeepAlive(true); + // adds Keep-Alive header. + request->setKeepAliveHint(true); expectedText = "CONNECT localhost:80 HTTP/1.1\r\n" "User-Agent: aria2\r\n" @@ -464,14 +489,26 @@ void HttpRequestTest::testCreateProxyRequest() "\r\n"; CPPUNIT_ASSERT_EQUAL(expectedText, httpRequest.createProxyRequest()); - + + request->setKeepAliveHint(false); + // pipelining also adds Keep-Alive header. + request->setPipeliningHint(true); + + expectedText = "CONNECT localhost:80 HTTP/1.1\r\n" + "User-Agent: aria2\r\n" + "Host: localhost:80\r\n" + "Proxy-Connection: Keep-Alive\r\n" + "\r\n"; + + CPPUNIT_ASSERT_EQUAL(expectedText, httpRequest.createProxyRequest()); } void HttpRequestTest::testIsRangeSatisfied() { SharedHandle request(new Request()); + request->supportsPersistentConnection(true); request->setUrl("http://localhost:8080/archives/aria2-1.0.0.tar.bz2"); - request->setKeepAlive(false); + request->setPipeliningHint(false); // default: false SharedHandle p(new Piece(0, 1024*1024)); SharedHandle segment(new PiecedSegment(1024*1024, p)); @@ -504,7 +541,7 @@ void HttpRequestTest::testIsRangeSatisfied() CPPUNIT_ASSERT(httpRequest.isRangeSatisfied(range)); - request->setKeepAlive(true); + request->setPipeliningHint(true); CPPUNIT_ASSERT(!httpRequest.isRangeSatisfied(range)); diff --git a/test/HttpResponseTest.cc b/test/HttpResponseTest.cc index df01b590..bcf02fc4 100644 --- a/test/HttpResponseTest.cc +++ b/test/HttpResponseTest.cc @@ -293,7 +293,6 @@ void HttpResponseTest::testValidateResponse_good_range() httpRequest->setSegment(segment); SharedHandle request(new Request()); request->setUrl("http://localhost/archives/aria2-1.0.0.tar.bz2"); - request->setKeepAlive(false); httpRequest->setRequest(request); httpResponse.setHttpRequest(httpRequest); httpHeader->setResponseStatus("206"); @@ -320,7 +319,6 @@ void HttpResponseTest::testValidateResponse_bad_range() httpRequest->setSegment(segment); SharedHandle request(new Request()); request->setUrl("http://localhost/archives/aria2-1.0.0.tar.bz2"); - request->setKeepAlive(false); httpRequest->setRequest(request); httpResponse.setHttpRequest(httpRequest); httpHeader->setResponseStatus("206"); @@ -346,7 +344,6 @@ void HttpResponseTest::testValidateResponse_chunked() httpRequest->setSegment(segment); SharedHandle request(new Request()); request->setUrl("http://localhost/archives/aria2-1.0.0.tar.bz2"); - request->setKeepAlive(false); httpRequest->setRequest(request); httpResponse.setHttpRequest(httpRequest); httpHeader->setResponseStatus("206"); diff --git a/test/RequestTest.cc b/test/RequestTest.cc index b6b7806c..0bfe28c5 100644 --- a/test/RequestTest.cc +++ b/test/RequestTest.cc @@ -258,13 +258,13 @@ void RequestTest::testSetUrl17() void RequestTest::testRedirectUrl() { Request req; - req.setKeepAlive(true); + req.supportsPersistentConnection(true); req.setUrl("http://aria.rednoah.com:8080/aria2/index.html"); bool v2 = req.redirectUrl("http://aria.rednoah.co.jp/"); CPPUNIT_ASSERT(v2); - // keep-alive set to be false after redirection - CPPUNIT_ASSERT(!req.isKeepAlive()); + // persistent connection flag is set to be false after redirection + CPPUNIT_ASSERT(!req.supportsPersistentConnection()); // url must be the same CPPUNIT_ASSERT_EQUAL(std::string("http://aria.rednoah.com:8080/aria2/index.html"), req.getUrl());