From 4ea28cb8377bc7b4654e6ae0216db6a7425d12c9 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Wed, 27 Jul 2011 23:28:31 +0900 Subject: [PATCH] Don't send basic auth header to service operated on differenct port. When --http-auth-challenge=true, aria2 only sends basic auth header when requested. Old implementation sends basic auth header to service operated in different port in successive request. This change avoid this bug. --- src/AuthConfigFactory.cc | 42 +++++++++++++++++++--------------- src/AuthConfigFactory.h | 18 +++++++++++---- src/HttpSkipResponseCommand.cc | 3 ++- test/AuthConfigFactoryTest.cc | 24 ++++++++++++------- test/HttpRequestTest.cc | 2 +- 5 files changed, 56 insertions(+), 33 deletions(-) diff --git a/src/AuthConfigFactory.cc b/src/AuthConfigFactory.cc index 4d52fbb4..1a053a23 100644 --- a/src/AuthConfigFactory.cc +++ b/src/AuthConfigFactory.cc @@ -66,11 +66,14 @@ AuthConfigFactory::createAuthConfig if(!request->getUsername().empty()) { updateBasicCred(BasicCred(request->getUsername(), request->getPassword(), - request->getHost(), request->getDir(), true)); + request->getHost(), + request->getPort(), + request->getDir(), true)); return createAuthConfig(request->getUsername(), request->getPassword()); } std::deque::const_iterator i = - findBasicCred(request->getHost(), request->getDir()); + findBasicCred(request->getHost(), request->getPort(), + request->getDir()); if(i == basicCreds_.end()) { return SharedHandle(); } else { @@ -180,10 +183,13 @@ void AuthConfigFactory::updateBasicCred(const BasicCred& basicCred) } bool AuthConfigFactory::activateBasicCred -(const std::string& host, const std::string& path, const Option* op) +(const std::string& host, + uint16_t port, + const std::string& path, + const Option* op) { - std::deque::iterator i = findBasicCred(host, path); + std::deque::iterator i = findBasicCred(host, port, path); if(i == basicCreds_.end()) { SharedHandle authConfig = createHttpAuthResolver(op)->resolveAuthConfig(host); @@ -191,7 +197,7 @@ bool AuthConfigFactory::activateBasicCred return false; } else { BasicCred bc(authConfig->getUser(), authConfig->getPassword(), - host, path, true); + host, port, path, true); i = std::lower_bound(basicCreds_.begin(), basicCreds_.end(), bc); basicCreds_.insert(i, bc); return true; @@ -204,10 +210,10 @@ bool AuthConfigFactory::activateBasicCred AuthConfigFactory::BasicCred::BasicCred (const std::string& user, const std::string& password, - const std::string& host, const std::string& path, + const std::string& host, uint16_t port, const std::string& path, bool activated): user_(user), password_(password), - host_(host), path_(path), activated_(activated) + host_(host), port_(port), path_(path), activated_(activated) { if(!util::endsWith(path_, "/")) { path_ += "/"; @@ -226,27 +232,27 @@ bool AuthConfigFactory::BasicCred::isActivated() const bool AuthConfigFactory::BasicCred::operator==(const BasicCred& cred) const { - return host_ == cred.host_ && path_ == cred.path_; + return host_ == cred.host_ && port_ == cred.port_ && path_ == cred.path_; } bool AuthConfigFactory::BasicCred::operator<(const BasicCred& cred) const { - int c = host_.compare(cred.host_); - if(c == 0) { - return path_ > cred.path_; - } else { - return c < 0; - } + return host_ < cred.host_ || + (!(cred.host_ < host_) && (port_ < cred.port_ || + (!(cred.port_ < port_) && path_ > cred.path_))); } std::deque::iterator -AuthConfigFactory::findBasicCred(const std::string& host, - const std::string& path) +AuthConfigFactory::findBasicCred +(const std::string& host, + uint16_t port, + const std::string& path) { - BasicCred bc("", "", host, path); + BasicCred bc("", "", host, port, path); std::deque::iterator i = std::lower_bound(basicCreds_.begin(), basicCreds_.end(), bc); - for(; i != basicCreds_.end() && (*i).host_ == host; ++i) { + for(; i != basicCreds_.end() && (*i).host_ == host && (*i).port_ == port; + ++i) { if(util::startsWith(bc.path_, (*i).path_)) { return i; } diff --git a/src/AuthConfigFactory.h b/src/AuthConfigFactory.h index 1291b225..8eb70f31 100644 --- a/src/AuthConfigFactory.h +++ b/src/AuthConfigFactory.h @@ -67,11 +67,12 @@ public: std::string user_; std::string password_; std::string host_; + uint16_t port_; std::string path_; bool activated_; BasicCred(const std::string& user, const std::string& password, - const std::string& host, const std::string& path, + const std::string& host, uint16_t port, const std::string& path, bool activated = false); void activate(); @@ -106,12 +107,19 @@ public: // using this AuthConfig object with given host and path "/" and // returns true. bool activateBasicCred - (const std::string& host, const std::string& path, const Option* op); + (const std::string& host, + uint16_t port, + const std::string& path, + const Option* op); - // Find a BasicCred using host and path and return the iterator - // pointing to it. If not found, then return basicCreds_.end(). + // Find a BasicCred using host, port and path and return the + // iterator pointing to it. If not found, then return + // basicCreds_.end(). std::deque::iterator - findBasicCred(const std::string& host, const std::string& path); + findBasicCred + (const std::string& host, + uint16_t port, + const std::string& path); // If the same BasicCred is already added, then it is replaced with // given basicCred. Otherwise, insert given basicCred to diff --git a/src/HttpSkipResponseCommand.cc b/src/HttpSkipResponseCommand.cc index d9824f0f..8e58a962 100644 --- a/src/HttpSkipResponseCommand.cc +++ b/src/HttpSkipResponseCommand.cc @@ -199,7 +199,8 @@ bool HttpSkipResponseCommand::processResponse() if(getOption()->getAsBool(PREF_HTTP_AUTH_CHALLENGE) && !httpResponse_->getHttpRequest()->authenticationUsed() && getDownloadEngine()->getAuthConfigFactory()->activateBasicCred - (getRequest()->getHost(), getRequest()->getDir(), getOption().get())) { + (getRequest()->getHost(), getRequest()->getPort(), + getRequest()->getDir(), getOption().get())) { return prepareForRetry(0); } else { throw DL_ABORT_EX2(EX_AUTH_FAILED, diff --git a/test/AuthConfigFactoryTest.cc b/test/AuthConfigFactoryTest.cc index 0de3238d..ed698f6b 100644 --- a/test/AuthConfigFactoryTest.cc +++ b/test/AuthConfigFactoryTest.cc @@ -57,7 +57,7 @@ void AuthConfigFactoryTest::testCreateAuthConfig_http() // not activated CPPUNIT_ASSERT(!factory.createAuthConfig(req, &option)); - CPPUNIT_ASSERT(factory.activateBasicCred("localhost", "/", &option)); + CPPUNIT_ASSERT(factory.activateBasicCred("localhost", 80, "/", &option)); CPPUNIT_ASSERT_EQUAL(std::string("localhostuser:localhostpass"), factory.createAuthConfig(req, &option)->getAuthText()); @@ -65,7 +65,7 @@ void AuthConfigFactoryTest::testCreateAuthConfig_http() // See default token in netrc is ignored. req->setUri("http://mirror/"); - CPPUNIT_ASSERT(!factory.activateBasicCred("mirror", "/", &option)); + CPPUNIT_ASSERT(!factory.activateBasicCred("mirror", 80, "/", &option)); CPPUNIT_ASSERT(!factory.createAuthConfig(req, &option)); @@ -75,7 +75,7 @@ void AuthConfigFactoryTest::testCreateAuthConfig_http() CPPUNIT_ASSERT(!factory.createAuthConfig(req, &option)); - CPPUNIT_ASSERT(factory.activateBasicCred("mirror", "/", &option)); + CPPUNIT_ASSERT(factory.activateBasicCred("mirror", 80, "/", &option)); CPPUNIT_ASSERT_EQUAL(std::string("userDefinedUser:userDefinedPassword"), factory.createAuthConfig(req, &option)->getAuthText()); @@ -204,20 +204,25 @@ void AuthConfigFactoryTest::testUpdateBasicCred() AuthConfigFactory factory; factory.updateBasicCred - (AuthConfigFactory::BasicCred("myname", "mypass", "localhost", "/", true)); + (AuthConfigFactory::BasicCred("myname", "mypass", "localhost", 80, "/", true)); factory.updateBasicCred - (AuthConfigFactory::BasicCred("price","j38jdc","localhost","/download", true)); + (AuthConfigFactory::BasicCred("price", "j38jdc", "localhost", 80, "/download", true)); factory.updateBasicCred - (AuthConfigFactory::BasicCred("alice","ium8","localhost","/documents", true)); + (AuthConfigFactory::BasicCred("soap", "planB", "localhost", 80, "/download/beta", true)); factory.updateBasicCred - (AuthConfigFactory::BasicCred("jack", "jackx","mirror", "/doc", true)); + (AuthConfigFactory::BasicCred("alice", "ium8", "localhost", 80, "/documents", true)); + factory.updateBasicCred + (AuthConfigFactory::BasicCred("jack", "jackx", "mirror", 80, "/doc", true)); SharedHandle req(new Request()); req->setUri("http://localhost/download/v2.6/Changelog"); - CPPUNIT_ASSERT_EQUAL(std::string("price:j38jdc"), factory.createAuthConfig(req, &option)->getAuthText()); + req->setUri("http://localhost/download/beta/v2.7/Changelog"); + CPPUNIT_ASSERT_EQUAL(std::string("soap:planB"), + factory.createAuthConfig(req, &option)->getAuthText()); + req->setUri("http://localhost/documents/reference.html"); CPPUNIT_ASSERT_EQUAL(std::string("alice:ium8"), factory.createAuthConfig(req, &option)->getAuthText()); @@ -230,6 +235,9 @@ void AuthConfigFactoryTest::testUpdateBasicCred() CPPUNIT_ASSERT_EQUAL(std::string("myname:mypass"), factory.createAuthConfig(req, &option)->getAuthText()); + req->setUri("http://localhost:8080/doc/readme.txt"); + CPPUNIT_ASSERT(!factory.createAuthConfig(req, &option)); + req->setUri("http://local/"); CPPUNIT_ASSERT(!factory.createAuthConfig(req, &option)); diff --git a/test/HttpRequestTest.cc b/test/HttpRequestTest.cc index 010eae39..cf395fc5 100644 --- a/test/HttpRequestTest.cc +++ b/test/HttpRequestTest.cc @@ -244,7 +244,7 @@ void HttpRequestTest::testCreateRequest() option_->put(PREF_HTTP_PASSWD, "aria2passwd"); CPPUNIT_ASSERT(authConfigFactory_->activateBasicCred - ("localhost", "/", option_.get())); + ("localhost", 8080, "/", option_.get())); expectedText = "GET /archives/aria2-1.0.0.tar.bz2 HTTP/1.1\r\n" "User-Agent: aria2\r\n"