Merge pull request #682 from smartdevicelink/bugfix/issue_660

Bugfix/issue 660 UAI Race condition logic

2 files changed, 227 insertions, 1 deletions

diff --git a/sdl_android/src/androidTest/java/com/smartdevicelink/transport/SdlRouterServiceTests.java b/sdl_android/src/androidTest/java/com/smartdevicelink/transport/SdlRouterServiceTests.java
index 40fe0e77f..a82c7a131 100644
--- a/sdl_android/src/androidTest/java/com/smartdevicelink/transport/SdlRouterServiceTests.java
+++ b/sdl_android/src/androidTest/java/com/smartdevicelink/transport/SdlRouterServiceTests.java
@@ -7,18 +7,34 @@ import android.os.Looper;
 import android.os.Message;
 import android.test.AndroidTestCase;
 import android.util.Log;
+import android.util.SparseArray;
+import android.util.SparseIntArray;
 
+import com.smartdevicelink.marshal.JsonRPCMarshaller;
+import com.smartdevicelink.protocol.BinaryFrameHeader;
+import com.smartdevicelink.protocol.ProtocolMessage;
 import com.smartdevicelink.protocol.SdlPacket;
+import com.smartdevicelink.protocol.SdlPacketFactory;
+import com.smartdevicelink.protocol.enums.FunctionID;
+import com.smartdevicelink.protocol.enums.MessageType;
+import com.smartdevicelink.protocol.enums.SessionType;
+import com.smartdevicelink.proxy.rpc.UnregisterAppInterface;
 
 import junit.framework.Assert;
 
 import java.lang.ref.WeakReference;
 import java.lang.reflect.Field;
 import java.lang.reflect.Method;
+import java.util.HashMap;
 
 public class SdlRouterServiceTests extends AndroidTestCase {
 
     public static final String TAG = "SdlRouterServiceTests";
+    private final int SAMPLE_RPC_CORRELATION_ID = 630;
+    int version = 1;
+    int sessionId = 1;
+    ProtocolMessage pm = null;
+    BinaryFrameHeader binFrameHeader = null;
 
     @Override
     protected void setUp() throws Exception {
@@ -129,4 +145,187 @@ public class SdlRouterServiceTests extends AndroidTestCase {
             }
         }
     }
+
+	/**
+	 * Test sending UAI to an app whose session id is the same as a removed app
+	 * but is indeed a different app
+	 *
+	 * @see SdlRouterService#sendPacketToRegisteredApp(SdlPacket)
+	 */
+	public void testRegisterAppExistingSessionIDDifferntApp() {
+		if (Looper.myLooper() == null) {
+			Looper.prepare();
+		}
+		Method method;
+		try {
+			// create instance of router service
+			SdlRouterService sdlRouterService = new SdlRouterService();
+
+			// We need a registered app for this to work
+			Message message = Message.obtain();
+			SdlRouterService.RegisteredApp app1 = sdlRouterService.new RegisteredApp("12345",message.replyTo);
+			SdlRouterService.RegisteredApp app2 = sdlRouterService.new RegisteredApp("12344",message.replyTo);
+			HashMap<String,SdlRouterService.RegisteredApp> registeredApps = new HashMap<>();
+			registeredApps.put(app1.getAppId(),app1);
+			registeredApps.put(app2.getAppId(),app2);
+
+			// set registered apps array
+			Field raf = sdlRouterService.getClass().getDeclaredField("registeredApps");
+			raf.setAccessible(true);
+			raf.set(sdlRouterService, registeredApps);
+
+			// need a session map too
+			SparseArray<String> sessionMap = new SparseArray<String>();
+			sessionMap.put(1, "12345");
+			Field sessionMapField = sdlRouterService.getClass().getDeclaredField("sessionMap");
+			sessionMapField.setAccessible(true);
+			sessionMapField.set(sdlRouterService, sessionMap);
+
+			// set cleaned session map
+			SparseIntArray testCleanedMap = new SparseIntArray();
+			testCleanedMap.put(1, 12345);
+			Field f = sdlRouterService.getClass().getDeclaredField("cleanedSessionMap");
+			f.setAccessible(true);
+			f.set(sdlRouterService, testCleanedMap);
+
+			// set session hash id map
+			SparseIntArray testHashIdMap = new SparseIntArray();
+			testHashIdMap.put(1, 12344);
+			Field f2 = sdlRouterService.getClass().getDeclaredField("sessionHashIdMap");
+			f2.setAccessible(true);
+			f2.set(sdlRouterService, testHashIdMap);
+
+			// make sure maps are set and NOT the same
+			Assert.assertNotNull(raf.get(sdlRouterService));
+			Assert.assertNotNull(sessionMapField.get(sdlRouterService));
+			Assert.assertNotNull(f.get(sdlRouterService));
+			Assert.assertNotNull(f2.get(sdlRouterService));
+
+			// make da RPC
+			UnregisterAppInterface request = new UnregisterAppInterface();
+			request.setCorrelationID(SAMPLE_RPC_CORRELATION_ID);
+
+			// build protocol message
+			byte[] msgBytes = JsonRPCMarshaller.marshall(request, (byte) version);
+			pm = new ProtocolMessage();
+			pm.setData(msgBytes);
+			pm.setSessionID((byte) sessionId);
+			pm.setMessageType(MessageType.RPC);
+			pm.setSessionType(SessionType.RPC);
+			pm.setFunctionID(FunctionID.getFunctionId(request.getFunctionName()));
+			pm.setCorrID(request.getCorrelationID());
+
+			if (request.getBulkData() != null) {
+				pm.setBulkData(request.getBulkData());
+			}
+
+			// binary frame header
+			byte[] data = new byte[12 + pm.getJsonSize()];
+			binFrameHeader = SdlPacketFactory.createBinaryFrameHeader(pm.getRPCType(), pm.getFunctionID(), pm.getCorrID(), pm.getJsonSize());
+			System.arraycopy(binFrameHeader.assembleHeaderBytes(), 0, data, 0, 12);
+			System.arraycopy(pm.getData(), 0, data, 12, pm.getJsonSize());
+
+			// create packet and invoke sendPacketToRegisteredApp
+			SdlPacket packet = new SdlPacket(4, false, SdlPacket.FRAME_TYPE_SINGLE, SdlPacket.SERVICE_TYPE_RPC, 0, sessionId, data.length, 123, data);
+			method = sdlRouterService.getClass().getDeclaredMethod("sendPacketToRegisteredApp", SdlPacket.class);
+			Boolean success = (Boolean) method.invoke(sdlRouterService, packet);
+
+			// we do not want the UAI packet to be sent. make sure it is dropped
+			Assert.assertFalse(success);
+
+		} catch (Exception e) {
+			Assert.fail("Exception in sendPacketToRegisteredApp, " + e);
+		}
+	}
+	/**
+	 * Test sending UAI to an app whose session id is the same as a removed app
+	 * but is indeed the SAME app
+	 *
+	 * @see SdlRouterService#sendPacketToRegisteredApp(SdlPacket)
+	 */
+	public void testRegisterAppExistingSessionIDSameApp() {
+		if (Looper.myLooper() == null) {
+			Looper.prepare();
+		}
+		Method method;
+		try {
+			// create instance of router service
+			SdlRouterService sdlRouterService = new SdlRouterService();
+
+			// We need a registered app for this to work
+			Message message = Message.obtain();
+			SdlRouterService.RegisteredApp app1 = sdlRouterService.new RegisteredApp("12345",message.replyTo);
+			SdlRouterService.RegisteredApp app2 = sdlRouterService.new RegisteredApp("12344",message.replyTo);
+			HashMap<String,SdlRouterService.RegisteredApp> registeredApps = new HashMap<>();
+			registeredApps.put(app1.getAppId(),app1);
+			registeredApps.put(app2.getAppId(),app2);
+
+			// set registered apps array
+			Field raf = sdlRouterService.getClass().getDeclaredField("registeredApps");
+			raf.setAccessible(true);
+			raf.set(sdlRouterService, registeredApps);
+
+			// need a session map too
+			SparseArray<String> sessionMap = new SparseArray<String>();
+			sessionMap.put(1, "12345");
+			Field sessionMapField = sdlRouterService.getClass().getDeclaredField("sessionMap");
+			sessionMapField.setAccessible(true);
+			sessionMapField.set(sdlRouterService, sessionMap);
+
+			// set cleaned session map
+			SparseIntArray testCleanedMap = new SparseIntArray();
+			testCleanedMap.put(1, 12345);
+			Field f = sdlRouterService.getClass().getDeclaredField("cleanedSessionMap");
+			f.setAccessible(true);
+			f.set(sdlRouterService, testCleanedMap);
+
+			// set session hash id map
+			SparseIntArray testHashIdMap = new SparseIntArray();
+			testHashIdMap.put(1, 12345);
+			Field f2 = sdlRouterService.getClass().getDeclaredField("sessionHashIdMap");
+			f2.setAccessible(true);
+			f2.set(sdlRouterService, testHashIdMap);
+
+			// make sure maps are set and NOT the same
+			Assert.assertNotNull(raf.get(sdlRouterService));
+			Assert.assertNotNull(sessionMapField.get(sdlRouterService));
+			Assert.assertNotNull(f.get(sdlRouterService));
+			Assert.assertNotNull(f2.get(sdlRouterService));
+
+			// make da RPC
+			UnregisterAppInterface request = new UnregisterAppInterface();
+			request.setCorrelationID(SAMPLE_RPC_CORRELATION_ID);
+
+			// build protocol message
+			byte[] msgBytes = JsonRPCMarshaller.marshall(request, (byte) version);
+			pm = new ProtocolMessage();
+			pm.setData(msgBytes);
+			pm.setSessionID((byte) sessionId);
+			pm.setMessageType(MessageType.RPC);
+			pm.setSessionType(SessionType.RPC);
+			pm.setFunctionID(FunctionID.getFunctionId(request.getFunctionName()));
+			pm.setCorrID(request.getCorrelationID());
+
+			if (request.getBulkData() != null) {
+				pm.setBulkData(request.getBulkData());
+			}
+
+			// binary frame header
+			byte[] data = new byte[12 + pm.getJsonSize()];
+			binFrameHeader = SdlPacketFactory.createBinaryFrameHeader(pm.getRPCType(), pm.getFunctionID(), pm.getCorrID(), pm.getJsonSize());
+			System.arraycopy(binFrameHeader.assembleHeaderBytes(), 0, data, 0, 12);
+			System.arraycopy(pm.getData(), 0, data, 12, pm.getJsonSize());
+
+			// create packet and invoke sendPacketToRegisteredApp
+			SdlPacket packet = new SdlPacket(4, false, SdlPacket.FRAME_TYPE_SINGLE, SdlPacket.SERVICE_TYPE_RPC, 0, sessionId, data.length, 123, data);
+			method = sdlRouterService.getClass().getDeclaredMethod("sendPacketToRegisteredApp", SdlPacket.class);
+			Boolean success = (Boolean) method.invoke(sdlRouterService, packet);
+
+			// Since it is the same app, allow the packet to be sent
+			Assert.assertTrue(success);
+
+		} catch (Exception e) {
+			Assert.fail("Exception in sendPacketToRegisteredApp, " + e);
+		}
+	}
 }
diff --git a/sdl_android/src/main/java/com/smartdevicelink/transport/SdlRouterService.java b/sdl_android/src/main/java/com/smartdevicelink/transport/SdlRouterService.java
index 46b392d13..868500f36 100644
--- a/sdl_android/src/main/java/com/smartdevicelink/transport/SdlRouterService.java
+++ b/sdl_android/src/main/java/com/smartdevicelink/transport/SdlRouterService.java
@@ -138,6 +138,7 @@ public class SdlRouterService extends Service{
 	public static HashMap<String,RegisteredApp> registeredApps;
 	private SparseArray<String> sessionMap;
 	private SparseIntArray sessionHashIdMap;
+	private SparseIntArray cleanedSessionMap;
 	private final Object SESSION_LOCK = new Object(), REGISTERED_APPS_LOCK = new Object(), PING_COUNT_LOCK = new Object();
 	
 	private static Messenger altTransportService = null;
@@ -803,6 +804,7 @@ public class SdlRouterService extends Service{
 		synchronized(SESSION_LOCK){
 			this.sessionMap = new SparseArray<String>();
 			this.sessionHashIdMap = new SparseIntArray();
+			this.cleanedSessionMap = new SparseIntArray();
 		}
 
 		packetExecutor =  Executors.newSingleThreadExecutor();
@@ -1425,7 +1427,8 @@ public class SdlRouterService extends Service{
 	    			synchronized(REGISTERED_APPS_LOCK){
 	    				 app = registeredApps.get(appid);
 	    			}
-	    			if(app==null){Log.e(TAG, "No app found for app id " + appid + " Removing session mapping and sending unregisterAI to head unit.");
+	    			if(app==null){
+	    				Log.e(TAG, "No app found for app id " + appid + " Removing session mapping and sending unregisterAI to head unit.");
 	    				//We have no app to match the app id tied to this session
 	    				removeSessionFromMap(session);
 	    				byte[] uai = createForceUnregisterApp((byte)session, (byte)packet.getVersion());
@@ -1451,6 +1454,29 @@ public class SdlRouterService extends Service{
 	    				}
 	    			}
 
+				// check and prevent a UAI from being passed to an app that is using a recycled session id
+				if (cleanedSessionMap != null && cleanedSessionMap.size() > 0 ) {
+					if(packet.getFrameType() == FrameType.Single && packet.getServiceType() == SdlPacket.SERVICE_TYPE_RPC) {
+						BinaryFrameHeader binFrameHeader = BinaryFrameHeader.parseBinaryHeader(packet.getPayload());
+						if (binFrameHeader != null && FunctionID.UNREGISTER_APP_INTERFACE.getId() == binFrameHeader.getFunctionID()) {
+							Log.d(TAG, "Received an unregister app interface. Checking session hash before sending");
+							// make sure that we don't try to unregister a recently added app that might have a
+							// session ID of a removed app whose UAI was delayed
+							int hashOfRemoved = this.cleanedSessionMap.get(session, -1);
+							int currentHash = this.sessionHashIdMap.get(session, -1);
+							if (hashOfRemoved != -1) {
+								// Current session contains key that was held before
+								if (hashOfRemoved != currentHash) {
+									// App assigned same session id but is a different app. Keep this from being killed
+									Log.d(TAG, "same session id for different apps found, dropping packet");
+									this.cleanedSessionMap.delete(session);
+									return false;
+								}
+							}
+						}
+					}
+				}
+
 	    			int packetSize = (int) (packet.getDataSize() + SdlPacket.HEADER_SIZE);
 	    			//Log.i(TAG, "Checking packet size: " + packetSize);
 	    			Message message = Message.obtain();
@@ -1534,6 +1560,7 @@ public class SdlRouterService extends Service{
 				if(this.sessionHashIdMap.indexOfKey(session)>=0){
 					hashId = this.sessionHashIdMap.get(session); 
 					this.sessionHashIdMap.delete(session);
+					this.cleanedSessionMap.put(session,hashId);
 				}
 			}
 			byte[] stopService = (SdlPacketFactory.createEndSession(SessionType.RPC, (byte)session, 0, (byte)version,BitConverter.intToByteArray(hashId))).constructPacket();