2010-11-15 Tatsuhiro Tsujikawa <t-tujikawa@users.sourceforge.net>

	Replaced HttpHeader::responseStatus_ with HttpHeader::statusCode_.
	statusCode_ is of type int.
	* src/AbstractProxyResponseCommand.cc
	* src/FtpNegotiationCommand.cc
	* src/HttpHeader.cc
	* src/HttpHeader.h
	* src/HttpHeaderProcessor.cc
	* src/HttpResponse.cc
	* src/HttpResponse.h
	* src/HttpResponseCommand.cc
	* src/HttpSkipResponseCommand.cc
	* src/util.cc
	* src/util.h
	* test/HttpHeaderProcessorTest.cc
	* test/HttpHeaderTest.cc
	* test/HttpResponseTest.cc
	* test/UtilTest.cc

ChangeLog

@@ -1,3 +1,23 @@
+2010-11-15  Tatsuhiro Tsujikawa  <t-tujikawa@users.sourceforge.net>
+
+	Replaced HttpHeader::responseStatus_ with HttpHeader::statusCode_.
+	statusCode_ is of type int.
+	* src/AbstractProxyResponseCommand.cc
+	* src/FtpNegotiationCommand.cc
+	* src/HttpHeader.cc
+	* src/HttpHeader.h
+	* src/HttpHeaderProcessor.cc
+	* src/HttpResponse.cc
+	* src/HttpResponse.h
+	* src/HttpResponseCommand.cc
+	* src/HttpSkipResponseCommand.cc
+	* src/util.cc
+	* src/util.h
+	* test/HttpHeaderProcessorTest.cc
+	* test/HttpHeaderTest.cc
+	* test/HttpResponseTest.cc
+	* test/UtilTest.cc
+
 2010-11-15  Tatsuhiro Tsujikawa  <t-tujikawa@users.sourceforge.net>
 
 	Use SharedHandle::swap() in some places.

src/AbstractProxyResponseCommand.cc

@@ -69,7 +69,7 @@ bool AbstractProxyResponseCommand::executeInternal() {
     getDownloadEngine()->addCommand(this);
     return false;
   }
-  if(httpResponse->getResponseStatus() != HttpHeader::S200) {
+  if(httpResponse->getStatusCode() != 200) {
     throw DL_RETRY_EX(EX_PROXY_CONNECTION_FAILED);
   }
   getDownloadEngine()->addCommand(getNextCommand());

src/FtpNegotiationCommand.cc

@@ -747,7 +747,7 @@ bool FtpNegotiationCommand::recvTunnelResponse()
   if(!httpResponse) {
     return false;
   }
-  if(httpResponse->getResponseStatus() != HttpHeader::S200) {
+  if(httpResponse->getStatusCode() != 200) {
     throw DL_RETRY_EX(EX_PROXY_CONNECTION_FAILED);
   }
   sequence_ = SEQ_SEND_REST_PASV;

src/HttpHeader.cc

@@ -76,28 +76,6 @@ const std::string HttpHeader::ACCEPT_ENCODING("Accept-Encoding");
 
 const std::string HttpHeader::HTTP_1_1("HTTP/1.1");
 
-const std::string HttpHeader::S200("200");
-
-const std::string HttpHeader::S206("206");
-
-const std::string HttpHeader::S300("300");
-
-const std::string HttpHeader::S301("301");
-
-const std::string HttpHeader::S302("302");
-
-const std::string HttpHeader::S303("303");
-
-const std::string HttpHeader::S304("304");
-
-const std::string HttpHeader::S307("307");
-
-const std::string HttpHeader::S400("400");
-
-const std::string HttpHeader::S401("401");
-  
-const std::string HttpHeader::S404("404");
-
 HttpHeader::HttpHeader() {}
 HttpHeader::~HttpHeader() {}
 
@@ -201,11 +179,6 @@ RangeHandle HttpHeader::getRange() const
   return SharedHandle<Range>(new Range(startByte, endByte, entityLength));
 }
 
-void HttpHeader::setResponseStatus(const std::string& responseStatus)
-{
-  responseStatus_ = responseStatus;
-}
-
 void HttpHeader::setVersion(const std::string& version)
 {
   version_ = version;

src/HttpHeader.h

@@ -52,9 +52,8 @@ class HttpHeader {
 private:
   std::multimap<std::string, std::string> table_;
 
-  // for HTTP response header only
-  // response status, e.g. "200"
-  std::string responseStatus_;
+  // HTTP status code, e.g. 200
+  int statusCode_;
 
   // HTTP version, e.g. HTTP/1.1
   std::string version_;
@@ -77,12 +76,15 @@ public:
 
   SharedHandle<Range> getRange() const;
 
-  const std::string& getResponseStatus() const
+  int getStatusCode() const
   {
-    return responseStatus_;
+    return statusCode_;
   }
 
-  void setResponseStatus(const std::string& responseStatus);
+  void setStatusCode(int code)
+  {
+    statusCode_ = code;
+  }
 
   const std::string& getVersion() const
   {
@@ -143,28 +145,6 @@ public:
   static const std::string ACCEPT_ENCODING;
 
   static const std::string HTTP_1_1;
-
-  static const std::string S200;
-
-  static const std::string S206;
-
-  static const std::string S300;
-
-  static const std::string S301;
-
-  static const std::string S302;
-
-  static const std::string S303;
-
-  static const std::string S304;
-
-  static const std::string S307;
-
-  static const std::string S400;
-
-  static const std::string S401;
-  
-  static const std::string S404;
 };
 
 typedef SharedHandle<HttpHeader> HttpHeaderHandle;

src/HttpHeaderProcessor.cc

@@ -110,9 +110,13 @@ SharedHandle<HttpHeader> HttpHeaderProcessor::getHttpResponseHeader()
      delimpos < 12) {
     throw DL_RETRY_EX(EX_NO_STATUS_HEADER);
   }
+  int32_t statusCode;
+  if(!util::parseIntNoThrow(statusCode, buf_.substr(9, 3))) {
+    throw DL_RETRY_EX("Status code could not be parsed as integer.");
+  }
   HttpHeaderHandle httpHeader(new HttpHeader());
   httpHeader->setVersion(buf_.substr(0, 8));
-  httpHeader->setResponseStatus(buf_.substr(9, 3));
+  httpHeader->setStatusCode(statusCode);
   std::istringstream strm(buf_);
   // TODO 1st line(HTTP/1.1 200...) is also send to HttpHeader, but it should
   // not.

src/HttpResponse.cc

@@ -64,26 +64,24 @@ HttpResponse::~HttpResponse() {}
 
 void HttpResponse::validateResponse() const
 {
-  const std::string& status = getResponseStatus();
-  if(status >= HttpHeader::S400) {
+  int statusCode = getStatusCode();
+  if(statusCode >= 400) {
     return;
   }
-  if(status == HttpHeader::S304) {
+  if(statusCode == 304) {
     if(httpRequest_->getIfModifiedSinceHeader().empty()) {
       throw DL_ABORT_EX("Got 304 without If-Modified-Since");
     }
-  } else if(status == HttpHeader::S301 ||
-     status == HttpHeader::S302 ||
-     status == HttpHeader::S303 ||
-     status == HttpHeader::S307) {
+  } else if(statusCode == 301 ||
+            statusCode == 302 ||
+            statusCode == 303 ||
+            statusCode == 307) {
     if(!httpHeader_->defined(HttpHeader::LOCATION)) {
       throw DL_ABORT_EX
-        (StringFormat(EX_LOCATION_HEADER_REQUIRED,
-                      util::parseUInt(status)).str());
+        (StringFormat(EX_LOCATION_HEADER_REQUIRED, statusCode).str());
     }
     return;
-  } else if(status == HttpHeader::S200 ||
-            status == HttpHeader::S206) {
+  } else if(statusCode == 200 || statusCode == 206) {
     if(!httpHeader_->defined(HttpHeader::TRANSFER_ENCODING)) {
       // compare the received range against the requested range
       RangeHandle responseRange = httpHeader_->getRange();
@@ -102,7 +100,7 @@ void HttpResponse::validateResponse() const
     }
   } else {
     throw DL_ABORT_EX
-      (StringFormat("Unexpected status %s", status.c_str()).str());
+      (StringFormat("Unexpected status %d", statusCode).str());
   }
 }
 
@@ -140,11 +138,11 @@ void HttpResponse::retrieveCookie()
 
 bool HttpResponse::isRedirect() const
 {
-  const std::string& status = getResponseStatus();
-  return (HttpHeader::S301 == status ||
-          HttpHeader::S302 == status ||
-          HttpHeader::S303 == status ||
-          HttpHeader::S307 == status) &&
+  int statusCode = getStatusCode();
+  return (301 == statusCode ||
+          302 == statusCode ||
+          303 == statusCode ||
+          307 == statusCode) &&
     httpHeader_->defined(HttpHeader::LOCATION);
 }
 
@@ -254,10 +252,9 @@ void HttpResponse::setHttpRequest(const SharedHandle<HttpRequest>& httpRequest)
   httpRequest_ = httpRequest;
 }
 
-// TODO return std::string
-const std::string& HttpResponse::getResponseStatus() const
+int HttpResponse::getStatusCode() const
 {
-  return httpHeader_->getResponseStatus();
+  return httpHeader_->getStatusCode();
 }
 
 bool HttpResponse::hasRetryAfter() const

src/HttpResponse.h

@@ -108,7 +108,7 @@ public:
     return httpHeader_;
   }
 
-  const std::string& getResponseStatus() const;
+  int getStatusCode() const;
 
   void setHttpRequest(const SharedHandle<HttpRequest>& httpRequest);
 

src/HttpResponseCommand.cc

@@ -167,8 +167,9 @@ bool HttpResponseCommand::executeInternal()
       (getOption()->getAsInt(PREF_MAX_HTTP_PIPELINING));
   }
 
+  int statusCode = httpResponse->getStatusCode();
   if(!httpResponse->getHttpRequest()->getIfModifiedSinceHeader().empty()) {
-    if(httpResponse->getResponseStatus() == HttpHeader::S304) {
+    if(statusCode == 304) {
       uint64_t totalLength = httpResponse->getEntityLength();
       getFileEntry()->setLength(totalLength);
       getRequestGroup()->initPieceStorage();
@@ -181,15 +182,13 @@ bool HttpResponseCommand::executeInternal()
       poolConnection();
       getFileEntry()->poolRequest(getRequest());
       return true;
-    } else if(httpResponse->getResponseStatus() == HttpHeader::S200 ||
-              httpResponse->getResponseStatus() == HttpHeader::S206) {
+    } else if(statusCode == 200 || statusCode == 206) {
       // Remote file is newer than local file. We allow overwrite.
       getOption()->put(PREF_ALLOW_OVERWRITE, A2_V_TRUE);
     }
   }
-  if(httpResponse->getResponseStatus() >= HttpHeader::S300 &&
-     httpResponse->getResponseStatus() != HttpHeader::S304) {
-    if(httpResponse->getResponseStatus() == HttpHeader::S404) {
+  if(statusCode != 304 && statusCode >= 300) {
+    if(statusCode == 404) {
       getRequestGroup()->increaseAndValidateFileNotFoundCount();
     }
     return skipResponseBody(httpResponse);

src/HttpSkipResponseCommand.cc

@@ -171,6 +171,7 @@ void HttpSkipResponseCommand::poolConnection() const
 
 bool HttpSkipResponseCommand::processResponse()
 {
+  int statusCode;
   if(httpResponse_->isRedirect()) {
     unsigned int rnum =
       httpResponse_->getHttpRequest()->getRequest()->getRedirectCount();
@@ -180,8 +181,8 @@ bool HttpSkipResponseCommand::processResponse()
     }
     httpResponse_->processRedirect();
     return prepareForRetry(0);
-  } else if(httpResponse_->getResponseStatus() >= HttpHeader::S400) {
-    if(httpResponse_->getResponseStatus() == HttpHeader::S401) {
+  } else if((statusCode = httpResponse_->getStatusCode()) >= 400) {
+    if(statusCode == 401) {
       if(getOption()->getAsBool(PREF_HTTP_AUTH_CHALLENGE) &&
          !httpResponse_->getHttpRequest()->authenticationUsed() &&
          getDownloadEngine()->getAuthConfigFactory()->activateBasicCred
@@ -190,14 +191,11 @@ bool HttpSkipResponseCommand::processResponse()
       } else {
         throw DL_ABORT_EX(EX_AUTH_FAILED);
       }
-    }else if(httpResponse_->getResponseStatus() == HttpHeader::S404) {
+    } else if(statusCode == 404) {
       throw DL_ABORT_EX2(MSG_RESOURCE_NOT_FOUND,
                          downloadresultcode::RESOURCE_NOT_FOUND);
     } else {
-      throw DL_ABORT_EX
-        (StringFormat
-         (EX_BAD_STATUS,
-          util::parseUInt(httpResponse_->getResponseStatus())).str());
+      throw DL_ABORT_EX(StringFormat(EX_BAD_STATUS, statusCode).str());
     }
   } else {
     return prepareForRetry(0);

src/util.cc

@@ -512,6 +512,26 @@ int32_t parseInt(const std::string& s, int32_t base)
   return v;
 }
 
+bool parseIntNoThrow(int32_t& result, const std::string& s, int base)
+{
+  // Without trim, strtol("  -1  ",..) emits error.
+  std::string trimed = strip(s);
+  if(trimed.empty()) {
+    return false;
+  }
+  char* stop;
+  errno = 0;
+  long int v = strtol(trimed.c_str(), &stop, base);
+  if(*stop != '\0') {
+    return false;
+  } else if(((v == LONG_MAX || v == LONG_MIN) && (errno == ERANGE)) ||
+            v < INT32_MIN || INT32_MAX < v) {
+    return false;
+  }
+  result = v;
+  return true;
+}
+
 uint32_t parseUInt(const std::string& s, int base)
 {
   uint64_t v = parseULLInt(s, base);
@@ -524,7 +544,7 @@ uint32_t parseUInt(const std::string& s, int base)
 
 bool parseUIntNoThrow(uint32_t& result, const std::string& s, int base)
 {
-  // TODO what happens when we don't trim s?
+  // Without trim, strtol("  -1  ",..) emits error.
   std::string trimed = strip(s);
   if(trimed.empty()) {
     return false;
@@ -567,6 +587,7 @@ int64_t parseLLInt(const std::string& s, int32_t base)
 
 bool parseLLIntNoThrow(int64_t& result, const std::string& s, int base)
 {
+  // Without trim, strtol("  -1  ",..) emits error.
   std::string trimed = strip(s);
   if(trimed.empty()) {
     return false;

src/util.h

@@ -188,6 +188,8 @@ std::string secfmt(time_t sec);
 
 int32_t parseInt(const std::string& s, int32_t base = 10);
 
+bool parseIntNoThrow(int32_t& result, const std::string& s, int base = 10);
+
 uint32_t parseUInt(const std::string& s, int base = 10);
   
 bool parseUIntNoThrow(uint32_t& result, const std::string& s, int base = 10);

test/HttpHeaderProcessorTest.cc

@@ -107,7 +107,7 @@ void HttpHeaderProcessorTest::testGetHttpResponseHeader()
   proc.update(hd);
 
   SharedHandle<HttpHeader> header = proc.getHttpResponseHeader();
-  CPPUNIT_ASSERT_EQUAL(std::string("200"), header->getResponseStatus());
+  CPPUNIT_ASSERT_EQUAL(200, header->getStatusCode());
   CPPUNIT_ASSERT_EQUAL(std::string("HTTP/1.1"), header->getVersion());
   CPPUNIT_ASSERT_EQUAL(std::string("Mon, 25 Jun 2007 16:04:59 GMT"),
                        header->getFirst("Date"));
@@ -138,7 +138,7 @@ void HttpHeaderProcessorTest::testGetHttpResponseHeader_statusOnly()
   std::string hd = "HTTP/1.1 200\r\n\r\n";
   proc.update(hd);
   SharedHandle<HttpHeader> header = proc.getHttpResponseHeader();
-  CPPUNIT_ASSERT_EQUAL(std::string("200"), header->getResponseStatus());
+  CPPUNIT_ASSERT_EQUAL(200, header->getStatusCode());
 }
 
 void HttpHeaderProcessorTest::testGetHttpResponseHeader_insufficientStatusLength()

test/HttpHeaderTest.cc

@@ -83,7 +83,7 @@ void HttpHeaderTest::testGet()
 void HttpHeaderTest::testClearField()
 {
   HttpHeader h;
-  h.setResponseStatus(HttpHeader::S200);
+  h.setStatusCode(200);
   h.setVersion(HttpHeader::HTTP_1_1);
   h.put("Foo", "Bar");
   
@@ -92,7 +92,7 @@ void HttpHeaderTest::testClearField()
   h.clearField();
 
   CPPUNIT_ASSERT_EQUAL(std::string(""), h.getFirst("Foo"));
-  CPPUNIT_ASSERT_EQUAL(HttpHeader::S200, h.getResponseStatus());
+  CPPUNIT_ASSERT_EQUAL(200, h.getStatusCode());
   CPPUNIT_ASSERT_EQUAL(HttpHeader::HTTP_1_1, h.getVersion());
 }
 

test/HttpResponseTest.cc

@@ -212,14 +212,14 @@ void HttpResponseTest::testIsRedirect()
 {
   HttpResponse httpResponse;
   SharedHandle<HttpHeader> httpHeader(new HttpHeader());
-  httpHeader->setResponseStatus("200");
+  httpHeader->setStatusCode(200);
   httpHeader->put("Location", "http://localhost/download/aria2-1.0.0.tar.bz2");
 
   httpResponse.setHttpHeader(httpHeader);
 
   CPPUNIT_ASSERT(!httpResponse.isRedirect());
 
-  httpHeader->setResponseStatus("301");
+  httpHeader->setStatusCode(301);
 
   CPPUNIT_ASSERT(httpResponse.isRedirect());  
 }
@@ -340,7 +340,7 @@ void HttpResponseTest::testValidateResponse()
   SharedHandle<HttpHeader> httpHeader(new HttpHeader());
   httpResponse.setHttpHeader(httpHeader);
 
-  httpHeader->setResponseStatus("301");
+  httpHeader->setStatusCode(301);
 
   try {
     httpResponse.validateResponse();
@@ -372,7 +372,7 @@ void HttpResponseTest::testValidateResponse_good_range()
   request->setUri("http://localhost/archives/aria2-1.0.0.tar.bz2");
   httpRequest->setRequest(request);
   httpResponse.setHttpRequest(httpRequest);
-  httpHeader->setResponseStatus("206");
+  httpHeader->setStatusCode(206);
   httpHeader->put("Content-Range", "bytes 1048576-10485760/10485760");
   
   try {
@@ -399,7 +399,7 @@ void HttpResponseTest::testValidateResponse_bad_range()
   request->setUri("http://localhost/archives/aria2-1.0.0.tar.bz2");
   httpRequest->setRequest(request);
   httpResponse.setHttpRequest(httpRequest);
-  httpHeader->setResponseStatus("206");
+  httpHeader->setStatusCode(206);
   httpHeader->put("Content-Range", "bytes 0-10485760/10485761");
 
   try {
@@ -425,7 +425,7 @@ void HttpResponseTest::testValidateResponse_chunked()
   request->setUri("http://localhost/archives/aria2-1.0.0.tar.bz2");
   httpRequest->setRequest(request);
   httpResponse.setHttpRequest(httpRequest);
-  httpHeader->setResponseStatus("206");
+  httpHeader->setStatusCode(206);
   httpHeader->put("Content-Range", "bytes 0-10485760/10485761");
   httpHeader->put("Transfer-Encoding", "chunked");
 
@@ -442,7 +442,7 @@ void HttpResponseTest::testValidateResponse_withIfModifiedSince()
   HttpResponse httpResponse;
   SharedHandle<HttpHeader> httpHeader(new HttpHeader());
   httpResponse.setHttpHeader(httpHeader);
-  httpHeader->setResponseStatus("304");
+  httpHeader->setStatusCode(304);
   SharedHandle<HttpRequest> httpRequest(new HttpRequest());
   httpResponse.setHttpRequest(httpRequest);
   try {

test/UtilTest.cc

@@ -48,6 +48,7 @@ class UtilTest:public CppUnit::TestFixture {
   CPPUNIT_TEST(testParseUInt);
   CPPUNIT_TEST(testParseLLInt);
   CPPUNIT_TEST(testParseULLInt);
+  CPPUNIT_TEST(testParseIntNoThrow);
   CPPUNIT_TEST(testParseUIntNoThrow);
   CPPUNIT_TEST(testParseLLIntNoThrow);
   CPPUNIT_TEST(testToString_binaryStream);
@@ -106,6 +107,7 @@ public:
   void testParseUInt();
   void testParseLLInt();
   void testParseULLInt();
+  void testParseIntNoThrow();
   void testParseUIntNoThrow();
   void testParseLLIntNoThrow();
   void testToString_binaryStream();
@@ -828,6 +830,22 @@ void UtilTest::testParseULLInt()
   }
 }
 
+void UtilTest::testParseIntNoThrow()
+{
+  int32_t n;
+  CPPUNIT_ASSERT(util::parseIntNoThrow(n, " -1 "));
+  CPPUNIT_ASSERT_EQUAL((int32_t)-1, n);
+
+  CPPUNIT_ASSERT(util::parseIntNoThrow(n, "2147483647"));
+  CPPUNIT_ASSERT_EQUAL((int32_t)2147483647, n);
+
+  CPPUNIT_ASSERT(!util::parseIntNoThrow(n, "2147483648"));
+  CPPUNIT_ASSERT(!util::parseIntNoThrow(n, "-2147483649"));
+
+  CPPUNIT_ASSERT(!util::parseIntNoThrow(n, "12x"));
+  CPPUNIT_ASSERT(!util::parseIntNoThrow(n, ""));
+}
+
 void UtilTest::testParseUIntNoThrow()
 {
   uint32_t n;