Review for gkiagia's krdc port to Tp::StreamTubeClient

[Olli Salli]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 07.10.2011 20:10, George Kiagiadakis wrote:
> On Fri, Oct 7, 2011 at 5:41 PM, Olli Salli <olli.salli at collabora.co.uk> wrote:
>>>     connect(m_stubeClient.data(),
>>> SIGNAL(tubeAcceptedAsTcp(QHostAddress,quint16,QHostAddress,quint16,
>>
>> Yeah, this is all that's needed for basic operation. But I think you
>> should also connect to tubeClosed to clean up from your hash tubes which
>> WE don't close, but get closed for other reasons. E.g. server connection
>> reconnect, or the remote going away.
>>
>> Or are you confident that closeTube will always get invoked in all those
>> occasions via some other mechanism? (Likely: getting disconnect event /
>> error on the server socket - but would that really call the functions to
>> INITIATE a disconnect like disconnectHost or closeTab?)
> 
> I think you are right, I should use the signal. Although I think it
> should work fine like this, because the closed socket will cause a
> disconnect, which will close the active tab (although this actually
> requires a patch that is not in krdc yet; in the current situation the
> tab is not closed automatically, which leaves krdc in a non-consistent
> state)

Ah, then fixing that bug should be enough, but you might want to listen
to the signal anyway to make sure.

> 
>> You might at this point want to investigate using the parameters for
>> passing some kind of credentials to avoid accidental / undesired access
>> to the rfb socket, as we discussed in the krfb review. Setting them when
>> exporting the socket in krfb, you could recover them tube->parameters()
>> in your onTubeAccepted() slot, then use to do rfb authentication or
>> whatever (as much as that's supported by the protocol libraries used).
> 
> Yeah, I should do that, although in the current design of krdc this is
> not so easy. It will require some extra changes in other parts of
> krdc.
> 

Right, can come later as an added security feature.

>> understanding enough of the spec to write the .client files still
>> remains an issue, though
> 
> I think that a simple script would be enough. Maybe together with some
> nice cmake macro which will be exported in TelepathyQt4Config.cmake
> (which I hope to have ready for merge soon...)
> 

Right, a script would probably be enough indeed. Let's discuss it again
when we have the Config file, it's in no way a blocker for the library
parts.

As a whole the krdc patch gets a ++ from me, after adding the tube close
signal handling and making it a TODO to improve the security aspects.

Br,
Olli
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJOkCwfAAoJEAQQkupGanj4ZCMIAJDILriW6T9iA6nkVb+eWQqk
PHHhXXtPSGaCOLF2I0Xj1HJrnRogUJNhoZ76w4eTtdrtaXG3J2L3LFOVmAZy5/WK
jTCQ4oqkbUsYuAEoD8AmVtqezmuAjMII+z46n+OeHxhipSGjR1bncYvKVVD1fDep
1M3RRmB4GQ2xzkmxjGu0OvFjZgH/QYkmrxg/iuyymMZfWN1DwxAWn5NvbJsCvydm
H3/4eYaos7O5zJOh2CxJWMK9vCJVr75V1RX7XKyAIC/76t3NNtcGZnllF+oYRTcj
iWXiWOmVe08zJu+N5s/1Fpd35UfVV5bqG4zIDlCJDSTR3vjsM7fPyiC3aviOTPU=
=+hWU
-----END PGP SIGNATURE-----