From 3ffbf86e1e4701ea316146c1686f06193166c2a6 Mon Sep 17 00:00:00 2001
From: Nathan Harold <nharold@google.com>
Date: Mon, 26 Feb 2018 19:14:18 -0800
Subject: [PATCH] Fix Range-Checking in CellSignalStrengthCdma

The CellSignalStrengthCdma class previously allowed
the values in the class to be kept as negative ints
but expected them to be parceled as positive ints.
This led to a confusing mess that is best unwound
by calling the actual constructor for the class and
letting the parcel values be an implementation detail.
This CL removes all of the parcel-time coersion and
instead expects that the class be constructed using
a constructor rather than by manually parceling and
then using the class to un-parcel. In addition, the
range checking for inputs is now done only once, and
values are no longer mutated in the parcel/unparcel
process.

Bug: 32364031
Test: runtest frameworks-telephony
Change-Id: I59ce8c9df1bd99547f3de941a30d6c3cea8f2b8f
---
 .../telephony/CellSignalStrengthCdma.java     | 49 ++++++++++++-------
 1 file changed, 32 insertions(+), 17 deletions(-)

diff --git a/telephony/java/android/telephony/CellSignalStrengthCdma.java b/telephony/java/android/telephony/CellSignalStrengthCdma.java
index ece1ee3781706..183f96ddab0b4 100644
--- a/telephony/java/android/telephony/CellSignalStrengthCdma.java
+++ b/telephony/java/android/telephony/CellSignalStrengthCdma.java
@@ -41,14 +41,36 @@ public final class CellSignalStrengthCdma extends CellSignalStrength implements
         setDefaultValues();
     }
 
-    /** @hide */
+    /**
+     * SignalStrength constructor for input from the HAL.
+     *
+     * Note that values received from the HAL require coersion to be compatible here. All values
+     * reported through IRadio are the negative of the actual values (which results in a positive
+     * input to this method.
+     *
+     * <p>Note that this HAL is inconsistent with UMTS-based radio techs as the value indicating
+     * that a field is unreported is negative, rather than a large(r) positive number.
+     * <p>Also note that to keep the public-facing methods of this class consistent with others,
+     * unreported values are coerced to Integer.MAX_VALUE rather than left as -1, which is
+     * a departure from SignalStrength, which is stuck with the values it currently reports.
+     *
+     * @param cdmaDbm negative of the CDMA signal strength value or -1 if invalid.
+     * @param cdmaEcio negative of the CDMA pilot/noise ratio or -1 if invalid.
+     * @param evdoDbm negative of the EvDO signal strength value or -1 if invalid.
+     * @param evdoEcio negative of the EvDO pilot/noise ratio or -1 if invalid.
+     * @param evdoSnr an SNR value 0..8 or -1 if invalid.
+     * @hide
+     */
     public CellSignalStrengthCdma(int cdmaDbm, int cdmaEcio, int evdoDbm, int evdoEcio,
             int evdoSnr) {
-        mCdmaDbm = cdmaDbm;
-        mCdmaEcio = cdmaEcio;
-        mEvdoDbm = evdoDbm;
-        mEvdoEcio = evdoEcio;
-        mEvdoSnr = evdoSnr;
+        // The values here were lifted from SignalStrength.validateInput()
+        // FIXME: Combine all checking and setting logic between this and SignalStrength.
+        mCdmaDbm = ((cdmaDbm > 0) && (cdmaDbm < 120))  ? -cdmaDbm : Integer.MAX_VALUE;
+        mCdmaEcio = ((cdmaEcio > 0) && (cdmaEcio < 160)) ? -cdmaEcio : Integer.MAX_VALUE;
+
+        mEvdoDbm = ((evdoDbm > 0) && (evdoDbm < 120)) ? -evdoDbm : Integer.MAX_VALUE;
+        mEvdoEcio = ((evdoEcio > 0) && (evdoEcio < 160)) ? -evdoEcio : Integer.MAX_VALUE;
+        mEvdoSnr = ((evdoSnr > 0) && (evdoSnr <= 8)) ? evdoSnr : Integer.MAX_VALUE;
     }
 
     /** @hide */
@@ -303,13 +325,10 @@ public final class CellSignalStrengthCdma extends CellSignalStrength implements
     @Override
     public void writeToParcel(Parcel dest, int flags) {
         if (DBG) log("writeToParcel(Parcel, int): " + toString());
-        // Need to multiply CdmaDbm, CdmaEcio, EvdoDbm and EvdoEcio by -1
-        // to ensure consistency when reading values written here
-        // unless the value is invalid
-        dest.writeInt(mCdmaDbm * (mCdmaDbm != Integer.MAX_VALUE ? -1 : 1));
-        dest.writeInt(mCdmaEcio * (mCdmaEcio != Integer.MAX_VALUE ? -1 : 1));
-        dest.writeInt(mEvdoDbm * (mEvdoDbm != Integer.MAX_VALUE ? -1 : 1));
-        dest.writeInt(mEvdoEcio * (mEvdoEcio != Integer.MAX_VALUE ? -1 : 1));
+        dest.writeInt(mCdmaDbm);
+        dest.writeInt(mCdmaEcio);
+        dest.writeInt(mEvdoDbm);
+        dest.writeInt(mEvdoEcio);
         dest.writeInt(mEvdoSnr);
     }
 
@@ -322,13 +341,9 @@ public final class CellSignalStrengthCdma extends CellSignalStrength implements
         // the parcel as positive values.
         // Need to convert into negative values unless the value is invalid
         mCdmaDbm = in.readInt();
-        if (mCdmaDbm != Integer.MAX_VALUE) mCdmaDbm *= -1;
         mCdmaEcio = in.readInt();
-        if (mCdmaEcio != Integer.MAX_VALUE) mCdmaEcio *= -1;
         mEvdoDbm = in.readInt();
-        if (mEvdoDbm != Integer.MAX_VALUE) mEvdoDbm *= -1;
         mEvdoEcio = in.readInt();
-        if (mEvdoEcio != Integer.MAX_VALUE) mEvdoEcio *= -1;
         mEvdoSnr = in.readInt();
         if (DBG) log("CellSignalStrengthCdma(Parcel): " + toString());
     }