From ac4f072322290ffe876a43e69ff9b800c5d4176f Mon Sep 17 00:00:00 2001 From: Albert Vaca Date: Fri, 17 Jun 2016 02:01:20 +0200 Subject: [PATCH] Code cleanup --- .../Backends/LanBackend/LanLink.java | 82 +++++------ .../Backends/LanBackend/LanLinkProvider.java | 129 ++++++------------ .../Helpers/NotificationsHelper.java | 12 -- .../kde/kdeconnect/Helpers/StringsHelper.java | 9 ++ 4 files changed, 89 insertions(+), 143 deletions(-) delete mode 100644 src/org/kde/kdeconnect/Helpers/NotificationsHelper.java create mode 100644 src/org/kde/kdeconnect/Helpers/StringsHelper.java diff --git a/src/org/kde/kdeconnect/Backends/LanBackend/LanLink.java b/src/org/kde/kdeconnect/Backends/LanBackend/LanLink.java index f4123b07..a2441ee4 100644 --- a/src/org/kde/kdeconnect/Backends/LanBackend/LanLink.java +++ b/src/org/kde/kdeconnect/Backends/LanBackend/LanLink.java @@ -30,6 +30,7 @@ import org.kde.kdeconnect.Backends.BasePairingHandler; import org.kde.kdeconnect.Device; import org.kde.kdeconnect.Helpers.SecurityHelpers.RsaHelper; import org.kde.kdeconnect.Helpers.SecurityHelpers.SslHelper; +import org.kde.kdeconnect.Helpers.StringsHelper; import org.kde.kdeconnect.NetworkPackage; import java.io.BufferedReader; @@ -45,6 +46,7 @@ import java.nio.channels.NotYetConnectedException; import java.security.PublicKey; import javax.net.ssl.SSLContext; +import javax.net.ssl.SSLException; import javax.net.ssl.SSLServerSocketFactory; import javax.net.ssl.SSLSocket; @@ -59,62 +61,53 @@ public class LanLink extends BaseLink { // because it's probably trying to find me and // potentially ask for pairing. - private Socket channel = null; - - Cancellable readThread; - - public abstract class Cancellable implements Runnable { - protected volatile boolean cancelled; - public void cancel() - { - cancelled = true; - } - } + private Socket socket = null; @Override public void disconnect() { - if (readThread != null) { - readThread.cancel(); - } - if (channel == null) { - Log.e("KDE/LanLink", "Not yet connected"); + + Log.e("Disconnect","socket:"+ socket.hashCode()); + + if (socket == null) { + Log.w("KDE/LanLink", "Not yet connected"); return; } + try { - channel.close(); + socket.close(); } catch (IOException e) { e.printStackTrace(); } + } - //Returns the old channel - public Socket reset(final Socket channel, ConnectionStarted connectionSource, final LanLinkProvider linkProvider) throws IOException { + //Returns the old socket + public Socket reset(final Socket newSocket, ConnectionStarted connectionSource, final LanLinkProvider linkProvider) throws IOException { + + Socket oldSocket = socket; + socket = newSocket; - Socket oldChannel = this.channel; - this.channel = channel; this.connectionSource = connectionSource; - if (oldChannel != null) { - readThread.cancel(); - oldChannel.close(); + if (oldSocket != null) { + oldSocket.close(); //This should cancel the readThread } Log.e("LanLink", "Start listening"); //Start listening - readThread = new Cancellable() { + new Thread(new Runnable() { @Override public void run() { try { - BufferedReader reader = new BufferedReader(new InputStreamReader(channel.getInputStream(), LanLinkProvider.UTF8)); - while (!cancelled) { - if (channel.isClosed()) { + BufferedReader reader = new BufferedReader(new InputStreamReader(newSocket.getInputStream(), StringsHelper.UTF8)); + while (true) { + if (socket.isClosed()) { Log.e("BufferReader", "Channel closed"); break; } String packet; try { packet = reader.readLine(); - Log.e("packet", "A" + packet); } catch (SocketTimeoutException e) { Log.w("BufferReader", "timeout"); continue; @@ -134,19 +127,18 @@ public class LanLink extends BaseLink { e.printStackTrace(); } - Log.e("LanLink", "Socket closed"); - linkProvider.socketClosed(channel); + Log.e("LanLink", "Socket closed"+newSocket.hashCode()); + boolean thereIsaANewSocket = (newSocket != socket); + linkProvider.socketClosed(newSocket, thereIsaANewSocket); } - }; - new Thread(readThread).start(); + }).start(); - - return oldChannel; + return oldSocket; } - public LanLink(Context context, String deviceId, LanLinkProvider linkProvider, Socket channel, ConnectionStarted connectionSource) throws IOException { + public LanLink(Context context, String deviceId, LanLinkProvider linkProvider, Socket socket, ConnectionStarted connectionSource) throws IOException { super(context, deviceId, linkProvider); - reset(channel, connectionSource, linkProvider); + reset(socket, connectionSource, linkProvider); } @@ -162,7 +154,7 @@ public class LanLink extends BaseLink { //Blocking, do not call from main thread private void sendPackageInternal(NetworkPackage np, final Device.SendPackageStatusCallback callback, PublicKey key) { - if (channel == null) { + if (socket == null) { Log.e("KDE/sendPackage", "Not yet connected"); callback.sendFailure(new NotYetConnectedException()); return; @@ -190,12 +182,13 @@ public class LanLink extends BaseLink { //Send body of the network package try { - OutputStream writter = channel.getOutputStream(); - writter.write(np.serialize().getBytes(LanLinkProvider.UTF8)); + OutputStream writter = socket.getOutputStream(); + writter.write(np.serialize().getBytes(StringsHelper.UTF8)); writter.flush(); } catch (Exception e) { callback.sendFailure(e); e.printStackTrace(); + disconnect(); return; } @@ -265,8 +258,6 @@ public class LanLink extends BaseLink { public void injectNetworkPackage(NetworkPackage np) { - Log.e("receivedNetworkPackage",np.getType()); - if (np.getType().equals(NetworkPackage.PACKAGE_TYPE_ENCRYPTED)) { try { np = RsaHelper.decrypt(np, privateKey); @@ -274,7 +265,6 @@ public class LanLink extends BaseLink { e.printStackTrace(); Log.e("KDE/onPackageReceived","Exception decrypting the package"); } - } if (np.hasPayloadTransferInfo()) { @@ -282,7 +272,7 @@ public class LanLink extends BaseLink { Socket payloadSocket = null; try { // Use ssl if existing link is on ssl - if (channel instanceof SSLSocket) { + if (socket instanceof SSLSocket) { SSLContext sslContext = SslHelper.getSslContext(context, getDeviceId(), true); payloadSocket = sslContext.getSocketFactory().createSocket(); } else { @@ -290,7 +280,7 @@ public class LanLink extends BaseLink { } int tcpPort = np.getPayloadTransferInfo().getInt("port"); - InetSocketAddress address = (InetSocketAddress)channel.getRemoteSocketAddress(); + InetSocketAddress address = (InetSocketAddress) socket.getRemoteSocketAddress(); payloadSocket.connect(new InetSocketAddress(address.getAddress(), tcpPort)); np.setPayload(payloadSocket.getInputStream(), np.getPayloadSize()); } catch (Exception e) { @@ -305,7 +295,7 @@ public class LanLink extends BaseLink { } ServerSocket openTcpSocketOnFreePort(Context context, String deviceId) throws IOException { - if (channel instanceof SSLSocket) { + if (socket instanceof SSLSocket) { return openSecureServerSocket(context, deviceId); } else { return openUnsecureSocketOnFreePort(1739); diff --git a/src/org/kde/kdeconnect/Backends/LanBackend/LanLinkProvider.java b/src/org/kde/kdeconnect/Backends/LanBackend/LanLinkProvider.java index 675a4e55..4b2d5764 100644 --- a/src/org/kde/kdeconnect/Backends/LanBackend/LanLinkProvider.java +++ b/src/org/kde/kdeconnect/Backends/LanBackend/LanLinkProvider.java @@ -24,7 +24,6 @@ import android.content.Context; import android.content.SharedPreferences; import android.os.Build; import android.preference.PreferenceManager; -import android.support.v4.util.LongSparseArray; import android.util.Base64; import android.util.Log; @@ -34,6 +33,7 @@ import org.kde.kdeconnect.BackgroundService; import org.kde.kdeconnect.Device; import org.kde.kdeconnect.Helpers.DeviceHelper; import org.kde.kdeconnect.Helpers.SecurityHelpers.SslHelper; +import org.kde.kdeconnect.Helpers.StringsHelper; import org.kde.kdeconnect.NetworkPackage; import org.kde.kdeconnect.UserInterface.CustomDevicesActivity; @@ -47,7 +47,6 @@ import java.net.InetAddress; import java.net.ServerSocket; import java.net.Socket; import java.net.SocketException; -import java.nio.charset.Charset; import java.security.cert.Certificate; import java.util.ArrayList; import java.util.HashMap; @@ -67,15 +66,13 @@ public class LanLinkProvider extends BaseLinkProvider { public static final String KEY_CUSTOM_DEVLIST_PREFERENCE = "device_list_preference"; - static final Charset UTF8 = Charset.forName("UTF-8"); - private final static int oldPort = 1714; private final static int port = 1716; private final Context context; private final HashMap visibleComputers = new HashMap<>(); //Links by device id - private final LongSparseArray nioLinks = new LongSparseArray<>(); //Links by channel id + private final HashMap nioLinks = new HashMap<>(); //Links by socket private ServerSocket tcpServer; private DatagramSocket udpServer; @@ -86,23 +83,13 @@ public class LanLinkProvider extends BaseLinkProvider { // To prevent infinte loop between Android < IceCream because both device can only broadcast identity package but cannot connect via TCP private ArrayList reverseConnectionBlackList = new ArrayList<>(); - public void socketClosed(Socket socket) { - try { - final LanLink brokenLink = nioLinks.get(socket.hashCode()); - if (brokenLink != null) { - nioLinks.remove(socket.hashCode()); - //Log.i("KDE/LanLinkProvider", "nioLinks.size(): " + nioLinks.size() + " (-)"); - try { - brokenLink.disconnect(); - } catch (Exception e) { - e.printStackTrace(); - Log.e("KDE/LanLinkProvider", "Exception. Already disconnected?"); - } - //Log.i("KDE/LanLinkProvider", "Disconnected!"); + public void socketClosed(Socket socket, boolean linkHasAnotherSocket) { + final LanLink brokenLink = nioLinks.remove(socket); + if (brokenLink != null) { + + if (!linkHasAnotherSocket) { String deviceId = brokenLink.getDeviceId(); - if (visibleComputers.get(deviceId) == brokenLink) { - visibleComputers.remove(deviceId); - } + visibleComputers.remove(deviceId); new Thread(new Runnable() { @Override public void run() { @@ -115,17 +102,12 @@ public class LanLinkProvider extends BaseLinkProvider { } }).start(); - } - } catch (Exception e) { - e.printStackTrace(); - Log.e("KDE/LanLinkProvider", "channelInactive exception"); } } //They received my UDP broadcast and are connecting to me. The first thing they sned should be their identity. public void tcpPackageReceived(Socket socket) throws Exception { - //Log.e("KDE/LanLinkProvider", "Received a TCP packet from " + ctx.channel().remoteAddress() + ":" + message); NetworkPackage networkPackage; try { @@ -146,7 +128,7 @@ public class LanLinkProvider extends BaseLinkProvider { } else { - LanLink link = nioLinks.get(socket.hashCode()); + LanLink link = nioLinks.get(socket); if (link== null) { Log.e("KDE/LanLinkProvider","Expecting an identity package instead of " + networkPackage.getType()); } else { @@ -163,8 +145,7 @@ public class LanLinkProvider extends BaseLinkProvider { try { - String message = new String(packet.getData(), UTF8); - + String message = new String(packet.getData(), StringsHelper.UTF8); final NetworkPackage identityPackage = NetworkPackage.unserialize(message); final String deviceId = identityPackage.getString("deviceId"); if (!identityPackage.getType().equals(NetworkPackage.PACKAGE_TYPE_IDENTITY)) { @@ -173,12 +154,11 @@ public class LanLinkProvider extends BaseLinkProvider { } else { String myId = DeviceHelper.getDeviceId(context); if (deviceId.equals(myId)) { - //Log.i("KDE/LanLinkProvider", "Ignoring my own broadcast"); + //Ignore my own broadcast return; } } - Log.e("AAAAAAAAAAAAAAAAAAAAA","OOOOOOOOOOOOOOOOOOOOOOOOO"); if (identityPackage.getInt("protocolVersion") >= MIN_VERSION_WITH_NEW_PORT_SUPPORT && identityPackage.getInt("tcpPort") < port) { Log.w("KDE/LanLinkProvider", "Ignoring a udp broadcast from an old port because it comes from a device which knows about the new port."); return; @@ -189,7 +169,7 @@ public class LanLinkProvider extends BaseLinkProvider { int tcpPort = identityPackage.getInt("tcpPort", port); SocketFactory socketFactory = SocketFactory.getDefault(); - Socket socket = socketFactory.createSocket(packet.getAddress(), tcpPort); + Socket socket = socketFactory.createSocket(address, tcpPort); configureSocket(socket); OutputStream out = socket.getOutputStream(); @@ -226,14 +206,7 @@ public class LanLinkProvider extends BaseLinkProvider { } } - private void identityPackageReceived(final NetworkPackage identityPackage, final Socket channel, final LanLink.ConnectionStarted connectionStarted) { - - try { - Log.e("IDENTITITYYYYY", identityPackage.serialize()); - } catch (JSONException e) { - e.printStackTrace(); - } - + private void identityPackageReceived(final NetworkPackage identityPackage, final Socket socket, final LanLink.ConnectionStarted connectionStarted) { String myId = DeviceHelper.getDeviceId(context); final String deviceId = identityPackage.getString("deviceId"); @@ -255,7 +228,7 @@ public class LanLinkProvider extends BaseLinkProvider { Log.i("KDE/LanLinkProvider","Starting SSL handshake with " + identityPackage.getString("deviceName")); SSLSocketFactory sslsocketFactory = SslHelper.getSslContext(context, deviceId, isDeviceTrusted).getSocketFactory(); - final SSLSocket sslsocket = (SSLSocket)sslsocketFactory.createSocket(channel, channel.getInetAddress().getHostAddress(), channel.getPort(), true); + final SSLSocket sslsocket = (SSLSocket)sslsocketFactory.createSocket(socket, socket.getInetAddress().getHostAddress(), socket.getPort(), true); SslHelper.configureSslSocket(sslsocket, isDeviceTrusted, clientMode); configureSocket(sslsocket); sslsocket.addHandshakeCompletedListener(new HandshakeCompletedListener() { @@ -281,6 +254,7 @@ public class LanLinkProvider extends BaseLinkProvider { } } }); + //Handshake is blocking, so do it on another thread and free this thread to keep receiving new connection new Thread(new Runnable() { @Override public void run() { @@ -289,11 +263,10 @@ public class LanLinkProvider extends BaseLinkProvider { } catch (Exception e) { e.printStackTrace(); } - } }).start(); } else { - addLink(identityPackage, channel, connectionStarted); + addLink(identityPackage, socket, connectionStarted); } } catch (Exception e) { e.printStackTrace(); @@ -301,7 +274,7 @@ public class LanLinkProvider extends BaseLinkProvider { } - private void addLink(final NetworkPackage identityPackage, Socket channel, LanLink.ConnectionStarted connectionOrigin) throws IOException { + private void addLink(final NetworkPackage identityPackage, Socket socket, LanLink.ConnectionStarted connectionOrigin) throws IOException { try { Log.e("addLink", identityPackage.serialize()); @@ -314,49 +287,31 @@ public class LanLinkProvider extends BaseLinkProvider { if (currentLink != null) { //Update old link Log.i("KDE/LanLinkProvider", "Reusing same link for device " + deviceId); - final Socket oldChannel = currentLink.reset(channel, connectionOrigin, this); + final Socket oldSocket = currentLink.reset(socket, connectionOrigin, this); new Timer().schedule(new TimerTask() { @Override public void run() { - nioLinks.remove(oldChannel.hashCode()); - //Log.e("KDE/LanLinkProvider", "Forgetting about channel " + channel.hashCode()); + nioLinks.remove(oldSocket); + //Log.e("KDE/LanLinkProvider", "Forgetting about socket " + socket.hashCode()); } - }, 500); //Stop accepting messages from the old channel after 500ms - nioLinks.put(channel.hashCode(), currentLink); - //Log.e("KDE/LanLinkProvider", "Replacing channel. old: "+ oldChannel.hashCode() + " - new: "+ channel.hashCode()); + }, 500); //Stop accepting messages from the old socket after 500ms + nioLinks.put(socket, currentLink); + //Log.e("KDE/LanLinkProvider", "Replacing socket. old: "+ oldSocket.hashCode() + " - new: "+ socket.hashCode()); } else { - Log.e("addLink", "create link"); //Let's create the link - LanLink link = new LanLink(context, deviceId, this, channel, connectionOrigin); - nioLinks.put(channel.hashCode(), link); + LanLink link = new LanLink(context, deviceId, this, socket, connectionOrigin); + nioLinks.put(socket, link); visibleComputers.put(deviceId, link); connectionAccepted(identityPackage, link); } } - - public LanLinkProvider(Context context) { - this.context = context; - - - // Due to certificate request from SSL server to client, the certificate request message from device with latest android version to device with - // old android version causes a FATAL ALERT message stating that incorrect certificate request - // Server is disabled on these devices and using a reverse connection strategy. This works well for connection of these devices with kde - // and newer android versions. Although devices with android version less than ICS cannot connect to other devices who also have android version less - // than ICS because server is disabled on both - if (Build.VERSION.SDK_INT < Build.VERSION_CODES.ICE_CREAM_SANDWICH) { - Log.w("KDE/LanLinkProvider","Not starting a TCP server because it's not supported on Android < 14. Operating only as client."); - return; - } - } - private void setupUdpListener() { - try { udpServer = new DatagramSocket(port); udpServer.setReuseAddress(true); @@ -380,7 +335,7 @@ public class LanLinkProvider extends BaseLinkProvider { Log.e("LanLinkProvider", "UdpReceive exception"); } } - Log.e("UdpListener","Stopping UDP listener"); + Log.w("UdpListener","Stopping UDP listener"); } }).start(); } @@ -404,7 +359,7 @@ public class LanLinkProvider extends BaseLinkProvider { Log.e("LanLinkProvider", "TcpReceive exception"); } } - Log.e("TcpListener", "Stopping TCP listener"); + Log.w("TcpListener", "Stopping TCP listener"); } }).start(); @@ -434,7 +389,7 @@ public class LanLinkProvider extends BaseLinkProvider { socket = new DatagramSocket(); socket.setReuseAddress(true); socket.setBroadcast(true); - bytes = identity.serialize().getBytes(UTF8); + bytes = identity.serialize().getBytes(StringsHelper.UTF8); } catch (Exception e) { e.printStackTrace(); Log.e("KDE/LanLinkProvider","Failed to create DatagramSocket"); @@ -464,16 +419,26 @@ public class LanLinkProvider extends BaseLinkProvider { } @Override public void onStart() { - - Log.i("KDE/LanLinkProvider", "onStart"); - + //Log.i("KDE/LanLinkProvider", "onStart"); if (!running) { + running = true; + setupUdpListener(); - setupTcpListener(); + + // Due to certificate request from SSL server to client, the certificate request message from device with latest android version to device with + // old android version causes a FATAL ALERT message stating that incorrect certificate request + // Server is disabled on these devices and using a reverse connection strategy. This works well for connection of these devices with kde + // and newer android versions. Although devices with android version less than ICS cannot connect to other devices who also have android version less + // than ICS because server is disabled on both + if (Build.VERSION.SDK_INT < Build.VERSION_CODES.ICE_CREAM_SANDWICH) { + Log.w("KDE/LanLinkProvider","Not starting a TCP server because it's not supported on Android < 14. Operating only as client."); + } else { + setupTcpListener(); + } + broadcastUdpPackage(); } - } @Override @@ -483,21 +448,18 @@ public class LanLinkProvider extends BaseLinkProvider { @Override public void onStop() { - Log.i("KDE/LanLinkProvider", "onStop"); - + //Log.i("KDE/LanLinkProvider", "onStop"); running = false; try { tcpServer.close(); } catch (Exception e){ e.printStackTrace(); } - try { udpServer.close(); } catch (Exception e){ e.printStackTrace(); } - } @Override @@ -505,7 +467,4 @@ public class LanLinkProvider extends BaseLinkProvider { return "LanLinkProvider"; } - - - } diff --git a/src/org/kde/kdeconnect/Helpers/NotificationsHelper.java b/src/org/kde/kdeconnect/Helpers/NotificationsHelper.java deleted file mode 100644 index 6be50d8e..00000000 --- a/src/org/kde/kdeconnect/Helpers/NotificationsHelper.java +++ /dev/null @@ -1,12 +0,0 @@ -package org.kde.kdeconnect.Helpers; - -import java.util.concurrent.atomic.AtomicInteger; - -public class NotificationsHelper { - - private final static AtomicInteger c = new AtomicInteger((int)System.currentTimeMillis()); - public static int getUniqueId() { - return c.incrementAndGet(); - } - -} diff --git a/src/org/kde/kdeconnect/Helpers/StringsHelper.java b/src/org/kde/kdeconnect/Helpers/StringsHelper.java new file mode 100644 index 00000000..5e567855 --- /dev/null +++ b/src/org/kde/kdeconnect/Helpers/StringsHelper.java @@ -0,0 +1,9 @@ +package org.kde.kdeconnect.Helpers; + +import java.nio.charset.Charset; + +public class StringsHelper { + + public static final Charset UTF8 = Charset.forName("UTF-8"); + +}