2008-04-22 Tatsuhiro Tsujikawa <tujikawa at rednoah dot com>

	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

ChangeLog

@@ -1,3 +1,19 @@
+2008-04-22  Tatsuhiro Tsujikawa  <tujikawa at rednoah dot com>
+
+	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  <tujikawa at rednoah dot com>
 
 	Added missing setUserHeaders call

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;

src/HttpConnection.cc

@@ -47,6 +47,7 @@
 #include "HttpHeader.h"
 #include "Logger.h"
 #include "Socket.h"
+#include "Option.h"
 #include <sstream>
 
 namespace aria2 {
@@ -136,7 +137,9 @@ HttpResponseHandle HttpConnection::receiveResponse()
 
   SharedHandle<HttpHeader> 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());

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;

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";

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<HttpRequest>
+createHttpRequest(const SharedHandle<Request>& req,
+		  const SharedHandle<Segment>& 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<Segment>(),
+			 _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);
       }
     }

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();

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);
 }
 

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;

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);

test/HttpRequestTest.cc

@@ -74,7 +74,8 @@ void HttpRequestTest::testGetEndByte()
   CPPUNIT_ASSERT_EQUAL(0LL, httpRequest.getEndByte());
 
   SharedHandle<Request> 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<SharedHandle<AuthConfigFactory> >::instance(authConfigFactory);
 
   SharedHandle<Request> 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> 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<Piece> p(new Piece(0, 1024*1024));
   SharedHandle<Segment> 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));
 

test/HttpResponseTest.cc

@@ -293,7 +293,6 @@ void HttpResponseTest::testValidateResponse_good_range()
   httpRequest->setSegment(segment);
   SharedHandle<Request> 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> 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> 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");

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());