Review Request 124140: Ported over to Netty and added SSL support

[Vineet Garg]

> On June 26, 2015, 7:57 p.m., Albert Vaca Cintora wrote:
> > src/org/kde/kdeconnect/Device.java, lines 498-503
> > <https://git.reviewboard.kde.org/r/124140/diff/1-2/?file=380829#file380829line498>
> >
> >     ?

This is when one device unpairs when network is off, and later tries to pair with other device where other device thinks it is paired. So if paired and still getting pairing request, just silently unpair and continue.


> On June 26, 2015, 7:57 p.m., Albert Vaca Cintora wrote:
> > src/org/kde/kdeconnect/Backends/LanBackend/LanLinkProvider.java, line 396
> > <https://git.reviewboard.kde.org/r/124140/diff/1-2/?file=380825#file380825line396>
> >
> >     No keepalive? Why?

It is set in TcpInitialiser.


> On June 26, 2015, 7:57 p.m., Albert Vaca Cintora wrote:
> > src/org/kde/kdeconnect/Backends/BasePairingHandler.java, line 26
> > <https://git.reviewboard.kde.org/r/124140/diff/2/?file=381381#file381381line26>
> >
> >     Could be an interface instead of an abstract class.

Then implenting classes need to implement all methods, they might not want to.


> On June 26, 2015, 7:57 p.m., Albert Vaca Cintora wrote:
> > src/org/kde/kdeconnect/Backends/LanBackend/LanPairingHandler.java, line 54
> > <https://git.reviewboard.kde.org/r/124140/diff/2/?file=381384#file381384line54>
> >
> >     This function is called requestPairing but does not request anything, it just puts info in a NP. See my comments about this below.

All these methods are named same as their calling methods for ease of remembrance. If their names are not matching what they are doing, what about prefixing them with an "on" like onRequestPairing?


> On June 26, 2015, 7:57 p.m., Albert Vaca Cintora wrote:
> > src/org/kde/kdeconnect/Device.java, lines 244-251
> > <https://git.reviewboard.kde.org/r/124140/diff/1-2/?file=380829#file380829line244>
> >
> >     We don't need to call sendPackage in Device, the link can do it because they have a similar (but lower-level) function. Also, this allows us to have Links that don't even need to send a packet to pair, because they use some other mechanism. If we make this change, Device should only allow sending packets if the device is paired.

But still we want the device to fully paired with all the links, that is why pair package traverses through all link providers. If we dont send a pair package when the link doesn't need it, it will create problems to other links. Current protocol is like, create a meta pair package to pair or unpair fully.


- Vineet


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124140/#review81789
-----------------------------------------------------------


On June 25, 2015, 2:35 p.m., Vineet Garg wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124140/
> -----------------------------------------------------------
> 
> (Updated June 25, 2015, 2:35 p.m.)
> 
> 
> Review request for kdeconnect and Albert Vaca Cintora.
> 
> 
> Repository: kdeconnect-android
> 
> 
> Description
> -------
> 
> * Since MINA was tying hands in adding SSL, ported over Netty which has a good, clean and easy interface and a larger user and developer base.
> * Added support to setup links on SSL
> * Links automatically removed if wrong certificate is sent
> * Shows keys based on hash of certificate to check right certificates are received
> * Added a preference to use SSL, as it is experienced to cause high CPU usage on devices.
> * Corrected unit tests, LanLinkProvider is removed as it is nearly impossible to write it with current design. Will find a way
> 
> 
> Diffs
> -----
> 
>   build.gradle 6520c1c 
>   res/values/strings.xml c128342 
>   src/org/kde/kdeconnect/Backends/BaseLink.java 579a7af 
>   src/org/kde/kdeconnect/Backends/BaseLinkProvider.java cfaf621 
>   src/org/kde/kdeconnect/Backends/BasePairingHandler.java PRE-CREATION 
>   src/org/kde/kdeconnect/Backends/LanBackend/LanLink.java 5994142 
>   src/org/kde/kdeconnect/Backends/LanBackend/LanLinkProvider.java ae26958 
>   src/org/kde/kdeconnect/Backends/LanBackend/LanPairingHandler.java PRE-CREATION 
>   src/org/kde/kdeconnect/Backends/LoopbackBackend/LoopbackLink.java add92f8 
>   src/org/kde/kdeconnect/Backends/LoopbackBackend/LoopbackLinkProvider.java bd9c41b 
>   src/org/kde/kdeconnect/Backends/LoopbackBackend/LoopbackPairingHandler.java PRE-CREATION 
>   src/org/kde/kdeconnect/BackgroundService.java 5e3d8c2 
>   src/org/kde/kdeconnect/Device.java 3262aef 
>   src/org/kde/kdeconnect/Helpers/SecurityHelpers/RsaHelper.java PRE-CREATION 
>   src/org/kde/kdeconnect/Helpers/SecurityHelpers/SslHelper.java PRE-CREATION 
>   src/org/kde/kdeconnect/NetworkPackage.java f62b5d0 
>   src/org/kde/kdeconnect/UserInterface/PairActivity.java 7a45751 
>   tests/org/kde/kdeconnect/DeviceTest.java 5d3383d 
>   tests/org/kde/kdeconnect/LanLinkProviderTest.java fe7863a 
>   tests/org/kde/kdeconnect/LanLinkTest.java d3d94c9 
>   tests/org/kde/kdeconnect/NetworkPackageTest.java a21114e 
> 
> Diff: https://git.reviewboard.kde.org/r/124140/diff/
> 
> 
> Testing
> -------
> 
> Tesed on some device with where both supports SSL, also with PC where ssl is not supported, working fine. Need a little bit more testing with more devices.
> 
> 
> Thanks,
> 
> Vineet Garg
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdeconnect/attachments/20150626/8d5da5df/attachment-0001.html>