Skip to content
This repository has been archived by the owner on Nov 8, 2023. It is now read-only.

Commit

Permalink
Fix a bunch of STOPSHIP comments, mostly about PII logging.
Browse files Browse the repository at this point in the history
TESTED:
  - normal outgoing call sequence: confirmed no PII in logs
  - normal incoming call sequence: no PII there either
  - verified the ITelephony call() and dial() APIs still work, and don't
    log any PII either.  (Added new tests to the CallDialTest activity for
    these.)
  - confirmed no remaining STOPSHIPs anywhere under apps/Phone

Bug: 5231962
Change-Id: I995bc58791f553f1a4ad51276b4b31603b196635
  • Loading branch information
David Brown committed Aug 30, 2011
1 parent c03d269 commit a841177
Show file tree
Hide file tree
Showing 8 changed files with 119 additions and 46 deletions.
14 changes: 7 additions & 7 deletions src/com/android/phone/CallController.java
Expand Up @@ -61,7 +61,8 @@ public class CallController extends Handler {
private static final String TAG = "CallController";
private static final boolean DBG =
(PhoneApp.DBG_LEVEL >= 1) && (SystemProperties.getInt("ro.debuggable", 0) == 1);
private static final boolean VDBG = (PhoneApp.DBG_LEVEL >= 2);
// Do not check in with VDBG = true, since that may write PII to the system log.
private static final boolean VDBG = false;

/** The singleton CallController instance. */
private static CallController sInstance;
Expand Down Expand Up @@ -139,8 +140,7 @@ public void handleMessage(Message msg) {

default:
Log.wtf(TAG, "handleMessage: unexpected code: " + msg);
// STOPSHIP: remove throw after initial testing, replace with "break;"
throw new IllegalStateException("handleMessage: unexpected code: " + msg);
break;
}
}

Expand Down Expand Up @@ -204,11 +204,11 @@ public void placeCall(Intent intent) {

String scheme = uri.getScheme();
String number = PhoneNumberUtils.getNumberFromIntent(intent, mApp);
if (DBG) {
if (VDBG) {
log("- action: " + action);
log("- uri: " + uri);
log("- scheme: " + scheme);
log("- number: " + number); // STOPSHIP: don't log number (PII)
log("- number: " + number);
}

// This method should only be used with the various flavors of CALL
Expand Down Expand Up @@ -330,7 +330,7 @@ private CallStatusCode placeCallInternal(Intent intent) {

try {
number = getInitialNumber(intent);
if (DBG) log("- actual number to dial: '" + number + "'"); // STOPSHIP: don't log PII
if (VDBG) log("- actual number to dial: '" + number + "'");

// find the phone first
// TODO Need a way to determine which phone to place the call
Expand Down Expand Up @@ -638,7 +638,7 @@ public static String getInitialNumber(Intent intent)
String actualNumberToDial =
intent.getStringExtra(OutgoingCallBroadcaster.EXTRA_ACTUAL_NUMBER_TO_DIAL);
if (VDBG) log("==> got EXTRA_ACTUAL_NUMBER_TO_DIAL; returning '"
+ actualNumberToDial + "'"); // STOPSHIP: don't log number (PII)
+ actualNumberToDial + "'");
return actualNumberToDial;
}

Expand Down
21 changes: 11 additions & 10 deletions src/com/android/phone/OutgoingCallBroadcaster.java
Expand Up @@ -55,6 +55,8 @@ public class OutgoingCallBroadcaster extends Activity
private static final String TAG = "OutgoingCallBroadcaster";
private static final boolean DBG =
(PhoneApp.DBG_LEVEL >= 1) && (SystemProperties.getInt("ro.debuggable", 0) == 1);
// Do not check in with VDBG = true, since that may write PII to the system log.
private static final boolean VDBG = false;

public static final String ACTION_SIP_SELECT_PHONE = "com.android.phone.SIP_SELECT_PHONE";
public static final String EXTRA_ALREADY_CALLED = "android.phone.extra.ALREADY_CALLED";
Expand Down Expand Up @@ -111,8 +113,7 @@ public void doReceive(Context context, Intent intent) {
// placed.)

number = getResultData();
// STOPSHIP: disable this log message before ship (PII)
if (DBG) Log.v(TAG, "- got number from resultData: '" + number + "'");
if (VDBG) Log.v(TAG, "- got number from resultData: '" + number + "'");

final PhoneApp app = PhoneApp.getInstance();

Expand Down Expand Up @@ -189,9 +190,8 @@ public void doReceive(Context context, Intent intent) {
number = PhoneNumberUtils.stripSeparators(number);

if (DBG) Log.v(TAG, "doReceive: proceeding with call...");
// STOPSHIP: disable these two log messages before ship (PII):
if (DBG) Log.v(TAG, "- uri: " + uri);
if (DBG) Log.v(TAG, "- actual number to dial: '" + number + "'");
if (VDBG) Log.v(TAG, "- uri: " + uri);
if (VDBG) Log.v(TAG, "- actual number to dial: '" + number + "'");

startSipCallOptionHandler(context, intent, uri, number);
}
Expand Down Expand Up @@ -230,11 +230,12 @@ public void doReceive(Context context, Intent intent) {
*/
private void startSipCallOptionHandler(Context context, Intent intent,
Uri uri, String number) {
// Verbose debugging. Do not check in with this enabled (PII).
// Log.i(TAG, "startSipCallOptionHandler...");
// Log.i(TAG, "- intent: " + intent);
// Log.i(TAG, "- uri: " + uri);
// Log.i(TAG, "- number: " + number);
if (VDBG) {
Log.i(TAG, "startSipCallOptionHandler...");
Log.i(TAG, "- intent: " + intent);
Log.i(TAG, "- uri: " + uri);
Log.i(TAG, "- number: " + number);
}

// Create a copy of the original CALL intent that started the whole
// outgoing-call sequence. This intent will ultimately be passed to
Expand Down
5 changes: 0 additions & 5 deletions src/com/android/phone/PhoneInterfaceManager.java
Expand Up @@ -276,11 +276,6 @@ public void call(String number) {

Intent intent = new Intent(Intent.ACTION_CALL, Uri.parse(url));
intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK);
intent.setClassName(mApp, PhoneApp.getCallScreenClassName());

// STOPSHIP: The ACTION_CALL intent needs to go to the
// OutgoingCallBroadcaster nowadays, not the InCallScreen!

mApp.startActivity(intent);
}

Expand Down
27 changes: 14 additions & 13 deletions src/com/android/phone/RespondViaSmsManager.java
Expand Up @@ -29,6 +29,7 @@
import android.content.res.Resources;
import android.net.Uri;
import android.os.Bundle;
import android.os.SystemProperties;
import android.preference.EditTextPreference;
import android.preference.Preference;
import android.preference.PreferenceActivity;
Expand All @@ -48,9 +49,10 @@
*/
public class RespondViaSmsManager {
private static final String TAG = "RespondViaSmsManager";
private static final boolean DBG = true;
// STOPSHIP: reduce DBG to
// (PhoneApp.DBG_LEVEL >= 1) && (SystemProperties.getInt("ro.debuggable", 0) == 1);
private static final boolean DBG =
(PhoneApp.DBG_LEVEL >= 1) && (SystemProperties.getInt("ro.debuggable", 0) == 1);
// Do not check in with VDBG = true, since that may write PII to the system log.
private static final boolean VDBG = false;

/**
* Reference to the InCallScreen activity that owns us. This may be
Expand Down Expand Up @@ -134,7 +136,7 @@ public void showRespondViaSmsPopup(Call ringingCall) {
// chooses a response.)

Connection c = ringingCall.getLatestConnection();
if (DBG) log("- connection: " + c);
if (VDBG) log("- connection: " + c);

if (c == null) {
// Uh oh -- the "ringingCall" doesn't have any connections any more.
Expand All @@ -154,7 +156,7 @@ public void showRespondViaSmsPopup(Call ringingCall) {
// first place.)

String phoneNumber = c.getAddress();
if (DBG) log("- phoneNumber: " + phoneNumber); // STOPSHIP: don't log PII
if (VDBG) log("- phoneNumber: " + phoneNumber);
lv.setOnItemClickListener(new RespondViaSmsItemClickListener(phoneNumber));

AlertDialog.Builder builder = new AlertDialog.Builder(mInCallScreen)
Expand Down Expand Up @@ -199,7 +201,7 @@ public void onItemClick(AdapterView<?> parent, // The ListView
long id) {
if (DBG) log("RespondViaSmsItemClickListener.onItemClick(" + position + ")...");
String message = (String) parent.getItemAtPosition(position);
if (DBG) log("- message: '" + message + "'");
if (VDBG) log("- message: '" + message + "'");

// The "Custom" choice is a special case.
// (For now, it's guaranteed to be the last item.)
Expand Down Expand Up @@ -285,9 +287,8 @@ public void onCancel(DialogInterface dialog) {
* Sends a text message without any interaction from the user.
*/
private void sendText(String phoneNumber, String message) {
// STOPSHIP: disable all logging of PII (everywhere in this file)
if (DBG) log("sendText: number "
+ phoneNumber + ", message '" + message + "'");
if (VDBG) log("sendText: number "
+ phoneNumber + ", message '" + message + "'");

Uri uri = Uri.fromParts(Constants.SCHEME_SMSTO, phoneNumber, null);
Intent intent = new Intent("com.android.mms.intent.action.SENDTO_NO_CONFIRMATION", uri);
Expand All @@ -299,12 +300,12 @@ private void sendText(String phoneNumber, String message) {
* Brings up the standard SMS compose UI.
*/
private void launchSmsCompose(String phoneNumber) {
if (DBG) log("launchSmsCompose: number " + phoneNumber);
if (VDBG) log("launchSmsCompose: number " + phoneNumber);

Uri uri = Uri.fromParts(Constants.SCHEME_SMS, phoneNumber, null);
Intent intent = new Intent(Intent.ACTION_VIEW, uri);

if (DBG) log("- Launching SMS compose UI: " + intent); // STOPSHIP: disable logging of PII
if (VDBG) log("- Launching SMS compose UI: " + intent);
mInCallScreen.startActivity(intent);

// TODO: One open issue here: if the user selects "Custom message"
Expand Down Expand Up @@ -381,8 +382,8 @@ protected void onCreate(Bundle icicle) {
// Preference.OnPreferenceChangeListener implementation
public boolean onPreferenceChange(Preference preference, Object newValue) {
if (DBG) log("onPreferenceChange: key = " + preference.getKey());
if (DBG) log(" preference = '" + preference + "'");
if (DBG) log(" newValue = '" + newValue + "'");
if (VDBG) log(" preference = '" + preference + "'");
if (VDBG) log(" newValue = '" + newValue + "'");

EditTextPreference pref = (EditTextPreference) preference;

Expand Down
4 changes: 2 additions & 2 deletions src/com/android/phone/SipCallOptionHandler.java
Expand Up @@ -97,8 +97,8 @@ public void onCreate(Bundle savedInstanceState) {
if (!OutgoingCallBroadcaster.ACTION_SIP_SELECT_PHONE.equals(action)) {
Log.wtf(TAG, "onCreate: got intent action '" + action + "', expected "
+ OutgoingCallBroadcaster.ACTION_SIP_SELECT_PHONE);
// STOPSHIP: remove this throw before ship (but leave the Log.wtf())
throw new IllegalArgumentException("Unexpected intent action '" + action + "'");
finish();
return;
}

// mIntent is a copy of the original CALL intent that started the
Expand Down
32 changes: 31 additions & 1 deletion tests/res/layout/call_dial_test.xml
Expand Up @@ -45,7 +45,7 @@
android:textAppearance="?android:attr/textAppearanceLarge" />
<View android:layout_width="1dip"
android:layout_height="1dip"
android:layout_weight="1" />
android:layout_weight="2" />

<LinearLayout
android:orientation="horizontal"
Expand Down Expand Up @@ -73,6 +73,36 @@
android:textAppearance="?android:attr/textAppearanceLarge"
/>
</LinearLayout>
<View android:layout_width="1dip"
android:layout_height="1dip"
android:layout_weight="2" />

<LinearLayout
android:orientation="vertical"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
>
<Button android:id="@+id/itelephonyCallButton"
android:text="@string/itelephonyCallCommand"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_marginLeft="20dip"
android:layout_marginRight="20dip"
android:paddingLeft="32dip"
android:paddingRight="32dip"
android:textAppearance="?android:attr/textAppearanceMedium"
/>
<Button android:id="@+id/itelephonyDialButton"
android:text="@string/itelephonyDialCommand"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_marginLeft="20dip"
android:layout_marginRight="20dip"
android:paddingLeft="32dip"
android:paddingRight="32dip"
android:textAppearance="?android:attr/textAppearanceMedium"
/>
</LinearLayout>
<View android:layout_width="1dip"
android:layout_height="1dip"
android:layout_weight="4" />
Expand Down
2 changes: 2 additions & 0 deletions tests/res/values/donottranslate_strings.xml
Expand Up @@ -22,4 +22,6 @@
<string name="callDialTestLabel">CALL/DIAL test</string>
<string name="callCommand">CALL</string>
<string name="dialCommand">DIAL</string>
<string name="itelephonyCallCommand">ITelephony.call()</string>
<string name="itelephonyDialCommand">ITelephony.dial()</string>
</resources>
60 changes: 52 additions & 8 deletions tests/src/com/android/phone/tests/CallDialTest.java
Expand Up @@ -21,6 +21,8 @@
import android.content.Intent;
import android.net.Uri;
import android.os.Bundle;
import android.os.RemoteException;
import android.os.ServiceManager;
import android.telephony.PhoneNumberUtils;
import android.text.TextUtils;
import android.util.Log;
Expand All @@ -30,6 +32,7 @@
import android.widget.TextView;
import android.widget.Toast;

import com.android.internal.telephony.ITelephony;
import com.android.phone.Constants;

/**
Expand All @@ -42,8 +45,6 @@ public class CallDialTest extends Activity implements View.OnClickListener {
// UI elements
private TextView mLabel;
private EditText mNumber;
private Button mCallButton;
private Button mDialButton;

@Override
protected void onCreate(Bundle savedInstanceState) {
Expand All @@ -59,11 +60,10 @@ protected void onCreate(Bundle savedInstanceState) {
mNumber = (EditText) findViewById(R.id.number);
mNumber.setText("6505551234"); // Preload it with something useful

mCallButton = (Button) findViewById(R.id.callButton);
mCallButton.setOnClickListener(this);

mDialButton = (Button) findViewById(R.id.dialButton);
mDialButton.setOnClickListener(this);
((Button) findViewById(R.id.callButton)).setOnClickListener(this);
((Button) findViewById(R.id.dialButton)).setOnClickListener(this);
((Button) findViewById(R.id.itelephonyCallButton)).setOnClickListener(this);
((Button) findViewById(R.id.itelephonyDialButton)).setOnClickListener(this);
}

@Override
Expand Down Expand Up @@ -93,8 +93,16 @@ public void onClick(View view) {
log("onClick: DIAL...");
fireIntent(Intent.ACTION_DIAL);
break;
case R.id.itelephonyCallButton:
log("onClick: ITelephony.call()...");
doITelephonyCall();
break;
case R.id.itelephonyDialButton:
log("onClick: ITelephony.dial()...");
doITelephonyDial();
break;
default:
Log.w(LOG_TAG, "onClick: unexpected View: " + view);
Log.wtf(LOG_TAG, "onClick: unexpected View: " + view);
break;
}
}
Expand Down Expand Up @@ -139,6 +147,42 @@ private void fireIntent(String action) {
}
}

private void doITelephonyCall() {
log("doITelephonyCall()...");

// Get a phone number from the EditText widget
String number = mNumber.getText().toString();
log("==> number: '" + number + "'");

try {
ITelephony phone = ITelephony.Stub.asInterface(ServiceManager.checkService("phone"));
log("- phone: " + phone);
log("- calling call()...");
phone.call(number);
log(" Done.");
} catch (RemoteException ex) {
Log.w(LOG_TAG, "RemoteException!", ex);
}
}

private void doITelephonyDial() {
log("doITelephonyDial()...");

// Get a phone number from the EditText widget
String number = mNumber.getText().toString();
log("==> number: '" + number + "'");

try {
ITelephony phone = ITelephony.Stub.asInterface(ServiceManager.checkService("phone"));
log("- phone: " + phone);
log("- calling dial()...");
phone.dial(number);
log(" Done.");
} catch (RemoteException ex) {
Log.w(LOG_TAG, "RemoteException!", ex);
}
}

private void log(String msg) {
Log.i(LOG_TAG, msg);
}
Expand Down

0 comments on commit a841177

Please sign in to comment.