robustify and regularize proxy fallback attempts

direct/src/distributed/ConnectionRepository.py

@@ -30,14 +30,14 @@ class ConnectionRepository(DirectObject.DirectObject):
         # (e.g. QueuedConnectionManager, etc.) to establish the
         # connection, which ultimately uses the NSPR socket library.
         # This is a much better socket library, but it may be more
-        # than you need for most applications; and the proxy support
-        # is weak.
+        # than you need for most applications; and there is no support
+        # for proxies.
 
         # Set it to 'default' to use the HTTPClient interface if a
         # proxy is in place, but the NSPR interface if we don't have a
         # proxy.
-        
         self.connectMethod = self.config.GetString('connect-method', 'default')
+        
         self.connectHttp = None
         self.http = None
         self.qcm = None
@@ -52,8 +52,7 @@ class ConnectionRepository(DirectObject.DirectObject):
         self.rsDoReport = self.config.GetBool('reader-statistics', 0)
         self.rsUpdateInterval = self.config.GetDouble('reader-statistics-interval', 10)
 
-
-    def connect(self, serverList, allowProxy,
+    def connect(self, serverList, 
                 successCallback = None, successArgs = [],
                 failureCallback = None, failureArgs = []):
         """
@@ -67,14 +66,12 @@ class ConnectionRepository(DirectObject.DirectObject):
         """
 
         hasProxy = 0
-        if allowProxy:
-            if self.http == None:
-                self.http = HTTPClient()
+        if self.checkHttp():
             proxies = self.http.getProxiesForUrl(serverList[0])
-            hasProxy = (proxies != '')
+            hasProxy = (proxies != 'DIRECT')
 
         if hasProxy:
-            self.notify.info("Connecting to gameserver via proxy: %s" % (proxies))
+            self.notify.info("Connecting to gameserver via proxy list: %s" % (proxies))
         else:
             self.notify.info("Connecting to gameserver directly (no proxy).");
 
@@ -96,16 +93,8 @@ class ConnectionRepository(DirectObject.DirectObject):
             # itself repeatedly until we establish a connection (or
             # run out of servers).
 
-            if self.http == None:
-                self.http = HTTPClient()
-
             ch = self.http.makeChannel(0)
-            # Temporary try..except for old Pandas.
-            try:
-                ch.setAllowProxy(allowProxy)
-            except:
-                pass
-            self.httpConnectCallback(ch, serverList, 0, hasProxy,
+            self.httpConnectCallback(ch, serverList, 0,
                                      successCallback, successArgs,
                                      failureCallback, failureArgs)
 
@@ -142,7 +131,7 @@ class ConnectionRepository(DirectObject.DirectObject):
 
             # Failed to connect.
             if failureCallback:
-                failureCallback(hasProxy, 0, *failureArgs)
+                failureCallback(0, *failureArgs)
 
     def disconnect(self):
         """Closes the previously-established connection.
@@ -156,7 +145,7 @@ class ConnectionRepository(DirectObject.DirectObject):
             self.tcpConn = None
         self.stopReaderPollTask()
                     
-    def httpConnectCallback(self, ch, serverList, serverIndex, hasProxy,
+    def httpConnectCallback(self, ch, serverList, serverIndex,
                             successCallback, successArgs,
                             failureCallback, failureArgs):
         if ch.isConnectionReady():
@@ -175,13 +164,26 @@ class ConnectionRepository(DirectObject.DirectObject):
             ch.spawnTask(name = 'connect-to-server',
                          callback = self.httpConnectCallback,
                          extraArgs = [ch, serverList, serverIndex + 1,
-                                      hasProxy,
                                       successCallback, successArgs,
                                       failureCallback, failureArgs])
         else:
             # No more servers to try; we have to give up now.
             if failureCallback:
-                failureCallback(hasProxy, ch.getStatusCode(), *failureArgs)
+                failureCallback(ch.getStatusCode(), *failureArgs)
+
+    def checkHttp(self):
+        # Creates an HTTPClient, if possible, if we don't have one
+        # already.  This might fail if the OpenSSL library isn't
+        # available.  Returns the HTTPClient (also self.http), or None
+        # if not set.
+        
+        if self.http == None:
+            try:
+                self.http = HTTPClient()
+            except:
+                pass
+
+        return self.http
 
     def startReaderPollTask(self):
         # Stop any tasks we are running now

panda/src/downloader/httpChannel.cxx

@@ -245,8 +245,8 @@ run() {
 
   if (downloader_cat.is_spam()) {
     downloader_cat.spam()
-      << "begin run(), _state = " << (int)_state << ", _done_state = "
-      << (int)_done_state << "\n";
+      << "begin run(), _state = " << _state << ", _done_state = "
+      << _done_state << "\n";
   }
 
   if (_state == _done_state) {
@@ -295,7 +295,7 @@ run() {
 
     if (downloader_cat.is_spam()) {
       downloader_cat.spam()
-        << "continue run(), _state = " << (int)_state << "\n";
+        << "continue run(), _state = " << _state << "\n";
     }
 
     switch (_state) {
@@ -381,7 +381,7 @@ run() {
       
     default:
       downloader_cat.warning()
-        << "Unhandled state " << (int)_state << "\n";
+        << "Unhandled state " << _state << "\n";
       return false;
     }
 
@@ -394,8 +394,8 @@ run() {
 
   if (downloader_cat.is_spam()) {
     downloader_cat.spam()
-      << "later run(), _state = " << (int)_state
-      << ", _done_state = " << (int)_done_state << "\n";
+      << "later run(), _state = " << _state
+      << ", _done_state = " << _done_state << "\n";
   }
   return true;
 }
@@ -623,8 +623,8 @@ bool HTTPChannel::
 reached_done_state() {
   if (downloader_cat.is_spam()) {
     downloader_cat.spam()
-      << "terminating run(), _state = " << (int)_state
-      << ", _done_state = " << (int)_done_state << "\n";
+      << "terminating run(), _state = " << _state
+      << ", _done_state = " << _done_state << "\n";
   }
 
   if (_state == S_failure || _download_dest == DD_none) {
@@ -665,6 +665,7 @@ run_try_next_proxy() {
     _proxy_auth = (HTTPAuthorization *)NULL;
     _proxy_next_index++;
     close_connection();
+    reconsider_proxy();
     _state = S_connecting;
     return false;
   }
@@ -684,6 +685,7 @@ bool HTTPChannel::
 run_connecting() {
   _status_code = 0;
   _status_string = string();
+
   if (BIO_do_connect(*_bio) <= 0) {
     if (BIO_should_retry(*_bio)) {
       _state = S_connecting_wait;
@@ -1884,6 +1886,36 @@ begin_request(HTTPEnum::Method method, const DocumentSpec &url,
   // connection.
   _want_ssl = _request.get_url().is_ssl();
 
+  _first_byte = first_byte;
+  _last_byte = last_byte;
+  _connect_count = 0;
+
+  reconsider_proxy();
+
+  // Also, reset from whatever previous request might still be pending.
+  if (_state == S_failure || (_state < S_read_header && _state != S_ready)) {
+    reset_to_new();
+
+  } else if (_state == S_read_header) {
+    // Roll one step forwards to start skipping past the previous
+    // body.
+    _state = S_begin_body;
+  }
+
+  _done_state = (_method == HTTPEnum::M_connect) ? S_ready : S_read_header;
+}
+
+////////////////////////////////////////////////////////////////////
+//     Function: HTTPChannel::reconsider_proxy
+//       Access: Private
+//  Description: Reevaluates the flags and strings that are computed
+//               based on the particular proxy we are attempting to
+//               connect to.  This should be called when we initiate a
+//               request, and also whenever we change proxies while
+//               processing a request.
+////////////////////////////////////////////////////////////////////
+void HTTPChannel::
+reconsider_proxy() {
   _proxy_tunnel = false;
   _proxy_serves_document = false;
 
@@ -1900,10 +1932,6 @@ begin_request(HTTPEnum::Method method, const DocumentSpec &url,
     _proxy_serves_document = !_proxy_tunnel;
   }
 
-  _first_byte = first_byte;
-  _last_byte = last_byte;
-  _connect_count = 0;
-
   make_header();
   make_request_text();
 
@@ -1925,20 +1953,9 @@ begin_request(HTTPEnum::Method method, const DocumentSpec &url,
     _proxy_header = string();
     _proxy_request_text = string();
   }
-
-  // Also, reset from whatever previous request might still be pending.
-  if (_state == S_failure || (_state < S_read_header && _state != S_ready)) {
-    reset_to_new();
-
-  } else if (_state == S_read_header) {
-    // Roll one step forwards to start skipping past the previous
-    // body.
-    _state = S_begin_body;
-  }
-
-  _done_state = (_method == HTTPEnum::M_connect) ? S_ready : S_read_header;
 }
 
+
 ////////////////////////////////////////////////////////////////////
 //     Function: HTTPChannel::reset_for_new_request
 //       Access: Private
@@ -3051,4 +3068,86 @@ close_connection() {
   _read_index++;
 }
 
+
+////////////////////////////////////////////////////////////////////
+//     Function: HTTPChannel::State output operator
+//  Description: 
+////////////////////////////////////////////////////////////////////
+ostream &
+operator << (ostream &out, HTTPChannel::State state) {
+#ifdef NDEBUG
+  return out << (int)state;
+#else
+  switch (state) {
+  case HTTPChannel::S_new:
+    return out << "new";
+
+  case HTTPChannel::S_try_next_proxy:
+    return out << "try_next_proxy";
+
+  case HTTPChannel::S_connecting:
+    return out << "connecting";
+
+  case HTTPChannel::S_connecting_wait:
+    return out << "connecting_wait";
+
+  case HTTPChannel::S_http_proxy_ready:
+    return out << "http_proxy_ready";
+
+  case HTTPChannel::S_http_proxy_request_sent:
+    return out << "http_proxy_request_sent";
+
+  case HTTPChannel::S_http_proxy_reading_header:
+    return out << "http_proxy_reading_header";
+
+  case HTTPChannel::S_socks_proxy_greet:
+    return out << "socks_proxy_greet";
+
+  case HTTPChannel::S_socks_proxy_greet_reply:
+    return out << "socks_proxy_greet_reply";
+
+  case HTTPChannel::S_socks_proxy_connect:
+    return out << "socks_proxy_connect";
+
+  case HTTPChannel::S_socks_proxy_connect_reply:
+    return out << "socks_proxy_connect_reply";
+
+  case HTTPChannel::S_setup_ssl:
+    return out << "setup_ssl";
+
+  case HTTPChannel::S_ssl_handshake:
+    return out << "ssl_handshake";
+
+  case HTTPChannel::S_ready:
+    return out << "ready";
+
+  case HTTPChannel::S_request_sent:
+    return out << "request_sent";
+
+  case HTTPChannel::S_reading_header:
+    return out << "reading_header";
+
+  case HTTPChannel::S_read_header:
+    return out << "read_header";
+
+  case HTTPChannel::S_begin_body:
+    return out << "begin_body";
+
+  case HTTPChannel::S_reading_body:
+    return out << "reading_body";
+
+  case HTTPChannel::S_read_body:
+    return out << "read_body";
+
+  case HTTPChannel::S_read_trailer:
+    return out << "read_trailer";
+
+  case HTTPChannel::S_failure:
+    return out << "failure";
+  }
+
+  return out << "invalid state(" << (int)state << ")";
+#endif  // NDEBUG
+}
+
 #endif  // HAVE_SSL

panda/src/downloader/httpChannel.h

@@ -180,6 +180,7 @@ private:
   void begin_request(HTTPEnum::Method method, const DocumentSpec &url, 
                      const string &body, bool nonblocking,
                      size_t first_byte, size_t last_byte);
+  void reconsider_proxy();
   void reset_for_new_request();
 
   void finished_body(bool has_trailer);
@@ -215,6 +216,35 @@ private:
   void reset_to_new();
   void close_connection();
 
+public:
+  // This is declared public solely so we can make an ostream operator
+  // for it.
+  enum State {
+    S_new,
+    S_try_next_proxy,
+    S_connecting,
+    S_connecting_wait,
+    S_http_proxy_ready,
+    S_http_proxy_request_sent,
+    S_http_proxy_reading_header,
+    S_socks_proxy_greet,
+    S_socks_proxy_greet_reply,
+    S_socks_proxy_connect,
+    S_socks_proxy_connect_reply,
+    S_setup_ssl,
+    S_ssl_handshake,
+    S_ready,
+    S_request_sent,
+    S_reading_header,
+    S_read_header,
+    S_begin_body,
+    S_reading_body,
+    S_read_body,
+    S_read_trailer,
+    S_failure
+  };
+
+private:
   typedef pvector<URLSpec> Proxies;
 
   HTTPClient *_client;
@@ -301,30 +331,6 @@ private:
   // case of nonblocking I/O we have to be able to return to the
   // caller after any I/O operation and resume later where we left
   // off.
-  enum State {
-    S_new,
-    S_try_next_proxy,
-    S_connecting,
-    S_connecting_wait,
-    S_http_proxy_ready,
-    S_http_proxy_request_sent,
-    S_http_proxy_reading_header,
-    S_socks_proxy_greet,
-    S_socks_proxy_greet_reply,
-    S_socks_proxy_connect,
-    S_socks_proxy_connect_reply,
-    S_setup_ssl,
-    S_ssl_handshake,
-    S_ready,
-    S_request_sent,
-    S_reading_header,
-    S_read_header,
-    S_begin_body,
-    S_reading_body,
-    S_read_body,
-    S_read_trailer,
-    S_failure
-  };
   State _state;
   State _done_state;
   double _started_connecting_time;
@@ -364,6 +370,8 @@ private:
   friend class HTTPClient;
 };
 
+ostream &operator << (ostream &out, HTTPChannel::State state);
+
 #include "httpChannel.I"
 
 #endif  // HAVE_SSL

panda/src/downloader/httpClient.cxx

@@ -226,10 +226,12 @@ get_proxy() const {
 //               schemes.  This is either a semicolon-delimited set of
 //               hostname:ports, or a semicolon-delimited set of pairs
 //               of the form "scheme=hostname:port", or a combination.
-//               A particular scheme and/or proxy host may be listed
-//               more than once.  This is a convenience function that
-//               can be used in place of explicit calls to add_proxy()
-//               for each scheme/proxy pair.
+//               Use the keyword DIRECT, or an empty string, to
+//               represent a direct connection.  A particular scheme
+//               and/or proxy host may be listed more than once.  This
+//               is a convenience function that can be used in place
+//               of explicit calls to add_proxy() for each
+//               scheme/proxy pair.
 ////////////////////////////////////////////////////////////////////
 void HTTPClient::
 set_proxy_spec(const string &proxy_spec) {
@@ -246,17 +248,21 @@ set_proxy_spec(const string &proxy_spec) {
 
     // Divide out the scheme and the hostname.
     string scheme;
-    URLSpec url;
+    string proxy;
     size_t equals = spec.find('=');
     if (equals == string::npos) {
       scheme = "";
-      url = URLSpec(spec, true);
+      proxy = trim_blanks(spec);
     } else {
       scheme = trim_blanks(spec.substr(0, equals));
-      url = URLSpec(spec.substr(equals + 1), true);
+      proxy = trim_blanks(spec.substr(equals + 1));
     }
 
-    add_proxy(scheme, url);
+    if (proxy == "DIRECT" || proxy.empty()) {
+      add_proxy(scheme, URLSpec());
+    } else {
+      add_proxy(scheme, URLSpec(proxy, true));
+    }
   }
 }
 
@@ -289,7 +295,11 @@ get_proxy_spec() const {
         result += scheme;
         result += "=";
       }
-      result += url.get_url();
+      if (url.empty()) {
+        result += "DIRECT";
+      } else {
+        result += url.get_url();
+      }
     }
   }
 
@@ -369,26 +379,32 @@ clear_proxy() {
 //  Description: Adds the indicated proxy host as a proxy for
 //               communications on the given scheme.  Usually the
 //               scheme is "http" or "https".  It may be the empty
-//               string to indicate a general proxy.
+//               string to indicate a general proxy.  The proxy string
+//               may be the empty URL to indicate a direct connection.
 ////////////////////////////////////////////////////////////////////
 void HTTPClient::
 add_proxy(const string &scheme, const URLSpec &proxy) {
-  if (!proxy.empty()) {
-    URLSpec proxy_url(proxy);
-
-    // The scheme is always converted to lowercase.
-    string lc_scheme;
-    lc_scheme.reserve(scheme.length());
-    string::const_iterator si;
-    for (si = scheme.begin(); si != scheme.end(); ++si) {
-      lc_scheme += tolower(*si);
-    }
-    
-    // Remove the trailing colon, if there is one.
-    if (!lc_scheme.empty() && lc_scheme[lc_scheme.length() - 1] == ':') {
-      lc_scheme = lc_scheme.substr(0, lc_scheme.length() - 1);
-    }
-    
+  URLSpec proxy_url(proxy);
+  
+  // The scheme is always converted to lowercase.
+  string lc_scheme;
+  lc_scheme.reserve(scheme.length());
+  string::const_iterator si;
+  for (si = scheme.begin(); si != scheme.end(); ++si) {
+    lc_scheme += tolower(*si);
+  }
+  
+  // Remove the trailing colon, if there is one.
+  if (!lc_scheme.empty() && lc_scheme[lc_scheme.length() - 1] == ':') {
+    lc_scheme = lc_scheme.substr(0, lc_scheme.length() - 1);
+  }
+
+  if (!proxy_url.empty()) {
+    // Enforce the scheme that we use to communicate to the proxy
+    // itself.  This is not the same as lc_scheme, which is the scheme
+    // of the requested connection.  Generally, all proxies speak
+    // HTTP, except for Socks proxies.
+
     if (lc_scheme == "socks") {
       // Scheme "socks" implies we talk to the proxy via the "socks"
       // scheme, no matter what scheme the user actually specified.
@@ -399,9 +415,9 @@ add_proxy(const string &scheme, const URLSpec &proxy) {
       // proxy, the default is "http".
       proxy_url.set_scheme("http");
     }
-    
-    _proxies_by_scheme[lc_scheme].push_back(proxy_url);
   }
+  
+  _proxies_by_scheme[lc_scheme].push_back(proxy_url);
 }
 
 ////////////////////////////////////////////////////////////////////
@@ -444,9 +460,9 @@ add_direct_host(const string &hostname) {
 //       Access: Published
 //  Description: Fills up the indicated vector with the list of
 //               URLSpec objects, in the order in which they should be
-//               tried, that are appropriate for the indicated URL.
-//               The vector is left empty if a direct connection
-//               should be used.
+//               tried, that are appropriate proxies to try for the
+//               indicated URL.  The empty URL is returned for a
+//               direct connection.
 //
 //               It is the user's responsibility to empty this vector
 //               before calling this method; otherwise, the proxy
@@ -464,51 +480,64 @@ get_proxies_for_url(const URLSpec &url, pvector<URLSpec> &proxies) const {
     for (si = _direct_hosts.begin(); si != _direct_hosts.end(); ++si) {
       if ((*si).matches(hostname)) {
         // It matches, so don't use any proxies.
+        proxies.push_back(URLSpec());
         return;
       }
     }
   }
 
+  // Build our list of proxies into a temporary vector, so we can pull
+  // out duplicates later.
+  pvector<URLSpec> temp_list;
+
   // Now choose the appropriate proxy based on the scheme.
   string scheme = url.get_scheme();
   bool got_any = false;
 
-  if (scheme.empty()) {
-    // An empty scheme implies we will want to make a direct
-    // connection to this host, so we will need a socks-style or
-    // https-style scheme.
-    if (get_proxies_for_scheme("socks", proxies)) {
+  if (!scheme.empty()) {
+    // If we have a scheme, try to match it.
+    if (get_proxies_for_scheme(scheme, temp_list)) {
       got_any = true;
     }
-    if (get_proxies_for_scheme("https", proxies)) {
+  }
+
+  if (!got_any && (scheme.empty() || url.is_ssl())) {
+    // An empty scheme (or an ssl-style scheme) implies we will need
+    // to make a direct connection, so fallback to a socks-style
+    // and/or https-style scheme.
+
+    if (get_proxies_for_scheme("socks", temp_list)) {
       got_any = true;
     }
-
-  } else {
-    // Otherwise, try to match the proxy to the scheme.
-    if (get_proxies_for_scheme(scheme, proxies)) {
+    if (get_proxies_for_scheme("https", temp_list)) {
       got_any = true;
     }
   }
 
-  // If we didn't find our scheme of choice, fall back to the default
-  // proxy type.
   if (!got_any) {
-    if (get_proxies_for_scheme("", proxies)) {
+    // If we didn't find our scheme of choice, fall back to the default
+    // proxy type, if we've got one.
+    if (get_proxies_for_scheme("", temp_list)) {
       got_any = true;
     }
   }
 
-  // Failing that, use SOCKS for an SSL-type connection.
-  if (!got_any && scheme == "https") {
-    if (get_proxies_for_scheme("socks", proxies)) {
-      got_any = true;
-    }
-  }
+  // We always try a direct connection if all else fails.
+  temp_list.push_back(URLSpec());
 
-  // And failing that, try the http proxy.
+  // Finally, as a very last resort, fall back to the HTTP proxy.
   if (!got_any) {
-    get_proxies_for_scheme("http", proxies);
+    get_proxies_for_scheme("http", temp_list);
+  }
+
+  // Now remove duplicate entries as we copy the resulting list out.
+  pvector<URLSpec>::iterator pi;
+  pset<URLSpec> used;
+  for (pi = temp_list.begin(); pi != temp_list.end(); ++pi) {
+    if (used.insert(*pi).second) {
+      // This is a unique one.
+      proxies.push_back(*pi);
+    }
   }
 }
 
@@ -517,8 +546,8 @@ get_proxies_for_url(const URLSpec &url, pvector<URLSpec> &proxies) const {
 //       Access: Published
 //  Description: Returns a semicolon-delimited list of proxies, in the
 //               order in which they should be tried, that are
-//               appropriate for the indicated URL.  The empty string
-//               is returned if a direct connection should be used.
+//               appropriate for the indicated URL.  The keyword
+//               DIRECT indicates a direct connection should be tried.
 ////////////////////////////////////////////////////////////////////
 string HTTPClient::
 get_proxies_for_url(const URLSpec &url) const {
@@ -528,12 +557,20 @@ get_proxies_for_url(const URLSpec &url) const {
   string result;
   if (!proxies.empty()) {
     pvector<URLSpec>::const_iterator pi = proxies.begin();
-    result += (*pi).get_url();
+    if ((*pi).get_url().empty()) {
+      result += "DIRECT";
+    } else {
+      result += (*pi).get_url();
+    }
     ++pi;
 
     while (pi != proxies.end()) {
       result += ";";
-      result += (*pi).get_url();
+      if ((*pi).get_url().empty()) {
+        result += "DIRECT";
+      } else {
+        result += (*pi).get_url();
+      }
       ++pi;
     }
   }