Code cleanup in Http*

src/HttpConnection.cc

@@ -144,6 +144,7 @@ std::unique_ptr<HttpResponse> HttpConnection::receiveResponse()
       throw DL_RETRY_EX(EX_GOT_EOF);
     }
   }
+
   const auto& proc = outstandingHttpRequests_.front()->getHttpHeaderProcessor();
   if(proc->parse(socketRecvBuffer_->getBuffer(),
                  socketRecvBuffer_->getBufferLength())) {
@@ -158,11 +159,11 @@ std::unique_ptr<HttpResponse> HttpConnection::receiveResponse()
     socketRecvBuffer_->shiftBuffer(proc->getLastBytesProcessed());
     outstandingHttpRequests_.pop_front();
     return httpResponse;
-  } else {
+  }
+
   socketRecvBuffer_->shiftBuffer(proc->getLastBytesProcessed());
   return nullptr;
 }
-}
 
 bool HttpConnection::isIssued(const std::shared_ptr<Segment>& segment) const
 {

src/HttpConnection.h

@@ -58,11 +58,11 @@ class HttpRequestEntry {
 private:
   std::unique_ptr<HttpRequest> httpRequest_;
   std::unique_ptr<HttpHeaderProcessor> proc_;
+
 public:
   HttpRequestEntry(std::unique_ptr<HttpRequest> httpRequest);
 
-  const std::unique_ptr<HttpRequest>& getHttpRequest() const
+  const std::unique_ptr<HttpRequest>& getHttpRequest() const {
-  {
     return httpRequest_;
   }
 
@@ -83,12 +83,11 @@ private:
   HttpRequestEntries outstandingHttpRequests_;
 
   std::string eraseConfidentialInfo(const std::string& request);
-  void sendRequest
+  void sendRequest(std::unique_ptr<HttpRequest> httpRequest,
-  (std::unique_ptr<HttpRequest> httpRequest, std::string request);
+                   std::string request);
+
 public:
-  HttpConnection
+  HttpConnection(cuid_t cuid, const std::shared_ptr<SocketCore>& socket,
-  (cuid_t cuid,
-   const std::shared_ptr<SocketCore>& socket,
                  const std::shared_ptr<SocketRecvBuffer>& socketRecvBuffer);
   ~HttpConnection();
 
@@ -125,8 +124,7 @@ public:
 
   void sendPendingData();
 
-  const std::shared_ptr<SocketRecvBuffer>& getSocketRecvBuffer() const
+  const std::shared_ptr<SocketRecvBuffer>& getSocketRecvBuffer() const {
-  {
     return socketRecvBuffer_;
   }
 };

src/HttpDownloadCommand.cc

@@ -56,8 +56,7 @@
 
 namespace aria2 {
 
-HttpDownloadCommand::HttpDownloadCommand
+HttpDownloadCommand::HttpDownloadCommand(cuid_t cuid,
-(cuid_t cuid,
                                          const std::shared_ptr<Request>& req,
                                          const std::shared_ptr<FileEntry>& fileEntry,
                                          RequestGroup* requestGroup,
@@ -73,7 +72,8 @@ HttpDownloadCommand::HttpDownloadCommand
 
 HttpDownloadCommand::~HttpDownloadCommand() {}
 
-bool HttpDownloadCommand::prepareForNextSegment() {
+bool HttpDownloadCommand::prepareForNextSegment()
+{
   bool downloadFinished = getRequestGroup()->downloadFinished();
   if (getRequest()->isPipeliningEnabled() && !downloadFinished) {
     auto command = make_unique<HttpRequestCommand>
@@ -87,7 +87,8 @@ bool HttpDownloadCommand::prepareForNextSegment() {
     }
     getDownloadEngine()->addCommand(std::move(command));
     return true;
-  } else {
+  }
+
   const std::string& streamFilterName = getStreamFilter()->getName();
   if (getRequest()->isPipeliningEnabled() ||
       (getRequest()->isKeepAliveEnabled() &&
@@ -105,9 +106,10 @@ bool HttpDownloadCommand::prepareForNextSegment() {
     // pool terminated socket.  In HTTP/1.1, keep-alive is default,
     // so closing connection without Connection: close header means
     // that server is broken or not configured properly.
-      getDownloadEngine()->poolSocket
+    getDownloadEngine()->poolSocket(getRequest(), createProxyRequest(),
-        (getRequest(), createProxyRequest(), getSocket());
+                                    getSocket());
   }
+
   // The request was sent assuming that server supported pipelining, but
   // it turned out that server didn't support it.
   // We detect this situation by comparing the end byte in range header
@@ -118,26 +120,25 @@ bool HttpDownloadCommand::prepareForNextSegment() {
       !downloadFinished) {
     const std::shared_ptr<Segment>& segment = getSegments().front();
 
-      int64_t lastOffset =getFileEntry()->gtoloff
+    int64_t lastOffset =getFileEntry()->gtoloff(
-        (std::min(segment->getPosition()+segment->getLength(),
+        std::min(segment->getPosition()+segment->getLength(),
                  getFileEntry()->getLastOffset()));
-      Range range = httpResponse_->getHttpHeader()->getRange();
+    auto range = httpResponse_->getHttpHeader()->getRange();
     if (lastOffset == range.endByte + 1) {
       return prepareForRetry(0);
     }
   }
+
   return DownloadCommand::prepareForNextSegment();
 }
-}
 
 int64_t HttpDownloadCommand::getRequestEndOffset() const
 {
-  int64_t endByte = httpResponse_->getHttpHeader()->getRange().endByte;
+  auto endByte = httpResponse_->getHttpHeader()->getRange().endByte;
   if (endByte > 0) {
     return endByte+1;
-  } else {
-    return endByte;
   }
+  return endByte;
 }
 
 } // namespace aria2

src/HttpDownloadCommand.h

@@ -46,9 +46,11 @@ class HttpDownloadCommand : public DownloadCommand {
 private:
   std::unique_ptr<HttpResponse> httpResponse_;
   std::shared_ptr<HttpConnection> httpConnection_;
+
 protected:
   virtual bool prepareForNextSegment() CXX11_OVERRIDE;
   virtual int64_t getRequestEndOffset() const CXX11_OVERRIDE;
+
 public:
   HttpDownloadCommand(cuid_t cuid,
                       const std::shared_ptr<Request>& req,

src/HttpResponse.cc

@@ -32,6 +32,7 @@
  * files in the program, then also delete it here.
  */
 /* copyright --> */
+
 #include "HttpResponse.h"
 #include "Request.h"
 #include "Segment.h"
@@ -77,12 +78,14 @@ void HttpResponse::validateResponse() const
   if (statusCode >= 400) {
     return;
   }
+
   if (statusCode == 304) {
     if (!httpRequest_->conditionalRequest()) {
       throw DL_ABORT_EX2("Got 304 without If-Modified-Since or If-None-Match",
                          error_code::HTTP_PROTOCOL_ERROR);
     }
-  } else if(statusCode == 301 ||
+  }
+  else if (statusCode == 301 ||
            statusCode == 302 ||
            statusCode == 303 ||
            statusCode == 307) {
@@ -91,13 +94,13 @@ void HttpResponse::validateResponse() const
                          error_code::HTTP_PROTOCOL_ERROR);
     }
     return;
-  } else if(statusCode == 200 || statusCode == 206) {
+  }
+  else if (statusCode == 200 || statusCode == 206) {
     if (!httpHeader_->defined(HttpHeader::TRANSFER_ENCODING)) {
       // compare the received range against the requested range
-      Range responseRange = httpHeader_->getRange();
+      auto responseRange = httpHeader_->getRange();
       if (!httpRequest_->isRangeSatisfied(responseRange)) {
-        throw DL_ABORT_EX2
+        throw DL_ABORT_EX2(fmt(EX_INVALID_RANGE_HEADER,
-          (fmt(EX_INVALID_RANGE_HEADER,
                                httpRequest_->getStartByte(),
                                httpRequest_->getEndByte(),
                                httpRequest_->getEntityLength(),
@@ -107,7 +110,8 @@ void HttpResponse::validateResponse() const
                            error_code::CANNOT_RESUME);
       }
     }
-  } else {
+  }
+  else {
     throw DL_ABORT_EX2(fmt("Unexpected status %d", statusCode),
                        error_code::HTTP_PROTOCOL_ERROR);
   }
@@ -115,18 +119,17 @@ void HttpResponse::validateResponse() const
 
 std::string HttpResponse::determinFilename() const
 {
-  std::string contentDisposition =
+  std::string contentDisposition = util::getContentDispositionFilename(
-    util::getContentDispositionFilename
+      httpHeader_->find(HttpHeader::CONTENT_DISPOSITION));
-    (httpHeader_->find(HttpHeader::CONTENT_DISPOSITION));
   if (contentDisposition.empty()) {
-    std::string file =
+    auto file = httpRequest_->getFile();
-      util::percentDecode(httpRequest_->getFile().begin(),
+    file = util::percentDecode(file.begin(), file.end());
-                          httpRequest_->getFile().end());
     if (file.empty()) {
       return "index.html";
     }
     return file;
   }
+
   A2_LOG_INFO(fmt(MSG_CONTENT_DISPOSITION_DETECTED,
                   cuid_,
                   contentDisposition.c_str()));
@@ -136,39 +139,36 @@ std::string HttpResponse::determinFilename() const
 void HttpResponse::retrieveCookie()
 {
   Time now;
-  std::pair<std::multimap<int, std::string>::const_iterator,
+  auto r = httpHeader_->equalRange(HttpHeader::SET_COOKIE);
-            std::multimap<int, std::string>::const_iterator> r =
-    httpHeader_->equalRange(HttpHeader::SET_COOKIE);
   for (; r.first != r.second; ++r.first) {
-    httpRequest_->getCookieStorage()->parseAndStore
+    httpRequest_->getCookieStorage()->parseAndStore(
-      ((*r.first).second, httpRequest_->getHost(), httpRequest_->getDir(),
+        (*r.first).second,
+        httpRequest_->getHost(),
+        httpRequest_->getDir(),
         now.getTime());
   }
 }
 
 bool HttpResponse::isRedirect() const
 {
-  int statusCode = getStatusCode();
+  auto code = getStatusCode();
-  return (301 == statusCode ||
+  return (301 == code || 302 == code || 303 == code || 307 == code) &&
-          302 == statusCode ||
-          303 == statusCode ||
-          307 == statusCode) &&
          httpHeader_->defined(HttpHeader::LOCATION);
 }
 
 void HttpResponse::processRedirect()
 {
-  if(httpRequest_->getRequest()->redirectUri
+  const auto& req = httpRequest_->getRequest();
-     (util::percentEncodeMini(getRedirectURI()))) {
+  if (!req->redirectUri(util::percentEncodeMini(getRedirectURI()))) {
+    throw DL_RETRY_EX(fmt("CUID#%" PRId64
+                          " - Redirect to %s failed. It may not be a valid URI.",
+                          cuid_,
+                          req->getCurrentUri().c_str()));
+  }
+
   A2_LOG_INFO(fmt(MSG_REDIRECT,
                   cuid_,
                   httpRequest_->getRequest()->getCurrentUri().c_str()));
-  } else {
-    throw DL_RETRY_EX
-      (fmt("CUID#%" PRId64 " - Redirect to %s failed. It may not be a valid URI.",
-           cuid_,
-           httpRequest_->getRequest()->getCurrentUri().c_str()));
-  }
 }
 
 const std::string& HttpResponse::getRedirectURI() const
@@ -219,6 +219,7 @@ HttpResponse::getContentEncodingStreamFilter() const
     return make_unique<GZipDecodingStreamFilter>();
   }
 #endif // HAVE_ZLIB
+
   return nullptr;
 }
 
@@ -226,31 +227,31 @@ int64_t HttpResponse::getContentLength() const
 {
   if(!httpHeader_) {
     return 0;
-  } else {
-    return httpHeader_->getRange().getContentLength();
   }
+
+  return httpHeader_->getRange().getContentLength();
 }
 
 int64_t HttpResponse::getEntityLength() const
 {
   if(!httpHeader_) {
     return 0;
-  } else {
-    return httpHeader_->getRange().entityLength;
   }
+
+  return httpHeader_->getRange().entityLength;
 }
 
 std::string HttpResponse::getContentType() const
 {
   if(!httpHeader_) {
     return A2STR::NIL;
-  } else {
+  }
-    const std::string& ctype = httpHeader_->find(HttpHeader::CONTENT_TYPE);
+
-    std::string::const_iterator i = std::find(ctype.begin(), ctype.end(), ';');
+  const auto& ctype = httpHeader_->find(HttpHeader::CONTENT_TYPE);
+  auto i = std::find(ctype.begin(), ctype.end(), ';');
   Scip p = util::stripIter(ctype.begin(), i);
   return std::string(p.first, p.second);
 }
-}
 
 void HttpResponse::setHttpHeader(std::unique_ptr<HttpHeader> httpHeader)
 {
@@ -283,21 +284,24 @@ bool HttpResponse::supportsPersistentConnection() const
 }
 
 namespace {
+
 bool parseMetalinkHttpLink(MetalinkHttpEntry& result, const std::string& s)
 {
-  std::string::const_iterator first = std::find(s.begin(), s.end(), '<');
+  const auto first = std::find(s.begin(), s.end(), '<');
   if (first == s.end()) {
     return false;
   }
-  std::string::const_iterator last = std::find(first, s.end(), '>');
+
+  auto last = std::find(first, s.end(), '>');
   if(last == s.end()) {
     return false;
   }
-  std::pair<std::string::const_iterator,
+
-            std::string::const_iterator> p = util::stripIter(first+1, last);
+  auto p = util::stripIter(first+1, last);
   if (p.first == p.second) {
     return false;
   }
+
   result.uri.assign(p.first, p.second);
   last = std::find(last, s.end(), ';');
   if (last != s.end()) {
@@ -306,55 +310,64 @@ bool parseMetalinkHttpLink(MetalinkHttpEntry& result, const std::string& s)
   bool ok = false;
   while (1) {
     std::string name, value;
-    std::pair<std::string::const_iterator, bool> r =
+    auto r = util::nextParam(name, value, last, s.end(), ';');
-      util::nextParam(name, value, last, s.end(), ';');
     last = r.first;
+
     if (!r.second) {
       break;
     }
+
     if(value.empty()) {
       if(name == "pref") {
         result.pref = true;
       }
-    } else {
+      continue;
+    }
+
     if(name == "rel") {
       if(value == "duplicate") {
         ok = true;
       } else {
         ok = false;
       }
-      } else if(name == "pri") {
+      continue;
+    }
+
+    if (name == "pri") {
       int32_t priValue;
       if(util::parseIntNoThrow(priValue, value)) {
         if(1 <= priValue && priValue <= 999999) {
           result.pri = priValue;
         }
       }
-      } else if(name == "geo") {
+      continue;
+    }
+
+    if (name == "geo") {
       util::lowercase(value);
       result.geo = value;
-      }
+      continue;
     }
   }
   return ok;
 }
+
 } // namespace
 
 // Metalink/HTTP is defined by http://tools.ietf.org/html/rfc6249.
 // Link header field is defined by http://tools.ietf.org/html/rfc5988.
-void HttpResponse::getMetalinKHttpEntries
+void HttpResponse::getMetalinKHttpEntries(
-(std::vector<MetalinkHttpEntry>& result,
+    std::vector<MetalinkHttpEntry>& result,
     const std::shared_ptr<Option>& option) const
 {
-  std::pair<std::multimap<int, std::string>::const_iterator,
+  auto p = httpHeader_->equalRange(HttpHeader::LINK);
-            std::multimap<int, std::string>::const_iterator> p =
-    httpHeader_->equalRange(HttpHeader::LINK);
   for (; p.first != p.second; ++p.first) {
     MetalinkHttpEntry e;
     if(parseMetalinkHttpLink(e, (*p.first).second)) {
       result.push_back(e);
     }
   }
+
   if (!result.empty()) {
     std::vector<std::string> locs;
     if (option->defined(PREF_METALINK_LOCATION)) {
@@ -370,6 +383,7 @@ void HttpResponse::getMetalinKHttpEntries
       }
     }
   }
+
   std::sort(result.begin(), result.end());
 }
 
@@ -378,30 +392,29 @@ void HttpResponse::getMetalinKHttpEntries
 // http://tools.ietf.org/html/rfc3230.
 void HttpResponse::getDigest(std::vector<Checksum>& result) const
 {
-  using std::swap;
+  auto p = httpHeader_->equalRange(HttpHeader::DIGEST);
-  std::pair<std::multimap<int, std::string>::const_iterator,
-            std::multimap<int, std::string>::const_iterator> p =
-    httpHeader_->equalRange(HttpHeader::DIGEST);
   for (; p.first != p.second; ++p.first) {
     const std::string& s = (*p.first).second;
     std::string::const_iterator itr = s.begin();
     while (1) {
       std::string hashType, digest;
-      std::pair<std::string::const_iterator, bool> r =
+      auto r = util::nextParam(hashType, digest, itr, s.end(), ',');
-        util::nextParam(hashType, digest, itr, s.end(), ',');
       itr = r.first;
       if (!r.second) {
         break;
       }
+
       util::lowercase(hashType);
       digest = base64::decode(digest.begin(), digest.end());
       if (!MessageDigest::supports(hashType) ||
          MessageDigest::getDigestLength(hashType) != digest.size()) {
         continue;
       }
+
       result.push_back(Checksum(hashType, digest));
     }
   }
+
   std::sort(result.begin(), result.end(), HashTypeStronger());
   std::vector<Checksum> temp;
   for (auto i = result.begin(), eoi = result.end(); i != eoi;) {
@@ -411,16 +424,18 @@ void HttpResponse::getDigest(std::vector<Checksum>& result) const
       if((*i).getHashType() != (*j).getHashType()) {
         break;
       }
+
       if ((*i).getDigest() != (*j).getDigest()) {
         ok = false;
       }
     }
+
     if (ok) {
       temp.push_back(*i);
     }
     i = j;
   }
-  swap(temp, result);
+  std::swap(temp, result);
 }
 #endif // ENABLE_MESSAGE_DIGEST
 

src/HttpResponse.h

@@ -58,6 +58,7 @@ private:
   cuid_t cuid_;
   std::unique_ptr<HttpRequest> httpRequest_;
   std::unique_ptr<HttpHeader> httpHeader_;
+
 public:
   HttpResponse();
 
@@ -109,13 +110,11 @@ public:
 
   void setHttpRequest(std::unique_ptr<HttpRequest> httpRequest);
 
-  const std::unique_ptr<HttpRequest>& getHttpRequest() const
+  const std::unique_ptr<HttpRequest>& getHttpRequest() const {
-  {
     return httpRequest_;
   }
 
-  void setCuid(cuid_t cuid)
+  void setCuid(cuid_t cuid) {
-  {
     cuid_ = cuid;
   }
 
@@ -123,9 +122,9 @@ public:
 
   bool supportsPersistentConnection() const;
 
-  void getMetalinKHttpEntries
+  void getMetalinKHttpEntries(std::vector<MetalinkHttpEntry>& result,
-  (std::vector<MetalinkHttpEntry>& result,
                               const std::shared_ptr<Option>& option) const;
+
 #ifdef ENABLE_MESSAGE_DIGEST
   // Returns all digests specified in Digest header field.  Sort
   // strong algorithm first. Strength is defined in MessageDigest. If
@@ -133,6 +132,7 @@ public:
   // different value, they are all ignored.
   void getDigest(std::vector<Checksum>& result) const;
 #endif // ENABLE_MESSAGE_DIGEST
+
 };
 
 } // namespace aria2

src/HttpResponseCommand.cc

@@ -32,6 +32,7 @@
  * files in the program, then also delete it here.
  */
 /* copyright --> */
+
 #include "HttpResponseCommand.h"
 #include "DownloadEngine.h"
 #include "DownloadContext.h"
@@ -84,37 +85,36 @@
 namespace aria2 {
 
 namespace {
-std::unique_ptr<StreamFilter> getTransferEncodingStreamFilter
+
-(HttpResponse* httpResponse,
+std::unique_ptr<StreamFilter> getTransferEncodingStreamFilter(
+    HttpResponse* httpResponse,
     std::unique_ptr<StreamFilter> delegate = nullptr)
 {
   if (httpResponse->isTransferEncodingSpecified()) {
     auto filter = httpResponse->getTransferEncodingStreamFilter();
     if (!filter) {
-      throw DL_ABORT_EX
+      throw DL_ABORT_EX(fmt(EX_TRANSFER_ENCODING_NOT_SUPPORTED,
-        (fmt(EX_TRANSFER_ENCODING_NOT_SUPPORTED,
                             httpResponse->getTransferEncoding().c_str()));
     }
     filter->init();
     filter->installDelegate(std::move(delegate));
     return filter;
   }
+
   return delegate;
 }
-} // namespace
 
-namespace {
+std::unique_ptr<StreamFilter> getContentEncodingStreamFilter(
-std::unique_ptr<StreamFilter> getContentEncodingStreamFilter
+    HttpResponse* httpResponse,
-(HttpResponse* httpResponse,
     std::unique_ptr<StreamFilter> delegate = nullptr)
 {
   if (httpResponse->isContentEncodingSpecified()) {
     auto filter = httpResponse->getContentEncodingStreamFilter();
     if (!filter) {
-      A2_LOG_INFO
+      A2_LOG_INFO(fmt("Content-Encoding %s is specified, but the current "
-        (fmt("Content-Encoding %s is specified, but the current implementation"
+                      "implementation doesn't support it. The decoding "
-             "doesn't support it. The decoding process is skipped and the"
+                      "process is skipped and the downloaded content will be "
-             "downloaded content will be still encoded.",
+                      "still encoded.",
                       httpResponse->getContentEncoding().c_str()));
     }
     filter->init();
@@ -123,17 +123,19 @@ std::unique_ptr<StreamFilter> getContentEncodingStreamFilter
   }
   return delegate;
 }
+
 } // namespace
 
-HttpResponseCommand::HttpResponseCommand
+HttpResponseCommand::HttpResponseCommand(
-(cuid_t cuid,
+    cuid_t cuid,
     const std::shared_ptr<Request>& req,
     const std::shared_ptr<FileEntry>& fileEntry,
     RequestGroup* requestGroup,
     const std::shared_ptr<HttpConnection>& httpConnection,
     DownloadEngine* e,
     const std::shared_ptr<SocketCore>& s)
-  : AbstractCommand(cuid, req, fileEntry, requestGroup, e, s,
+  :
+    AbstractCommand(cuid, req, fileEntry, requestGroup, e, s,
                     httpConnection->getSocketRecvBuffer()),
     httpConnection_(httpConnection)
 {
@@ -153,7 +155,8 @@ bool HttpResponseCommand::executeInternal()
     addCommandSelf();
     return false;
   }
-  // check HTTP status number
+
+  // check HTTP status code
   httpResponse->validateResponse();
   httpResponse->retrieveCookie();
 
@@ -161,100 +164,106 @@ bool HttpResponseCommand::executeInternal()
   // Disable persistent connection if:
   //   Connection: close is received or the remote server is not HTTP/1.1.
   // We don't care whether non-HTTP/1.1 server returns Connection: keep-alive.
-  getRequest()->supportsPersistentConnection
+  auto& req = getRequest();
-    (httpResponse->supportsPersistentConnection());
+  req->supportsPersistentConnection(
-  if(getRequest()->isPipeliningEnabled()) {
+      httpResponse->supportsPersistentConnection());
-    getRequest()->setMaxPipelinedRequest
+  if (req->isPipeliningEnabled()) {
-      (getOption()->getAsInt(PREF_MAX_HTTP_PIPELINING));
+    req->setMaxPipelinedRequest(
-  } else {
+        getOption()->getAsInt(PREF_MAX_HTTP_PIPELINING));
-    getRequest()->setMaxPipelinedRequest(1);
+  }
+  else {
+    req->setMaxPipelinedRequest(1);
   }
 
-  int statusCode = httpResponse->getStatusCode();
+  auto statusCode = httpResponse->getStatusCode();
+  auto& ctx = getDownloadContext();
+  auto grp = getRequestGroup();
+  auto& fe = getFileEntry();
 
   if (statusCode == 304) {
     int64_t totalLength = httpResponse->getEntityLength();
-    getFileEntry()->setLength(totalLength);
+    fe->setLength(totalLength);
-    getRequestGroup()->initPieceStorage();
+    grp->initPieceStorage();
     getPieceStorage()->markAllPiecesDone();
     // Just set checksum verification done.
-    getDownloadContext()->setChecksumVerified(true);
+    ctx->setChecksumVerified(true);
-    A2_LOG_NOTICE
+    A2_LOG_NOTICE(fmt(MSG_DOWNLOAD_ALREADY_COMPLETED,
-      (fmt(MSG_DOWNLOAD_ALREADY_COMPLETED,
+                      GroupId::toHex(grp->getGID()).c_str(),
-           GroupId::toHex(getRequestGroup()->getGID()).c_str(),
+                      grp->getFirstFilePath().c_str()));
-           getRequestGroup()->getFirstFilePath().c_str()));
     poolConnection();
-    getFileEntry()->poolRequest(getRequest());
+    fe->poolRequest(req);
     return true;
   }
+
   if (!getPieceStorage()) {
     // Metalink/HTTP
-    if(getDownloadContext()->getAcceptMetalink()) {
+    if (ctx->getAcceptMetalink()) {
       if (httpHeader->defined(HttpHeader::LINK)) {
-        getDownloadContext()->setAcceptMetalink(false);
+        ctx->setAcceptMetalink(false);
         std::vector<MetalinkHttpEntry> entries;
         httpResponse->getMetalinKHttpEntries(entries, getOption());
         for(const auto& e : entries) {
-          getFileEntry()->addUri(e.uri);
+          fe->addUri(e.uri);
           A2_LOG_DEBUG(fmt("Adding URI=%s", e.uri.c_str()));
         }
       }
     }
+
 #ifdef ENABLE_MESSAGE_DIGEST
     if (httpHeader->defined(HttpHeader::DIGEST)) {
       std::vector<Checksum> checksums;
       httpResponse->getDigest(checksums);
       for(const auto &checksum : checksums) {
-        if(getDownloadContext()->getHashType().empty()) {
+        if (ctx->getHashType().empty()) {
           A2_LOG_DEBUG(fmt("Setting digest: type=%s, digest=%s",
                            checksum.getHashType().c_str(),
                            checksum.getDigest().c_str()));
-          getDownloadContext()->setDigest(checksum.getHashType(),
+          ctx->setDigest(checksum.getHashType(), checksum.getDigest());
-                                          checksum.getDigest());
-          break;
-        } else {
-          if(checkChecksum(getDownloadContext(), checksum)) {
           break;
         }
+
+        if (checkChecksum(ctx, checksum)) {
+          break;
         }
       }
     }
 #endif // ENABLE_MESSAGE_DIGEST
   }
+
   if (statusCode >= 300) {
     if (statusCode == 404) {
-      getRequestGroup()->increaseAndValidateFileNotFoundCount();
+      grp->increaseAndValidateFileNotFoundCount();
     }
     return skipResponseBody(std::move(httpResponse));
   }
-  if(getFileEntry()->isUniqueProtocol()) {
+
+  if (fe->isUniqueProtocol()) {
     // Redirection should be considered here. We need to parse
     // original URI to get hostname.
     const std::string& uri = getRequest()->getUri();
     uri_split_result us;
     if (uri_split(&us, uri.c_str()) == 0) {
       std::string host = uri::getFieldString(us, USR_HOST, uri.c_str());
-      getFileEntry()->removeURIWhoseHostnameIs(host);
+      fe->removeURIWhoseHostnameIs(host);
     }
   }
+
   if (!getPieceStorage()) {
-    getDownloadContext()->setAcceptMetalink(false);
+    ctx->setAcceptMetalink(false);
     int64_t totalLength = httpResponse->getEntityLength();
-    getFileEntry()->setLength(totalLength);
+    fe->setLength(totalLength);
-    if(getFileEntry()->getPath().empty()) {
+    if (fe->getPath().empty()) {
-      getFileEntry()->setPath
+      fe->setPath(util::createSafePath(getOption()->get(PREF_DIR),
-        (util::createSafePath
+                                       httpResponse->determinFilename()));
-         (getOption()->get(PREF_DIR), httpResponse->determinFilename()));
     }
-    getFileEntry()->setContentType(httpResponse->getContentType());
+    fe->setContentType(httpResponse->getContentType());
-    getRequestGroup()->preDownloadProcessing();
+    grp->preDownloadProcessing();
-    if(getDownloadEngine()->getRequestGroupMan()->
+    if (getDownloadEngine()->getRequestGroupMan()->isSameFileBeingDownloaded(grp)) {
-       isSameFileBeingDownloaded(getRequestGroup())) {
+      throw DOWNLOAD_FAILURE_EXCEPTION2(fmt(EX_DUPLICATE_FILE_DOWNLOAD,
-      throw DOWNLOAD_FAILURE_EXCEPTION2
+                                            grp->getFirstFilePath().c_str()),
-        (fmt(EX_DUPLICATE_FILE_DOWNLOAD,
-             getRequestGroup()->getFirstFilePath().c_str()),
                                         error_code::DUPLICATE_DOWNLOAD);
     }
+
     // update last modified time
     updateLastModifiedTime(httpResponse->getLastModifiedTime());
 
@@ -262,57 +271,58 @@ bool HttpResponseCommand::executeInternal()
     // assume we can do segmented downloading
     if (totalLength == 0 || shouldInflateContentEncoding(httpResponse.get())) {
       // we ignore content-length when inflate is required
-      getFileEntry()->setLength(0);
+      fe->setLength(0);
-      if(getRequest()->getMethod() == Request::METHOD_GET &&
+      if (req->getMethod() == Request::METHOD_GET && (totalLength != 0 ||
-         (totalLength != 0 ||
             !httpResponse->getHttpHeader()->defined(HttpHeader::CONTENT_LENGTH))){
         // DownloadContext::knowsTotalLength() == true only when
         // server says the size of file is 0 explicitly.
         getDownloadContext()->markTotalLengthIsUnknown();
       }
       return handleOtherEncoding(std::move(httpResponse));
-    } else {
+    }
+
     return handleDefaultEncoding(std::move(httpResponse));
   }
-  } else {
+
 #ifdef ENABLE_MESSAGE_DIGEST
-    if(!getDownloadContext()->getHashType().empty() &&
+  if (!ctx->getHashType().empty() && httpHeader->defined(HttpHeader::DIGEST)) {
-       httpHeader->defined(HttpHeader::DIGEST)) {
     std::vector<Checksum> checksums;
     httpResponse->getDigest(checksums);
     for (const auto &checksum : checksums) {
-        if(checkChecksum(getDownloadContext(), checksum)) {
+      if (checkChecksum(ctx, checksum)) {
         break;
       }
     }
   }
 #endif // ENABLE_MESSAGE_DIGEST
+
   // validate totalsize
-    getRequestGroup()->validateTotalLength(getFileEntry()->getLength(),
+  grp->validateTotalLength(fe->getLength(), httpResponse->getEntityLength());
-                                       httpResponse->getEntityLength());
   // update last modified time
   updateLastModifiedTime(httpResponse->getLastModifiedTime());
-    if(getRequestGroup()->getTotalLength() == 0) {
+
+  if (grp->getTotalLength() == 0) {
     // Since total length is unknown, the file size in previously
     // failed download could be larger than the size this time.
     // Also we can't resume in this case too.  So truncate the file
     // anyway.
     getPieceStorage()->getDiskAdaptor()->truncate(0);
-      auto teFilter = getTransferEncodingStreamFilter
+    auto teFilter = getTransferEncodingStreamFilter(
-        (httpResponse.get(),
+        httpResponse.get(),
         getContentEncodingStreamFilter(httpResponse.get()));
-      getDownloadEngine()->addCommand
+    getDownloadEngine()->addCommand(
-        (createHttpDownloadCommand(std::move(httpResponse),
+        createHttpDownloadCommand(std::move(httpResponse),
                                   std::move(teFilter)));
-    } else {
+  }
+  else {
     auto teFilter = getTransferEncodingStreamFilter(httpResponse.get());
-      getDownloadEngine()->addCommand
+    getDownloadEngine()->addCommand(
-        (createHttpDownloadCommand(std::move(httpResponse),
+        createHttpDownloadCommand(std::move(httpResponse),
                                   std::move(teFilter)));
   }
+
   return true;
 }
-}
 
 void HttpResponseCommand::updateLastModifiedTime(const Time& lastModified)
 {
@@ -321,8 +331,8 @@ void HttpResponseCommand::updateLastModifiedTime(const Time& lastModified)
   }
 }
 
-bool HttpResponseCommand::shouldInflateContentEncoding
+bool HttpResponseCommand::shouldInflateContentEncoding(
-(HttpResponse* httpResponse)
+    HttpResponse* httpResponse)
 {
   // Basically, on the fly inflation cannot be made with segment
   // download, because in each segment we don't know where the date
@@ -336,11 +346,11 @@ bool HttpResponseCommand::shouldInflateContentEncoding
     (ce == "gzip" || ce == "deflate");
 }
 
-bool HttpResponseCommand::handleDefaultEncoding
+bool HttpResponseCommand::handleDefaultEncoding(
-(std::unique_ptr<HttpResponse> httpResponse)
+    std::unique_ptr<HttpResponse> httpResponse)
 {
-  auto progressInfoFile = std::make_shared<DefaultBtProgressInfoFile>
+  auto progressInfoFile = std::make_shared<DefaultBtProgressInfoFile>(
-    (getDownloadContext(), std::shared_ptr<PieceStorage>{}, getOption().get());
+      getDownloadContext(), std::shared_ptr<PieceStorage>{}, getOption().get());
   getRequestGroup()->adjustFilename(progressInfoFile);
   getRequestGroup()->initPieceStorage();
 
@@ -353,6 +363,7 @@ bool HttpResponseCommand::handleDefaultEncoding
   if (!checkEntry) {
     return true;
   }
+
   File file(getRequestGroup()->getFirstFilePath());
   // We have to make sure that command that has Request object must
   // have segment after PieceStorage is initialized. See
@@ -367,10 +378,11 @@ bool HttpResponseCommand::handleDefaultEncoding
      segment && segment->getPositionToWrite() == 0 &&
      !getRequest()->isPipeliningEnabled()) {
     auto teFilter = getTransferEncodingStreamFilter(httpResponse.get());
-    checkEntry->pushNextCommand
+    checkEntry->pushNextCommand(
-      (createHttpDownloadCommand(std::move(httpResponse),
+        createHttpDownloadCommand(std::move(httpResponse),
                                   std::move(teFilter)));
-  } else {
+  }
+  else {
     getSegmentMan()->cancelSegment(getCuid());
     getFileEntry()->poolRequest(getRequest());
   }
@@ -381,11 +393,13 @@ bool HttpResponseCommand::handleDefaultEncoding
     poolConnection();
     getRequest()->setMethod(Request::METHOD_GET);
   }
+
   return true;
 }
 
-bool HttpResponseCommand::handleOtherEncoding
+bool HttpResponseCommand::handleOtherEncoding(
-(std::unique_ptr<HttpResponse> httpResponse) {
+    std::unique_ptr<HttpResponse> httpResponse)
+{
   // We assume that RequestGroup::getTotalLength() == 0 here
   if (getOption()->getAsBool(PREF_DRY_RUN)) {
     getRequestGroup()->initPieceStorage();
@@ -402,8 +416,8 @@ bool HttpResponseCommand::handleOtherEncoding
   // In this context, knowsTotalLength() is true only when the file is
   // really zero-length.
 
-  auto streamFilter = getTransferEncodingStreamFilter
+  auto streamFilter = getTransferEncodingStreamFilter(
-    (httpResponse.get(), getContentEncodingStreamFilter(httpResponse.get()));
+      httpResponse.get(), getContentEncodingStreamFilter(httpResponse.get()));
   // If chunked transfer-encoding is specified, we have to read end of
   // chunk markers(0\r\n\r\n, for example).
   bool chunkedUsed = streamFilter &&
@@ -413,14 +427,14 @@ bool HttpResponseCommand::handleOtherEncoding
   if (!chunkedUsed && getDownloadContext()->knowsTotalLength() &&
       getRequestGroup()->downloadFinishedByFileLength()) {
     getRequestGroup()->initPieceStorage();
+
 #ifdef ENABLE_MESSAGE_DIGEST
     // TODO Known issue: if .aria2 file exists, it will not be deleted
     // on successful verification, because .aria2 file is not loaded.
     // See also FtpNegotiationCommand::onFileSizeDetermined()
     if(getDownloadContext()->isChecksumVerificationNeeded()) {
       A2_LOG_DEBUG("Zero length file exists. Verify checksum.");
-      auto  entry = make_unique<ChecksumCheckIntegrityEntry>
+      auto entry = make_unique<ChecksumCheckIntegrityEntry>(getRequestGroup());
-        (getRequestGroup());
       entry->initValidator();
       getPieceStorage()->getDiskAdaptor()->openExistingFile();
       getDownloadEngine()->getCheckIntegrityMan()->pushEntry(std::move(entry));
@@ -429,8 +443,7 @@ bool HttpResponseCommand::handleOtherEncoding
     {
       getPieceStorage()->markAllPiecesDone();
       getDownloadContext()->setChecksumVerified(true);
-        A2_LOG_NOTICE
+      A2_LOG_NOTICE(fmt(MSG_DOWNLOAD_ALREADY_COMPLETED,
-          (fmt(MSG_DOWNLOAD_ALREADY_COMPLETED,
                         GroupId::toHex(getRequestGroup()->getGID()).c_str(),
                         getRequestGroup()->getFirstFilePath().c_str()));
     }
@@ -453,8 +466,7 @@ bool HttpResponseCommand::handleOtherEncoding
 #ifdef ENABLE_MESSAGE_DIGEST
     if (getDownloadContext()->isChecksumVerificationNeeded()) {
       A2_LOG_DEBUG("Verify checksum for zero-length file");
-      auto entry = make_unique<ChecksumCheckIntegrityEntry>
+      auto entry = make_unique<ChecksumCheckIntegrityEntry>(getRequestGroup());
-        (getRequestGroup());
       entry->initValidator();
       getDownloadEngine()->getCheckIntegrityMan()->pushEntry(std::move(entry));
     } else
@@ -465,28 +477,29 @@ bool HttpResponseCommand::handleOtherEncoding
     poolConnection();
     return true;
   }
+
   // We have to make sure that command that has Request object must
   // have segment after PieceStorage is initialized. See
   // AbstractCommand::execute()
   getSegmentMan()->getSegmentWithIndex(getCuid(), 0);
 
-  getDownloadEngine()->addCommand
+  getDownloadEngine()->addCommand(
-    (createHttpDownloadCommand(std::move(httpResponse),
+      createHttpDownloadCommand(std::move(httpResponse),
                                 std::move(streamFilter)));
   return true;
 }
 
-bool HttpResponseCommand::skipResponseBody
+bool HttpResponseCommand::skipResponseBody(
-(std::unique_ptr<HttpResponse> httpResponse)
+    std::unique_ptr<HttpResponse> httpResponse)
 {
   auto filter = getTransferEncodingStreamFilter(httpResponse.get());
   // We don't use Content-Encoding here because this response body is just
   // thrown away.
   auto httpResponsePtr = httpResponse.get();
-  auto command = make_unique<HttpSkipResponseCommand>
+  auto command = make_unique<HttpSkipResponseCommand>(
-    (getCuid(), getRequest(), getFileEntry(), getRequestGroup(),
+      getCuid(), getRequest(), getFileEntry(), getRequestGroup(),
-     httpConnection_, std::move(httpResponse),
+      httpConnection_, std::move(httpResponse), getDownloadEngine(),
-     getDownloadEngine(), getSocket());
+      getSocket());
   command->installStreamFilter(std::move(filter));
 
   // If request method is HEAD or the response body is zero-length,
@@ -505,6 +518,7 @@ bool HttpResponseCommand::skipResponseBody
 }
 
 namespace {
+
 bool decideFileAllocation(StreamFilter* filter)
 {
 #ifdef HAVE_ZLIB
@@ -517,31 +531,32 @@ bool decideFileAllocation(StreamFilter* filter)
     }
   }
 #endif // HAVE_ZLIB
+
   return true;
 }
+
 } // namespace
 
 std::unique_ptr<HttpDownloadCommand>
-HttpResponseCommand::createHttpDownloadCommand
+HttpResponseCommand::createHttpDownloadCommand(
-(std::unique_ptr<HttpResponse> httpResponse,
+    std::unique_ptr<HttpResponse> httpResponse,
     std::unique_ptr<StreamFilter> filter)
 {
 
-  auto command = make_unique<HttpDownloadCommand>
+  auto command = make_unique<HttpDownloadCommand>(
-    (getCuid(), getRequest(), getFileEntry(),
+      getCuid(), getRequest(), getFileEntry(), getRequestGroup(),
-     getRequestGroup(),
+      std::move(httpResponse), httpConnection_, getDownloadEngine(),
-     std::move(httpResponse), httpConnection_,
+      getSocket());
-     getDownloadEngine(), getSocket());
   command->setStartupIdleTime(getOption()->getAsInt(PREF_STARTUP_IDLE_TIME));
-  command->setLowestDownloadSpeedLimit
+  command->setLowestDownloadSpeedLimit(
-    (getOption()->getAsInt(PREF_LOWEST_SPEED_LIMIT));
+      getOption()->getAsInt(PREF_LOWEST_SPEED_LIMIT));
   if (getRequestGroup()->isFileAllocationEnabled() &&
       !decideFileAllocation(filter.get())) {
     getRequestGroup()->setFileAllocationEnabled(false);
   }
   command->installStreamFilter(std::move(filter));
-  getRequestGroup()->getURISelector()->tuneDownloadCommand
+  getRequestGroup()->getURISelector()->tuneDownloadCommand(
-    (getFileEntry()->getRemainingUris(), command.get());
+      getFileEntry()->getRemainingUris(), command.get());
 
   return std::move(command);
 }
@@ -562,18 +577,17 @@ void HttpResponseCommand::onDryRunFileFound()
 }
 
 #ifdef ENABLE_MESSAGE_DIGEST
-bool HttpResponseCommand::checkChecksum
+bool HttpResponseCommand::checkChecksum(
-(const std::shared_ptr<DownloadContext>& dctx,
+    const std::shared_ptr<DownloadContext>& dctx, const Checksum& checksum)
- const Checksum& checksum)
 {
   if (dctx->getHashType() == checksum.getHashType()) {
-    if(dctx->getDigest() == checksum.getDigest()) {
+    if (dctx->getDigest() != checksum.getDigest()) {
-      A2_LOG_INFO("Valid hash found in Digest header field.");
-      return true;
-    } else {
       throw DL_ABORT_EX("Invalid hash found in Digest header field.");
     }
+    A2_LOG_INFO("Valid hash found in Digest header field.");
+    return true;
   }
+
   return false;
 }
 #endif // ENABLE_MESSAGE_DIGEST

src/HttpResponseCommand.h

@@ -68,8 +68,7 @@ private:
   bool skipResponseBody(std::unique_ptr<HttpResponse> httpResponse);
 
   std::unique_ptr<HttpDownloadCommand>
-  createHttpDownloadCommand
+  createHttpDownloadCommand(std::unique_ptr<HttpResponse> httpResponse,
-  (std::unique_ptr<HttpResponse> httpResponse,
                             std::unique_ptr<StreamFilter> streamFilter);
 
   void updateLastModifiedTime(const Time& lastModified);
@@ -81,10 +80,10 @@ private:
   // Returns true if dctx and checksum has same hash type and hash
   // value.  If they have same hash type but different hash value,
   // throws exception.  Otherwise returns false.
-  bool checkChecksum
+  bool checkChecksum(const std::shared_ptr<DownloadContext>& dctx,
-  (const std::shared_ptr<DownloadContext>& dctx,
                      const Checksum& checksum);
 #endif // ENABLE_MESSAGE_DIGEST
+
 protected:
   bool executeInternal();