From be11b3283325fe23788bf1d597911dfbe21d6296 Mon Sep 17 00:00:00 2001
From: twyen <twyen@google.com>
Date: Fri, 18 May 2018 11:09:32 -0700
Subject: Handle invalid PhoneAccountHandle for CarrierIdentifier

Previously the phoneAccountHandle is validated before creating the CarrierIdentifier, but it seems like there's a chance that it would be invalidated in the gap before the identifier is created.

TEST=TAP
Bug: 73890027
Test: TAP
PiperOrigin-RevId: 197172168
Change-Id: I6f4f77819ed6ce9f3449f7e47a3538af1098d826
---
 .../android/voicemail/impl/CarrierIdentifier.java  | 17 +++++++----
 .../voicemail/impl/OmtpVvmCarrierConfigHelper.java | 35 +++++++++++-----------
 2 files changed, 29 insertions(+), 23 deletions(-)

(limited to 'java')

diff --git a/java/com/android/voicemail/impl/CarrierIdentifier.java b/java/com/android/voicemail/impl/CarrierIdentifier.java
index 82b6a2440..76576e7ad 100644
--- a/java/com/android/voicemail/impl/CarrierIdentifier.java
+++ b/java/com/android/voicemail/impl/CarrierIdentifier.java
@@ -19,14 +19,16 @@ package com.android.voicemail.impl;
 import android.annotation.TargetApi;
 import android.content.Context;
 import android.os.Build.VERSION_CODES;
+import android.support.annotation.Nullable;
 import android.telecom.PhoneAccountHandle;
 import android.telephony.TelephonyManager;
 import com.google.auto.value.AutoValue;
+import java.util.Optional;
 
 /** Identifies a carrier. */
 @AutoValue
 @TargetApi(VERSION_CODES.O)
-@SuppressWarnings("missingpermission")
+@SuppressWarnings({"missingpermission", "AndroidApiChecker"})
 public abstract class CarrierIdentifier {
 
   public abstract String mccMnc();
@@ -52,20 +54,25 @@ public abstract class CarrierIdentifier {
     return new AutoValue_CarrierIdentifier.Builder().setGid1("");
   }
 
-  public static CarrierIdentifier forHandle(
-      Context context, PhoneAccountHandle phoneAccountHandle) {
+  /** Create a identifier for a {@link PhoneAccountHandle}. Absent if the handle is not valid. */
+  public static Optional<CarrierIdentifier> forHandle(
+      Context context, @Nullable PhoneAccountHandle phoneAccountHandle) {
+    if (phoneAccountHandle == null) {
+      return Optional.empty();
+    }
     TelephonyManager telephonyManager =
         context
             .getSystemService(TelephonyManager.class)
             .createForPhoneAccountHandle(phoneAccountHandle);
     if (telephonyManager == null) {
-      throw new IllegalArgumentException("Invalid PhoneAccountHandle");
+      return Optional.empty();
     }
     String gid1 = telephonyManager.getGroupIdLevel1();
     if (gid1 == null) {
       gid1 = "";
     }
 
-    return builder().setMccMnc(telephonyManager.getSimOperator()).setGid1(gid1).build();
+    return Optional.of(
+        builder().setMccMnc(telephonyManager.getSimOperator()).setGid1(gid1).build());
   }
 }
diff --git a/java/com/android/voicemail/impl/OmtpVvmCarrierConfigHelper.java b/java/com/android/voicemail/impl/OmtpVvmCarrierConfigHelper.java
index b56f792cb..4c4df27a7 100644
--- a/java/com/android/voicemail/impl/OmtpVvmCarrierConfigHelper.java
+++ b/java/com/android/voicemail/impl/OmtpVvmCarrierConfigHelper.java
@@ -39,6 +39,7 @@ import com.android.voicemail.impl.protocol.VisualVoicemailProtocolFactory;
 import com.android.voicemail.impl.sms.StatusMessage;
 import com.android.voicemail.impl.sync.VvmAccountManager;
 import java.util.Collections;
+import java.util.Optional;
 import java.util.Set;
 
 /**
@@ -55,7 +56,7 @@ import java.util.Set;
  * <p>TODO(twyen): refactor this to an interface.
  */
 @TargetApi(VERSION_CODES.O)
-@SuppressWarnings("missingpermission")
+@SuppressWarnings({"missingpermission", "AndroidApiChecker"})
 public class OmtpVvmCarrierConfigHelper {
 
   private static final String TAG = "OmtpVvmCarrierCfgHlpr";
@@ -105,25 +106,25 @@ public class OmtpVvmCarrierConfigHelper {
   public OmtpVvmCarrierConfigHelper(Context context, @Nullable PhoneAccountHandle handle) {
     this.context = context;
     phoneAccountHandle = handle;
-    TelephonyManager telephonyManager =
-        context
-            .getSystemService(TelephonyManager.class)
-            .createForPhoneAccountHandle(phoneAccountHandle);
-    if (telephonyManager == null) {
-      VvmLog.e(TAG, "PhoneAccountHandle is invalid");
-      carrierConfig = null;
-      telephonyConfig = null;
-      overrideConfig = null;
-      vvmType = null;
-      protocol = null;
-      return;
-    }
-
     if (overrideConfigForTest != null) {
       overrideConfig = overrideConfigForTest;
       carrierConfig = new PersistableBundle();
       telephonyConfig = new PersistableBundle();
     } else {
+      Optional<CarrierIdentifier> carrierIdentifier = CarrierIdentifier.forHandle(context, handle);
+      TelephonyManager telephonyManager =
+          context
+              .getSystemService(TelephonyManager.class)
+              .createForPhoneAccountHandle(phoneAccountHandle);
+      if (telephonyManager == null || !carrierIdentifier.isPresent()) {
+        VvmLog.e(TAG, "PhoneAccountHandle is invalid");
+        carrierConfig = null;
+        telephonyConfig = null;
+        overrideConfig = null;
+        vvmType = null;
+        protocol = null;
+        return;
+      }
       if (ConfigOverrideFragment.isOverridden(context)) {
         overrideConfig = ConfigOverrideFragment.getConfig(context);
         VvmLog.w(TAG, "Config override is activated: " + overrideConfig);
@@ -132,9 +133,7 @@ public class OmtpVvmCarrierConfigHelper {
       }
 
       carrierConfig = getCarrierConfig(telephonyManager);
-      telephonyConfig =
-          new DialerVvmConfigManager(context)
-              .getConfig(CarrierIdentifier.forHandle(context, phoneAccountHandle));
+      telephonyConfig = new DialerVvmConfigManager(context).getConfig(carrierIdentifier.get());
     }
 
     vvmType = getVvmType();
-- 
cgit v1.2.3