[Spice-devel] [PATCH spice 1/1] spicec: do not call connect_secure when connect_unsecure fails due to protocol version mismatch

[Arnon Gilboa]

10x, u r absolutely right.

Hans de Goede wrote:
> 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;