Good catch!

But nack for the current implementation of the fix:

If options.allow_unsecure() == True and options.allow_secure() == False,
and the unsecure connection fails with error_code != 
SPICEC_ERROR_CODE_VERSION_MISMATCH
we will exit RedChannelBase::connect without throwing an error while
we've not established a connection.

Regards,

Hans


On 12/07/2010 10:47 AM, Arnon Gilboa wrote:
If connect_unsecure failed due to protocol version mismatch, don't try to
connect_secure with the same version, but retry (connect_secure or
connect_unsecure) with older version.

This solves the following bug: when "new" Spice client (protocol version 2)
with given secure_port connects to "old" server which is not using the same
secure_port (or not using a secure_port at all), the client exits immediately.

In this scenario, the client first tries to use Spice protocol version 2 to
connect the unsecure port, and altough this fails due to version mismatch, it
tries to connect to the secure port with the same protocol version 2, which is
a wrong behavior, fails due to socket error 10061 (WSAECONNREFUSED -
Connection refused) and handled mistakenly by immediate exit, instead of
retrying with protocol version 1.
---
  client/red_channel.cpp |   22 ++++++++++++----------
  1 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/client/red_channel.cpp b/client/red_channel.cpp
index 273b28d..2886505 100644
--- a/client/red_channel.cpp
+++ b/client/red_channel.cpp
@@ -217,20 +217,22 @@ void RedChannelBase::connect(const ConnectionOptions&  
options, uint32_t connecti
                  RedPeer::connect_unsecure(host, options.unsecure_port);
                  link(connection_id, password, protocol);
                  return;
-            } catch (...) {
-                if (!options.allow_secure()) {
+            } catch (Exception&  e) {
+                // On protocol version mismatch, don't connect_secure with the 
same version
+                if (e.get_error_code() == SPICEC_ERROR_CODE_VERSION_MISMATCH) {
                      throw;
-                }
-                RedPeer::close();
+                }
              }
          }
-        ASSERT(options.allow_secure());
-        RedPeer::connect_secure(options, host);
-        link(connection_id, password, protocol);
+        if (options.allow_secure()) {
+            RedPeer::close();
+            RedPeer::connect_secure(options, host);
+            link(connection_id, password, protocol);
+        }
      } catch (Exception&  e) {
-        if (protocol == 2&&
-            options.protocol == 0&&
-            e.get_error_code() == SPICEC_ERROR_CODE_VERSION_MISMATCH) {
+        // On protocol version mismatch, retry with older version
+        if (e.get_error_code() == SPICEC_ERROR_CODE_VERSION_MISMATCH&&
+                                 protocol == 2&&  options.protocol == 0) {
              RedPeer::cleanup();
              protocol = 1;
              goto retry;
_______________________________________________
Spice-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/spice-devel

Reply via email to