Skip to content

Commit

Permalink
Disable logging by default
Browse files Browse the repository at this point in the history
Summary:
Logging was turned on unconditionally before, which led to apps leaking
sensitive data.  This change puts the logging api behind an explicit gate that
developers have to turn on.

It's unfortunate that this isn't automatic - ideally this would automatically
turn on for non-release signed bits.  I couldn't find such a check in Android
framework.  If android experts have better ways of tackling this, i'm all ears.
But bear in mind this is a security fix and needs to go out asap.

Test Plan: Launched in default mode and verified no logging in emulator.
Turned on log gate and verified logging.

Reviewers: mmarucheck, lshepard, yariv, raghuc1

Reviewed By: mmarucheck

CC: gregschechte, jacl

Differential Revision: https://phabricator.fb.com/D411377

Task ID: 933141
  • Loading branch information
vijaye committed Feb 17, 2012
1 parent fc4785c commit 1f0b0ea
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 14 deletions.
21 changes: 10 additions & 11 deletions facebook/src/com/facebook/android/Facebook.java
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@
import android.os.Messenger;
import android.os.RemoteException;
import android.text.TextUtils;
import android.util.Log;
import android.webkit.CookieSyncManager;

/**
Expand Down Expand Up @@ -349,7 +348,7 @@ public void onComplete(Bundle values) {
setAccessToken(values.getString(TOKEN));
setAccessExpiresIn(values.getString(EXPIRES));
if (isSessionValid()) {
Log.d("Facebook-authorize", "Login Success! access_token="
Util.logd("Facebook-authorize", "Login Success! access_token="
+ getAccessToken() + " expires="
+ getAccessExpires());
mAuthDialogListener.onComplete(values);
Expand All @@ -360,17 +359,17 @@ public void onComplete(Bundle values) {
}

public void onError(DialogError error) {
Log.d("Facebook-authorize", "Login failed: " + error);
Util.logd("Facebook-authorize", "Login failed: " + error);
mAuthDialogListener.onError(error);
}

public void onFacebookError(FacebookError error) {
Log.d("Facebook-authorize", "Login failed: " + error);
Util.logd("Facebook-authorize", "Login failed: " + error);
mAuthDialogListener.onFacebookError(error);
}

public void onCancel() {
Log.d("Facebook-authorize", "Login canceled");
Util.logd("Facebook-authorize", "Login canceled");
mAuthDialogListener.onCancel();
}
});
Expand Down Expand Up @@ -405,19 +404,19 @@ public void authorizeCallback(int requestCode, int resultCode, Intent data) {
if (error != null) {
if (error.equals(SINGLE_SIGN_ON_DISABLED)
|| error.equals("AndroidAuthKillSwitchException")) {
Log.d("Facebook-authorize", "Hosted auth currently "
Util.logd("Facebook-authorize", "Hosted auth currently "
+ "disabled. Retrying dialog auth...");
startDialogAuth(mAuthActivity, mAuthPermissions);
} else if (error.equals("access_denied")
|| error.equals("OAuthAccessDeniedException")) {
Log.d("Facebook-authorize", "Login canceled by user.");
Util.logd("Facebook-authorize", "Login canceled by user.");
mAuthDialogListener.onCancel();
} else {
String description = data.getStringExtra("error_description");
if (description != null) {
error = error + ":" + description;
}
Log.d("Facebook-authorize", "Login failed: " + error);
Util.logd("Facebook-authorize", "Login failed: " + error);
mAuthDialogListener.onFacebookError(
new FacebookError(error));
}
Expand All @@ -427,7 +426,7 @@ public void authorizeCallback(int requestCode, int resultCode, Intent data) {
setAccessToken(data.getStringExtra(TOKEN));
setAccessExpiresIn(data.getStringExtra(EXPIRES));
if (isSessionValid()) {
Log.d("Facebook-authorize",
Util.logd("Facebook-authorize",
"Login Success! access_token="
+ getAccessToken() + " expires="
+ getAccessExpires());
Expand All @@ -443,7 +442,7 @@ public void authorizeCallback(int requestCode, int resultCode, Intent data) {

// An Android error occured.
if (data != null) {
Log.d("Facebook-authorize",
Util.logd("Facebook-authorize",
"Login failed: " + data.getStringExtra("error"));
mAuthDialogListener.onError(
new DialogError(
Expand All @@ -453,7 +452,7 @@ public void authorizeCallback(int requestCode, int resultCode, Intent data) {

// User pressed the 'back' button.
} else {
Log.d("Facebook-authorize", "Login canceled by user.");
Util.logd("Facebook-authorize", "Login canceled by user.");
mAuthDialogListener.onCancel();
}
}
Expand Down
4 changes: 2 additions & 2 deletions facebook/src/com/facebook/android/FbDialog.java
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ private class FbWebViewClient extends WebViewClient {

@Override
public boolean shouldOverrideUrlLoading(WebView view, String url) {
Log.d("Facebook-WebView", "Redirect URL: " + url);
Util.logd("Facebook-WebView", "Redirect URL: " + url);
if (url.startsWith(Facebook.REDIRECT_URI)) {
Bundle values = Util.parseUrl(url);

Expand Down Expand Up @@ -175,7 +175,7 @@ public void onReceivedError(WebView view, int errorCode,

@Override
public void onPageStarted(WebView view, String url, Bitmap favicon) {
Log.d("Facebook-WebView", "Webview loading URL: " + url);
Util.logd("Facebook-WebView", "Webview loading URL: " + url);
super.onPageStarted(view, url, favicon);
mSpinner.show();
}
Expand Down
21 changes: 20 additions & 1 deletion facebook/src/com/facebook/android/Util.java
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,12 @@
*/
public final class Util {

/**
* Set this to true to enable log output. Remember to turn this back off
* before releasing. Sending sensitive data to log is a security risk.
*/
private static boolean ENABLE_LOG = false;

/**
* Generate the multi-part post body providing the parameters and boundary
* string
Expand Down Expand Up @@ -144,7 +150,7 @@ public static String openUrl(String url, String method, Bundle params)
if (method.equals("GET")) {
url = url + "?" + encodeUrl(params);
}
Log.d("Facebook-Util", method + " URL: " + url);
Util.logd("Facebook-Util", method + " URL: " + url);
HttpURLConnection conn =
(HttpURLConnection) new URL(url).openConnection();
conn.setRequestProperty("User-Agent", System.getProperties().
Expand Down Expand Up @@ -298,4 +304,17 @@ public static void showAlert(Context context, String title, String text) {
alertBuilder.create().show();
}

/**
* A proxy for Log.d api that kills log messages in release build. It
* not recommended to send sensitive information to log output in
* shipping apps.
*
* @param tag
* @param msg
*/
public static void logd(String tag, String msg) {
if (ENABLE_LOG) {
Log.d(tag, msg);
}
}
}

0 comments on commit 1f0b0ea

Please sign in to comment.