Proxy URI is now constructed in getProxyUri().

The previous implementation constructs proxy URI in OptionHandler but
it cannot handle with the situation when user, password and proxy URI
is given various order.  Now we just set rules: username set in
--*-proxy-user overrides username in --*-proxy option, no matter in
any order username, password and proxy URI are parsed. Likewise,
password set in --*--proxy-passwd overrides password in --*-proxy
option.

src/AbstractCommand.cc

@@ -543,35 +543,72 @@ void AbstractCommand::setWriteCheckSocketIf
 }
 
 namespace {
-// Returns proxy option value for the given protocol.
+// Constructs proxy URI, merging username and password if they are
-const std::string& getProxyOptionFor
+// defined.
-(const Pref* proxyPref, const SharedHandle<Option>& option)
+std::string makeProxyUri
+(const Pref* proxyPref,
+ const Pref* proxyUser,
+ const Pref* proxyPasswd,
+ const Option* option)
 {
-  if(option->defined(proxyPref)) {
+  uri::UriStruct us;
-    return option->get(proxyPref);
+  if(uri::parse(us, option->get(proxyPref))) {
+    if(option->defined(proxyUser)) {
+      us.username = option->get(proxyUser);
+    }
+    if(option->defined(proxyPasswd)) {
+      us.password = option->get(proxyPasswd);
+      us.hasPassword = true;
+    }
+    return uri::construct(us);
   } else {
-    return option->get(PREF_ALL_PROXY);
+    return "";
   }
 }
 } // namespace
 
 namespace {
+// Returns proxy option value for the given protocol.
+std::string getProxyOptionFor
+(const Pref* proxyPref,
+ const Pref* proxyUser,
+ const Pref* proxyPasswd,
+ const Option* option)
+{
+  std::string uri = makeProxyUri(proxyPref, proxyUser, proxyPasswd, option);
+  if(uri.empty()) {
+    return makeProxyUri(PREF_ALL_PROXY, PREF_ALL_PROXY_USER,
+                        PREF_ALL_PROXY_PASSWD, option);
+  } else {
+    return uri;
+  }
+}
+} // namespace
+
 // Returns proxy URI for given protocol.  If no proxy URI is defined,
 // then returns an empty string.
-const std::string& getProxyUri
+std::string getProxyUri
-(const std::string& protocol, const SharedHandle<Option>& option)
+(const std::string& protocol, const Option* option)
 {
   if(protocol == Request::PROTO_HTTP) {
-    return getProxyOptionFor(PREF_HTTP_PROXY, option);
+    return getProxyOptionFor(PREF_HTTP_PROXY,
+                             PREF_HTTP_PROXY_USER,
+                             PREF_HTTP_PROXY_PASSWD,
+                             option);
   } else if(protocol == Request::PROTO_HTTPS) {
-    return getProxyOptionFor(PREF_HTTPS_PROXY, option);
+    return getProxyOptionFor(PREF_HTTPS_PROXY,
+                             PREF_HTTPS_PROXY_USER,
+                             PREF_HTTPS_PROXY_PASSWD,
+                             option);
   } else if(protocol == Request::PROTO_FTP) {
-    return getProxyOptionFor(PREF_FTP_PROXY, option);
+    return getProxyOptionFor(PREF_FTP_PROXY,
+                             PREF_FTP_PROXY_USER,
+                             PREF_FTP_PROXY_PASSWD,
+                             option);
   } else {
     return A2STR::NIL;
   }
 }
-} // namespace
 
 namespace {
 // Returns true if proxy is defined for the given protocol. Otherwise
@@ -579,9 +616,8 @@ namespace {
 bool isProxyRequest
 (const std::string& protocol, const SharedHandle<Option>& option)
 {
-  const std::string& proxyUri = getProxyUri(protocol, option);
+  std::string proxyUri = getProxyUri(protocol, option.get());
-  uri::UriStruct us;
+  return !proxyUri.empty();
-  return !proxyUri.empty() && uri::parse(us, proxyUri);
 }
 } // namespace
 
@@ -635,7 +671,7 @@ SharedHandle<Request> AbstractCommand::createProxyRequest() const
   if(inNoProxy(req_, getOption()->get(PREF_NO_PROXY))) {
     return proxyRequest;
   }
-  std::string proxy = getProxyUri(req_->getProtocol(), getOption());
+  std::string proxy = getProxyUri(req_->getProtocol(), getOption().get());
   if(!proxy.empty()) {
     proxyRequest.reset(new Request());
     if(proxyRequest->setUri(proxy)) {

src/AbstractCommand.h

@@ -243,6 +243,11 @@ public:
   bool execute();
 };
 
+// Returns proxy URI for given protocol.  If no proxy URI is defined,
+// then returns an empty string.
+std::string getProxyUri
+(const std::string& protocol, const Option* option);
+
 } // namespace aria2
 
 #endif // D_ABSTRACT_COMMAND_H

src/OptionHandlerFactory.cc

@@ -1306,7 +1306,7 @@ OptionHandlerFactory::createOptionHandlers()
     handlers.push_back(op);
   }
   {
-    SharedHandle<OptionHandler> op(new HttpProxyPasswdOptionHandler
+    SharedHandle<OptionHandler> op(new DefaultOptionHandler
                                    (PREF_HTTP_PROXY_PASSWD,
                                     TEXT_HTTP_PROXY_PASSWD,
                                     NO_DEFAULT_VALUE));
@@ -1318,7 +1318,7 @@ OptionHandlerFactory::createOptionHandlers()
     handlers.push_back(op);
   }
   {
-    SharedHandle<OptionHandler> op(new HttpProxyUserOptionHandler
+    SharedHandle<OptionHandler> op(new DefaultOptionHandler
                                    (PREF_HTTP_PROXY_USER,
                                     TEXT_HTTP_PROXY_USER,
                                     NO_DEFAULT_VALUE));
@@ -1342,7 +1342,7 @@ OptionHandlerFactory::createOptionHandlers()
     handlers.push_back(op);
   }
   {
-    SharedHandle<OptionHandler> op(new HttpProxyPasswdOptionHandler
+    SharedHandle<OptionHandler> op(new DefaultOptionHandler
                                    (PREF_HTTPS_PROXY_PASSWD,
                                     TEXT_HTTPS_PROXY_PASSWD,
                                     NO_DEFAULT_VALUE));
@@ -1355,7 +1355,7 @@ OptionHandlerFactory::createOptionHandlers()
     handlers.push_back(op);
   }
   {
-    SharedHandle<OptionHandler> op(new HttpProxyUserOptionHandler
+    SharedHandle<OptionHandler> op(new DefaultOptionHandler
                                    (PREF_HTTPS_PROXY_USER,
                                     TEXT_HTTPS_PROXY_USER,
                                     NO_DEFAULT_VALUE));
@@ -1379,7 +1379,7 @@ OptionHandlerFactory::createOptionHandlers()
     handlers.push_back(op);
   }
   {
-    SharedHandle<OptionHandler> op(new HttpProxyPasswdOptionHandler
+    SharedHandle<OptionHandler> op(new DefaultOptionHandler
                                    (PREF_FTP_PROXY_PASSWD,
                                     TEXT_FTP_PROXY_PASSWD,
                                     NO_DEFAULT_VALUE));
@@ -1391,7 +1391,7 @@ OptionHandlerFactory::createOptionHandlers()
     handlers.push_back(op);
   }
   {
-    SharedHandle<OptionHandler> op(new HttpProxyUserOptionHandler
+    SharedHandle<OptionHandler> op(new DefaultOptionHandler
                                    (PREF_FTP_PROXY_USER,
                                     TEXT_FTP_PROXY_USER,
                                     NO_DEFAULT_VALUE));
@@ -1416,7 +1416,7 @@ OptionHandlerFactory::createOptionHandlers()
     handlers.push_back(op);
   }
   {
-    SharedHandle<OptionHandler> op(new HttpProxyPasswdOptionHandler
+    SharedHandle<OptionHandler> op(new DefaultOptionHandler
                                    (PREF_ALL_PROXY_PASSWD,
                                     TEXT_ALL_PROXY_PASSWD,
                                     NO_DEFAULT_VALUE));
@@ -1430,7 +1430,7 @@ OptionHandlerFactory::createOptionHandlers()
     handlers.push_back(op);
   }
   {
-    SharedHandle<OptionHandler> op(new HttpProxyUserOptionHandler
+    SharedHandle<OptionHandler> op(new DefaultOptionHandler
                                    (PREF_ALL_PROXY_USER,
                                     TEXT_ALL_PROXY_USER,
                                     NO_DEFAULT_VALUE));

src/OptionHandlerImpl.cc

@@ -509,98 +509,6 @@ std::string HostPortOptionHandler::createPossibleValuesString() const
   return "HOST:PORT";
 }
 
-HttpProxyUserOptionHandler::HttpProxyUserOptionHandler
-(const Pref* pref,
- const char* description,
- const std::string& defaultValue,
- char shortName)
-  : AbstractOptionHandler(pref, description, defaultValue,
-                          OptionHandler::REQ_ARG, shortName)
-{}
-
-void HttpProxyUserOptionHandler::parseArg
-(Option& option, const std::string& optarg)
-{
-  static const char A2_USER[] = "-user";
-  size_t kLen = strlen(pref_->k);
-  if(util::endsWith(pref_->k, pref_->k+kLen, A2_USER, vend(A2_USER)-1)) {
-    const Pref* proxyPref = option::k2p(std::string(pref_->k, kLen-5));
-    const std::string& olduri = option.get(proxyPref);
-    if(!olduri.empty()) {
-      Request req;
-      bool b = req.setUri(olduri);
-      assert(b);
-      std::string uri = "http://";
-      if(!optarg.empty()) {
-        uri += util::percentEncode(optarg);
-      }
-      if(req.hasPassword()) {
-        uri += A2STR::COLON_C;
-        uri += util::percentEncode(req.getPassword());
-      }
-      if(uri.size() > 7) {
-        uri += "@";
-      }
-      uri += req.getHost();
-      uri += ":";
-      uri += util::uitos(req.getPort());
-      option.put(proxyPref, uri);
-    }
-  }
-  option.put(pref_, optarg);
-}
-
-std::string HttpProxyUserOptionHandler::createPossibleValuesString() const
-{
-  return "";
-}
-
-HttpProxyPasswdOptionHandler::HttpProxyPasswdOptionHandler
-(const Pref* pref,
- const char* description,
- const std::string& defaultValue,
- char shortName)
-  : AbstractOptionHandler(pref, description, defaultValue,
-                          OptionHandler::REQ_ARG, shortName)
-{}
-
-void HttpProxyPasswdOptionHandler::parseArg
-(Option& option, const std::string& optarg)
-{
-  static const char A2_PASSWD[] = "-passwd";
-  size_t kLen = strlen(pref_->k);
-  if(util::endsWith(pref_->k, pref_->k+kLen, A2_PASSWD, vend(A2_PASSWD)-1)) {
-    const Pref* proxyPref = option::k2p(std::string(pref_->k, kLen-7));
-    const std::string& olduri = option.get(proxyPref);
-    if(!olduri.empty()) {
-      Request req;
-      bool b = req.setUri(olduri);
-      assert(b);
-      std::string uri = "http://";
-      if(!req.getUsername().empty()) {
-        uri += util::percentEncode(req.getUsername());
-      }
-      uri += A2STR::COLON_C;
-      if(!optarg.empty()) {
-        uri += util::percentEncode(optarg);
-      }
-      if(uri.size() > 7) {
-        uri += "@";
-      }
-      uri += req.getHost();
-      uri += ":";
-      uri += util::itos(req.getPort());
-      option.put(proxyPref, uri);
-    }
-  }
-  option.put(pref_, optarg);
-}
-
-std::string HttpProxyPasswdOptionHandler::createPossibleValuesString() const
-{
-  return "";
-}
-
 HttpProxyOptionHandler::HttpProxyOptionHandler
 (const Pref* pref,
  const char* description,
@@ -619,7 +527,6 @@ void HttpProxyOptionHandler::parseArg(Option& option, const std::string& optarg)
   if(optarg.empty()) {
     option.put(pref_, optarg);
   } else {
-    Request req;
     std::string uri;
     static const char A2_HTTP[] = "http://";
     static const char A2_HTTPS[] = "https://";
@@ -640,17 +547,6 @@ void HttpProxyOptionHandler::parseArg(Option& option, const std::string& optarg)
       throw DL_ABORT_EX(_("unrecognized proxy format"));
     }
     us.protocol = "http";
-    if(us.username.empty()) {
-      if(option.defined(proxyUserPref_)) {
-        us.username = option.get(proxyUserPref_);
-      }
-    }
-    if(!us.hasPassword) {
-      if(option.defined(proxyPasswdPref_)) {
-        us.password = option.get(proxyPasswdPref_);
-        us.hasPassword = true;
-      }
-    }
     option.put(pref_, uri::construct(us));
   }
 }

src/OptionHandlerImpl.h

@@ -223,26 +223,6 @@ public:
   virtual std::string createPossibleValuesString() const;
 };
 
-class HttpProxyUserOptionHandler:public AbstractOptionHandler {
-public:
-  HttpProxyUserOptionHandler(const Pref* pref,
-                             const char* description,
-                             const std::string& defaultValue,
-                             char shortName = 0);
-  virtual void parseArg(Option& option, const std::string& optarg);
-  virtual std::string createPossibleValuesString() const;
-};
-
-class HttpProxyPasswdOptionHandler:public AbstractOptionHandler {
-public:
-  HttpProxyPasswdOptionHandler(const Pref* pref,
-                               const char* description,
-                               const std::string& defaultValue,
-                               char shortName = 0);
-  virtual void parseArg(Option& option, const std::string& optarg);
-  virtual std::string createPossibleValuesString() const;
-};
-
 class HttpProxyOptionHandler : public AbstractOptionHandler {
 private:
   const Pref* proxyUserPref_;

test/AbstractCommandTest.cc

@@ -0,0 +1,69 @@
+#include "AbstractCommand.h"
+
+#include <iostream>
+#include <cppunit/extensions/HelperMacros.h>
+
+#include "Option.h"
+#include "prefs.h"
+#include "SocketCore.h"
+#include "SocketRecvBuffer.h"
+
+namespace aria2 {
+
+class AbstractCommandTest:public CppUnit::TestFixture {
+
+  CPPUNIT_TEST_SUITE(AbstractCommandTest);
+  CPPUNIT_TEST(testGetProxyUri);
+  CPPUNIT_TEST_SUITE_END();
+public:
+  void setUp() {}
+
+  void tearDown() {}
+
+  void testGetProxyUri();
+};
+
+
+CPPUNIT_TEST_SUITE_REGISTRATION(AbstractCommandTest);
+
+void AbstractCommandTest::testGetProxyUri()
+{
+  Option op;
+  CPPUNIT_ASSERT_EQUAL(std::string(), getProxyUri("http", &op));
+  
+  op.put(PREF_HTTP_PROXY, "http://hu:hp@httpproxy/");
+  op.put(PREF_FTP_PROXY, "ftp://fu:fp@ftpproxy/");
+  CPPUNIT_ASSERT_EQUAL(std::string("http://hu:hp@httpproxy/"),
+                       getProxyUri("http", &op));
+  CPPUNIT_ASSERT_EQUAL(std::string("ftp://fu:fp@ftpproxy/"),
+                       getProxyUri("ftp", &op));
+
+  op.put(PREF_ALL_PROXY, "http://au:ap@allproxy/");
+  CPPUNIT_ASSERT_EQUAL(std::string("http://au:ap@allproxy/"),
+                       getProxyUri("https", &op));
+
+  op.put(PREF_ALL_PROXY_USER, "aunew");
+  op.put(PREF_ALL_PROXY_PASSWD, "apnew");
+  CPPUNIT_ASSERT_EQUAL(std::string("http://aunew:apnew@allproxy/"),
+                       getProxyUri("https", &op));
+
+  op.put(PREF_HTTPS_PROXY, "http://hsu:hsp@httpsproxy/");
+  CPPUNIT_ASSERT_EQUAL(std::string("http://hsu:hsp@httpsproxy/"),
+                       getProxyUri("https", &op));
+
+  CPPUNIT_ASSERT_EQUAL(std::string(), getProxyUri("unknown", &op));
+
+  op.put(PREF_HTTP_PROXY_USER, "hunew");
+  CPPUNIT_ASSERT_EQUAL(std::string("http://hunew:hp@httpproxy/"),
+                       getProxyUri("http", &op));
+
+  op.put(PREF_HTTP_PROXY_PASSWD, "hpnew");
+  CPPUNIT_ASSERT_EQUAL(std::string("http://hunew:hpnew@httpproxy/"),
+                       getProxyUri("http", &op));
+
+  op.put(PREF_HTTP_PROXY_USER, "");
+  CPPUNIT_ASSERT_EQUAL(std::string("http://httpproxy/"),
+                       getProxyUri("http", &op));
+}
+
+} // namespace aria2

test/Makefile.am

@@ -80,7 +80,8 @@ aria2c_SOURCES = AllTest.cc\
 	GeomStreamPieceSelectorTest.cc\
 	SegListTest.cc\
 	ParamedStringTest.cc\
-	RpcHelperTest.cc
+	RpcHelperTest.cc\
+	AbstractCommandTest.cc
 
 if ENABLE_XML_RPC
 aria2c_SOURCES += XmlRpcRequestParserControllerTest.cc

test/OptionHandlerTest.cc

@@ -26,8 +26,6 @@ class OptionHandlerTest:public CppUnit::TestFixture {
   CPPUNIT_TEST(testFloatNumberOptionHandler_max);
   CPPUNIT_TEST(testFloatNumberOptionHandler_min_max);
   CPPUNIT_TEST(testHttpProxyOptionHandler);
-  CPPUNIT_TEST(testHttpProxyUserOptionHandler);
-  CPPUNIT_TEST(testHttpProxyPasswdOptionHandler);
   CPPUNIT_TEST(testDeprecatedOptionHandler);
   CPPUNIT_TEST_SUITE_END();
   
@@ -47,12 +45,9 @@ public:
   void testFloatNumberOptionHandler_max();
   void testFloatNumberOptionHandler_min_max();
   void testHttpProxyOptionHandler();
-  void testHttpProxyUserOptionHandler();
-  void testHttpProxyPasswdOptionHandler();
   void testDeprecatedOptionHandler();
 };
 
-
 CPPUNIT_TEST_SUITE_REGISTRATION( OptionHandlerTest );
 
 void OptionHandlerTest::testBooleanOptionHandler()
@@ -306,92 +301,11 @@ void OptionHandlerTest::testHttpProxyOptionHandler()
   CPPUNIT_ASSERT_EQUAL(std::string("http://user%40:passwd%40@proxy:8080/"),
                        option.get(PREF_HTTP_PROXY));
 
-  option.put(PREF_HTTP_PROXY_USER, "proxy@user");
-  handler.parse(option, "http://proxy:8080");
-  CPPUNIT_ASSERT_EQUAL(std::string("http://proxy%40user@proxy:8080/"),
-                       option.get(PREF_HTTP_PROXY));
-
-  option.put(PREF_HTTP_PROXY_PASSWD, "proxy@passwd");
-  handler.parse(option, "http://proxy:8080");
-  CPPUNIT_ASSERT_EQUAL
-    (std::string("http://proxy%40user:proxy%40passwd@proxy:8080/"),
-     option.get(PREF_HTTP_PROXY));
-
-  handler.parse(option, "http://user:passwd@proxy:8080");
-  CPPUNIT_ASSERT_EQUAL(std::string("http://user:passwd@proxy:8080/"),
-                       option.get(PREF_HTTP_PROXY));
-
-  option.put(PREF_HTTP_PROXY_PASSWD, "");
-  handler.parse(option, "http://proxy:8080");
-  CPPUNIT_ASSERT_EQUAL(std::string("http://proxy%40user:@proxy:8080/"),
-                       option.get(PREF_HTTP_PROXY));
-
   handler.parse(option, "http://[::1]:8080");
-  CPPUNIT_ASSERT_EQUAL(std::string("http://proxy%40user:@[::1]:8080/"),
+  CPPUNIT_ASSERT_EQUAL(std::string("http://[::1]:8080/"),
                        option.get(PREF_HTTP_PROXY));
 }
 
-void OptionHandlerTest::testHttpProxyUserOptionHandler()
-{
-  HttpProxyUserOptionHandler handler(PREF_HTTP_PROXY_USER, "", "");
-  Option option;
-  handler.parse(option, "proxyuser");
-  CPPUNIT_ASSERT_EQUAL(std::string("proxyuser"),
-                       option.get(PREF_HTTP_PROXY_USER));
-
-  option.put(PREF_HTTP_PROXY, "http://proxy:8080");
-  handler.parse(option, "proxy@user");
-  CPPUNIT_ASSERT_EQUAL(std::string("proxy@user"),
-                       option.get(PREF_HTTP_PROXY_USER));
-  CPPUNIT_ASSERT_EQUAL(std::string("http://proxy%40user@proxy:8080"),
-                       option.get(PREF_HTTP_PROXY));
-
-  option.put(PREF_HTTP_PROXY, "http://user@proxy:8080");
-  handler.parse(option, "proxyuser");
-  CPPUNIT_ASSERT_EQUAL(std::string("http://proxyuser@proxy:8080"),
-                       option.get(PREF_HTTP_PROXY));
-
-  option.put(PREF_HTTP_PROXY, "http://user:passwd%40@proxy:8080");
-  handler.parse(option, "proxyuser");
-  CPPUNIT_ASSERT_EQUAL(std::string("http://proxyuser:passwd%40@proxy:8080"),
-                       option.get(PREF_HTTP_PROXY));
-
-  handler.parse(option, "");
-  CPPUNIT_ASSERT_EQUAL(std::string("http://:passwd%40@proxy:8080"),
-                       option.get(PREF_HTTP_PROXY));
-}
-
-void OptionHandlerTest::testHttpProxyPasswdOptionHandler()
-{
-  HttpProxyPasswdOptionHandler handler(PREF_HTTP_PROXY_PASSWD, "", "");
-  Option option;
-  handler.parse(option, "proxypasswd");
-  CPPUNIT_ASSERT_EQUAL(std::string("proxypasswd"),
-                       option.get(PREF_HTTP_PROXY_PASSWD));
-
-  option.put(PREF_HTTP_PROXY, "http://proxy:8080");
-  handler.parse(option, "proxy@passwd");
-  CPPUNIT_ASSERT_EQUAL(std::string("proxy@passwd"),
-                       option.get(PREF_HTTP_PROXY_PASSWD));
-  CPPUNIT_ASSERT_EQUAL(std::string("http://:proxy%40passwd@proxy:8080"),
-                       option.get(PREF_HTTP_PROXY));
-
-  option.put(PREF_HTTP_PROXY, "http://:pass@proxy:8080");
-  handler.parse(option, "proxypasswd");
-  CPPUNIT_ASSERT_EQUAL(std::string("http://:proxypasswd@proxy:8080"),
-                       option.get(PREF_HTTP_PROXY));
-
-  option.put(PREF_HTTP_PROXY, "http://user%40:pass@proxy:8080");
-  handler.parse(option, "proxypasswd");
-  CPPUNIT_ASSERT_EQUAL(std::string("http://user%40:proxypasswd@proxy:8080"),
-                       option.get(PREF_HTTP_PROXY));
-
-  handler.parse(option, "");
-  CPPUNIT_ASSERT_EQUAL(std::string("http://user%40:@proxy:8080"),
-                       option.get(PREF_HTTP_PROXY));
-  
-}
-
 void OptionHandlerTest::testDeprecatedOptionHandler()
 {
   {