From 47adbe618c85b437220588fa1f7d602e113f7875 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Fri, 21 May 2010 12:22:04 +0000 Subject: [PATCH] 2010-05-21 Tatsuhiro Tsujikawa Fixed the bug that FTP download may fail when control connection is reused. This happens because FTP server can offer different root directory for different account. If pooled connections has different root directory, then download will fail. * src/DownloadEngine.cc * src/DownloadEngine.h * src/FtpConnection.cc * src/FtpConnection.h * src/FtpFinishDownloadCommand.cc * src/FtpInitiateConnectionCommand.cc * src/FtpNegotiationCommand.cc --- ChangeLog | 14 ++++++++++ src/DownloadEngine.cc | 41 ++++++++++++++++++++++------- src/DownloadEngine.h | 29 ++++++++++++-------- src/FtpConnection.cc | 5 ++++ src/FtpConnection.h | 2 ++ src/FtpFinishDownloadCommand.cc | 3 ++- src/FtpInitiateConnectionCommand.cc | 17 +++++++++--- src/FtpNegotiationCommand.cc | 2 +- 8 files changed, 87 insertions(+), 26 deletions(-) diff --git a/ChangeLog b/ChangeLog index 448eaa43..5c0d2e3e 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,17 @@ +2010-05-21 Tatsuhiro Tsujikawa + + Fixed the bug that FTP download may fail when control connection + is reused. This happens because FTP server can offer different + root directory for different account. If pooled connections has + different root directory, then download will fail. + * src/DownloadEngine.cc + * src/DownloadEngine.h + * src/FtpConnection.cc + * src/FtpConnection.h + * src/FtpFinishDownloadCommand.cc + * src/FtpInitiateConnectionCommand.cc + * src/FtpNegotiationCommand.cc + 2010-05-20 Tatsuhiro Tsujikawa Removed DownloadResult's ctor because it has many args. diff --git a/src/DownloadEngine.cc b/src/DownloadEngine.cc index 8f7d6b7d..725f782f 100644 --- a/src/DownloadEngine.cc +++ b/src/DownloadEngine.cc @@ -309,15 +309,26 @@ void DownloadEngine::poolSocket(const std::string& ipaddr, } } +static std::string createSockPoolKey +(const std::string& host, const std::string& username) +{ + std::string key; + key += util::percentEncode(username); + key += '@'; + key += host; + return key; +} + void DownloadEngine::poolSocket (const std::string& ipaddr, uint16_t port, + const std::string& username, const SharedHandle& sock, const std::map& options, time_t timeout) { SocketPoolEntry e(sock, options, timeout); - poolSocket(ipaddr, port, e); + poolSocket(createSockPoolKey(ipaddr, username), port, e); } void DownloadEngine::poolSocket @@ -326,7 +337,7 @@ void DownloadEngine::poolSocket const SharedHandle& sock, time_t timeout) { - SocketPoolEntry e(sock, std::map(), timeout); + SocketPoolEntry e(sock, timeout); poolSocket(ipaddr, port, e); } @@ -337,28 +348,31 @@ void DownloadEngine::poolSocket(const SharedHandle& request, { if(proxyDefined) { // If proxy is defined, then pool socket with its hostname. - poolSocket(request->getHost(), request->getPort(), socket); + poolSocket(request->getHost(), request->getPort(), socket, timeout); } else { std::pair peerInfo; socket->getPeerInfo(peerInfo); - poolSocket(peerInfo.first, peerInfo.second, socket); + poolSocket(peerInfo.first, peerInfo.second, socket, timeout); } } void DownloadEngine::poolSocket (const SharedHandle& request, bool proxyDefined, + const std::string& username, const SharedHandle& socket, const std::map& options, time_t timeout) { if(proxyDefined) { // If proxy is defined, then pool socket with its hostname. - poolSocket(request->getHost(), request->getPort(), socket, options); + poolSocket(request->getHost(), request->getPort(), username, + socket, options, timeout); } else { std::pair peerInfo; socket->getPeerInfo(peerInfo); - poolSocket(peerInfo.first, peerInfo.second, socket, options); + poolSocket(peerInfo.first, peerInfo.second, username, + socket, options, timeout); } } @@ -396,11 +410,12 @@ DownloadEngine::popPooledSocket(const std::string& ipaddr, uint16_t port) SharedHandle DownloadEngine::popPooledSocket(std::map& options, - const std::string& ipaddr, uint16_t port) + const std::string& ipaddr, uint16_t port, + const std::string& username) { SharedHandle s; std::multimap::iterator i = - findSocketPoolEntry(ipaddr, port); + findSocketPoolEntry(createSockPoolKey(ipaddr, username), port); if(i != _socketPool.end()) { s = (*i).second.getSocket(); options = (*i).second.getOptions(); @@ -427,12 +442,13 @@ DownloadEngine::popPooledSocket SharedHandle DownloadEngine::popPooledSocket (std::map& options, - const std::vector& ipaddrs, uint16_t port) + const std::vector& ipaddrs, uint16_t port, + const std::string& username) { SharedHandle s; for(std::vector::const_iterator i = ipaddrs.begin(), eoi = ipaddrs.end(); i != eoi; ++i) { - s = popPooledSocket(options, *i, port); + s = popPooledSocket(options, *i, port, username); if(!s.isNull()) { break; } @@ -448,6 +464,11 @@ DownloadEngine::SocketPoolEntry::SocketPoolEntry _options(options), _timeout(timeout) {} +DownloadEngine::SocketPoolEntry::SocketPoolEntry +(const SharedHandle& socket, time_t timeout): + _socket(socket), + _timeout(timeout) {} + DownloadEngine::SocketPoolEntry::~SocketPoolEntry() {} bool DownloadEngine::SocketPoolEntry::isTimeout() const diff --git a/src/DownloadEngine.h b/src/DownloadEngine.h index b29adca5..14da2b90 100644 --- a/src/DownloadEngine.h +++ b/src/DownloadEngine.h @@ -98,6 +98,9 @@ private: const std::map& option, time_t timeout); + SocketPoolEntry(const SharedHandle& socket, + time_t timeout); + ~SocketPoolEntry(); bool isTimeout() const; @@ -151,6 +154,16 @@ private: uint16_t port, const SocketPoolEntry& entry); + void poolSocket(const std::string& ipaddr, uint16_t port, + const std::string& username, + const SharedHandle& sock, + const std::map& options, + time_t timeout); + + void poolSocket(const std::string& ipaddr, uint16_t port, + const SharedHandle& sock, + time_t timeout); + std::multimap::iterator findSocketPoolEntry(const std::string& ipaddr, uint16_t port); public: @@ -202,17 +215,9 @@ public: void addRoutineCommand(Command* command); - void poolSocket(const std::string& ipaddr, uint16_t port, - const SharedHandle& sock, - const std::map& options, - time_t timeout = 15); - - void poolSocket(const std::string& ipaddr, uint16_t port, - const SharedHandle& sock, - time_t timeout = 15); - void poolSocket(const SharedHandle& request, bool proxyDefined, + const std::string& username, const SharedHandle& socket, const std::map& options, time_t timeout = 15); @@ -228,7 +233,8 @@ public: SharedHandle popPooledSocket (std::map& options, const std::string& ipaddr, - uint16_t port); + uint16_t port, + const std::string& username); SharedHandle popPooledSocket(const std::vector& ipaddrs, uint16_t port); @@ -237,7 +243,8 @@ public: popPooledSocket (std::map& options, const std::vector& ipaddrs, - uint16_t port); + uint16_t port, + const std::string& username); const SharedHandle& getCookieStorage() const { diff --git a/src/FtpConnection.cc b/src/FtpConnection.cc index 90e8970d..e22796e7 100644 --- a/src/FtpConnection.cc +++ b/src/FtpConnection.cc @@ -517,4 +517,9 @@ void FtpConnection::setBaseWorkingDir(const std::string& baseWorkingDir) _baseWorkingDir = baseWorkingDir; } +const std::string& FtpConnection::getUser() const +{ + return _authConfig->getUser(); +} + } // namespace aria2 diff --git a/src/FtpConnection.h b/src/FtpConnection.h index d95594ac..2137caeb 100644 --- a/src/FtpConnection.h +++ b/src/FtpConnection.h @@ -119,6 +119,8 @@ public: { return _baseWorkingDir; } + + const std::string& getUser() const; }; } // namespace aria2 diff --git a/src/FtpFinishDownloadCommand.cc b/src/FtpFinishDownloadCommand.cc index dfc856f3..a3cb1b8c 100644 --- a/src/FtpFinishDownloadCommand.cc +++ b/src/FtpFinishDownloadCommand.cc @@ -82,7 +82,8 @@ bool FtpFinishDownloadCommand::execute() if(getOption()->getAsBool(PREF_FTP_REUSE_CONNECTION)) { std::map options; options["baseWorkingDir"] = _ftpConnection->getBaseWorkingDir(); - e->poolSocket(req, isProxyDefined(), socket, options); + e->poolSocket(req, isProxyDefined(), _ftpConnection->getUser(), + socket, options); } } catch(RecoverableException& e) { logger->info(EX_EXCEPTION_CAUGHT, e); diff --git a/src/FtpInitiateConnectionCommand.cc b/src/FtpInitiateConnectionCommand.cc index d9547610..7eced52a 100644 --- a/src/FtpInitiateConnectionCommand.cc +++ b/src/FtpInitiateConnectionCommand.cc @@ -52,6 +52,8 @@ #include "Socket.h" #include "DownloadContext.h" #include "util.h" +#include "AuthConfigFactory.h" +#include "AuthConfig.h" namespace aria2 { @@ -73,9 +75,16 @@ Command* FtpInitiateConnectionCommand::createNextCommand Command* command; if(!proxyRequest.isNull()) { std::map options; - SharedHandle pooledSocket = - e->popPooledSocket(options, req->getHost(), req->getPort()); + SharedHandle pooledSocket; std::string proxyMethod = resolveProxyMethod(req->getProtocol()); + if(proxyMethod == V_GET) { + pooledSocket = e->popPooledSocket(req->getHost(), req->getPort()); + } else { + pooledSocket = e->popPooledSocket + (options, req->getHost(), req->getPort(), + e->getAuthConfigFactory()->createAuthConfig + (req, getOption().get())->getUser()); + } if(pooledSocket.isNull()) { if(logger->info()) { logger->info(MSG_CONNECTING_TO_SERVER, @@ -133,7 +142,9 @@ Command* FtpInitiateConnectionCommand::createNextCommand } else { std::map options; SharedHandle pooledSocket = - e->popPooledSocket(options, resolvedAddresses, req->getPort()); + e->popPooledSocket(options, resolvedAddresses, req->getPort(), + e->getAuthConfigFactory()->createAuthConfig + (req, getOption().get())->getUser()); if(pooledSocket.isNull()) { if(logger->info()) { logger->info(MSG_CONNECTING_TO_SERVER, diff --git a/src/FtpNegotiationCommand.cc b/src/FtpNegotiationCommand.cc index 9f89b6b2..d36e6326 100644 --- a/src/FtpNegotiationCommand.cc +++ b/src/FtpNegotiationCommand.cc @@ -806,7 +806,7 @@ void FtpNegotiationCommand::poolConnection() const if(getOption()->getAsBool(PREF_FTP_REUSE_CONNECTION)) { std::map options; options["baseWorkingDir"] = ftp->getBaseWorkingDir(); - e->poolSocket(req, isProxyDefined(), socket, options); + e->poolSocket(req, isProxyDefined(), ftp->getUser(), socket, options); } }