From bfd9b2eb9675f6d76662eb074beea4afed09f8ee Mon Sep 17 00:00:00 2001 From: Sharad Binjola <31142146+sharadb-amazon@users.noreply.github.com> Date: Tue, 7 Mar 2023 10:16:43 -0800 Subject: [PATCH] Android/tv-casting-app: Adding defensive checks in Discovery/record parsing (#25464) * Checking if multicastLock isHeld before trying to release it, avoiding stopServiceDiscovery call if discovery failed to start * Android tv-casting-app: Adding defensive checks in DiscoveredNodeData constructor (#108) * Android tv-casting-app: Adding defensive null checks in DiscoveredNodeData constructor * Adding more defensive checks / exception handling --- .../com/chip/casting/DiscoveredNodeData.java | 61 ++++++++++++++----- .../chip/casting/NsdDiscoveryListener.java | 2 - .../jni/com/chip/casting/TvCastingApp.java | 4 +- 3 files changed, 49 insertions(+), 18 deletions(-) diff --git a/examples/tv-casting-app/android/App/app/src/main/jni/com/chip/casting/DiscoveredNodeData.java b/examples/tv-casting-app/android/App/app/src/main/jni/com/chip/casting/DiscoveredNodeData.java index e3bc55388658ed..a9bca3aaf46698 100644 --- a/examples/tv-casting-app/android/App/app/src/main/jni/com/chip/casting/DiscoveredNodeData.java +++ b/examples/tv-casting-app/android/App/app/src/main/jni/com/chip/casting/DiscoveredNodeData.java @@ -18,6 +18,7 @@ package com.chip.casting; import android.net.nsd.NsdServiceInfo; +import android.util.Log; import java.net.InetAddress; import java.nio.charset.StandardCharsets; import java.util.Arrays; @@ -25,6 +26,8 @@ import java.util.Map; public class DiscoveredNodeData { + private static final String TAG = DiscoveredNodeData.class.getSimpleName(); + private static final int MAX_IP_ADDRESSES = 5; private static final int MAX_ROTATING_ID_LEN = 50; private static final String KEY_DEVICE_NAME = "DN"; @@ -51,26 +54,54 @@ public class DiscoveredNodeData { public DiscoveredNodeData(NsdServiceInfo serviceInfo) { Map attributes = serviceInfo.getAttributes(); - this.deviceName = new String(attributes.get(KEY_DEVICE_NAME), StandardCharsets.UTF_8); - if (serviceInfo.getHost() != null) { - this.hostName = serviceInfo.getHost().getHostName(); - } - this.deviceType = - Long.parseLong(new String(attributes.get(KEY_DEVICE_TYPE), StandardCharsets.UTF_8)); - - String vp = new String(attributes.get(KEY_VENDOR_PRODUCT), StandardCharsets.UTF_8); - if (vp != null) { - String[] vpArray = vp.split("\\+"); - if (vpArray.length > 0) { - this.vendorId = Long.parseLong(vpArray[0]); - if (vpArray.length == 2) { - this.productId = Long.parseLong(vpArray[1]); + + if (attributes != null) { + if (attributes.get(KEY_DEVICE_NAME) != null) { + this.deviceName = new String(attributes.get(KEY_DEVICE_NAME), StandardCharsets.UTF_8); + } else { + Log.e(TAG, "No device name (DN) found in DiscovoredNodeData"); + } + + if (attributes.get(KEY_DEVICE_TYPE) != null) { + try { + this.deviceType = + Long.parseLong(new String(attributes.get(KEY_DEVICE_TYPE), StandardCharsets.UTF_8)); + } catch (NumberFormatException e) { + Log.e(TAG, "Could not parse TXT record for DT: " + e.getMessage()); } + } else { + Log.e(TAG, "TXT Record for DT was null"); } + + if (attributes.get(KEY_VENDOR_PRODUCT) != null) { + String vp = new String(attributes.get(KEY_VENDOR_PRODUCT), StandardCharsets.UTF_8); + if (vp != null) { + String[] vpArray = vp.split("\\+"); + try { + if (vpArray.length > 0) { + this.vendorId = Long.parseLong(vpArray[0]); + if (vpArray.length == 2) { + this.productId = Long.parseLong(vpArray[1]); + } + } + } catch (NumberFormatException e) { + Log.e(TAG, "Could not parse TXT record for VP: " + e.getMessage()); + } + } + } else { + Log.e(TAG, "TXT Record for VP was null"); + } + } else { + Log.e(TAG, "NsdServiceInfo.attributes was null"); } + if (serviceInfo.getHost() != null) { + this.hostName = serviceInfo.getHost().getHostName(); + this.ipAddresses = Arrays.asList(serviceInfo.getHost()); + } else { + Log.e(TAG, "Host name was null"); + } this.port = serviceInfo.getPort(); - this.ipAddresses = Arrays.asList(serviceInfo.getHost()); this.numIPs = 1; } diff --git a/examples/tv-casting-app/android/App/app/src/main/jni/com/chip/casting/NsdDiscoveryListener.java b/examples/tv-casting-app/android/App/app/src/main/jni/com/chip/casting/NsdDiscoveryListener.java index 5fda64cbbba8ef..ef5d66a6b5b566 100644 --- a/examples/tv-casting-app/android/App/app/src/main/jni/com/chip/casting/NsdDiscoveryListener.java +++ b/examples/tv-casting-app/android/App/app/src/main/jni/com/chip/casting/NsdDiscoveryListener.java @@ -111,7 +111,6 @@ public void onStartDiscoveryFailed(String serviceType, int errorCode) { failureCallback.handle( new MatterError( 3, "NsdDiscoveryListener Discovery failed to start: Nsd Error code:" + errorCode)); - nsdManager.stopServiceDiscovery(this); } @Override @@ -123,6 +122,5 @@ public void onStopDiscoveryFailed(String serviceType, int errorCode) { failureCallback.handle( new MatterError( 3, "NsdDiscoveryListener Discovery failed to stop: Nsd Error code:" + errorCode)); - nsdManager.stopServiceDiscovery(this); } } diff --git a/examples/tv-casting-app/android/App/app/src/main/jni/com/chip/casting/TvCastingApp.java b/examples/tv-casting-app/android/App/app/src/main/jni/com/chip/casting/TvCastingApp.java index 43e7860e6b0527..009678ff163ecf 100644 --- a/examples/tv-casting-app/android/App/app/src/main/jni/com/chip/casting/TvCastingApp.java +++ b/examples/tv-casting-app/android/App/app/src/main/jni/com/chip/casting/TvCastingApp.java @@ -137,7 +137,9 @@ public void discoverVideoPlayerCommissioners( public void run() { Log.d(TAG, "TvCastingApp stopping Video Player commissioner discovery"); nsdManager.stopServiceDiscovery(nsdDiscoveryListener); - multicastLock.release(); + if (multicastLock.isHeld()) { + multicastLock.release(); + } } }, discoveryDurationSeconds,