RestrictedProfiles.isAllowed(int action, string url) defaults to true when it can't find the action

RESOLVED FIXED in Firefox 35

Status

()

defect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: jaws, Assigned: rnewman)

Tracking

unspecified
Firefox 35
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/RestrictedProfiles.java?rev=be045ef85f3f#147

When isAllowed() is called with an action that can't be found (see https://bugzilla.mozilla.org/show_bug.cgi?id=1046941#c21) it appears that the code here catches the exception and returns `true`.

A couple things could change with this function:
>  @WrapElementForJNI
>  public static boolean isAllowed(int action, String url) {
>+     // ALl actions are blocked in Guest mode
>+     if (getInGuest()) {
>+         return false;
>+     }
>  
>      final Restriction restriction;
>      try {
>          restriction = geckoActionToRestriction(action);
>      } catch(IllegalArgumentException ex) {
>-         return true;
>+         Log.i(LOGTAG, "Invalid action", ex);
>+         return false;
>      }
>  
>      if (Restriction.DISALLOW_BROWSE_FILES == restriction) {
>          return canLoadUrl(url);
>      }
>  
>-     // ALl actions are blocked in Guest mode
>-     if (getInGuest()) {
>-         return false;
>-     }
>  
>      if (Versions.preJBMR2) {
>          return true;
>      }
>  
>      try {
>          // NOTE: Restrictions hold the opposite intention, so we need to flip it
>          return !getRestrictions().getBoolean(restriction.name, false);
>      } catch(IllegalArgumentException ex) {
>          Log.i(LOGTAG, "Invalid action", ex);
>      }
>  
>      return true;
>  }

Above I proposed that we return `false` as it defaults to the "safer" side of things, as well as will make it more noticeable when something doesn't work.
Component: General → Profile Handling
This patch:

* Adds some Android version safety.
* Encapsulates the version check in a helper method.
* Tidies up spelling, grammar, and imports.
* Fixes the bug as suggested.
Attachment #8502239 - Flags: review?(mark.finkle)
Assignee: nobody → rnewman
Status: NEW → ASSIGNED
Attachment #8502239 - Flags: review?(mark.finkle) → review+
https://hg.mozilla.org/mozilla-central/rev/4b9e08526336
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
Blocks: 1083846
You need to log in before you can comment on or make changes to this bug.