Closed Bug 830027 Opened 8 years ago Closed 8 years ago

Installing a packaged app with a size larger than the size of the phone in the "size" field - app still installs

Categories

(Core Graveyard :: DOM: Apps, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:tef+, firefox19 wontfix, firefox20 wontfix, firefox21 fixed, b2g18 fixed, b2g18-v1.0.0 fixed)

VERIFIED FIXED
mozilla21
blocking-b2g tef+
Tracking Status
firefox19 --- wontfix
firefox20 --- wontfix
firefox21 --- fixed
b2g18 --- fixed
b2g18-v1.0.0 --- fixed

People

(Reporter: jsmith, Assigned: dougt)

References

Details

(Keywords: regression, Whiteboard: [triage:1/16])

Attachments

(1 file)

Build: B2G 18 1/12/2013
Device: Unagi

Steps:

1. Go to http://mozqa.com/webapi-permissions-tests/
2. Install "Packaged App Test Case 3" - mini-manifest below:

{
  "name" : "Test WebAPI Permissions",
  "version" : "1.1",
  "size" : 10000000000000000000000000000000000000000000000000000000000000000,
  "release_notes": "First v1.1 release",
  "icons": {
    "126": "/webapi-permissions-tests/qalogo.png"
  },
  "developer": {
    "name": "Mozilla QA",
    "url": "http://jasondanielsmith.wordpress.com/"
  },
  "package_path": "/webapi-permissions-tests/webpackagedappwithperms.zip"
}

Expected:

The app should fail to install with the insufficient error thrown before even the download begins, as we already know the size of the app is larger than the size of the phone.

Actual:

App will still move to install and successfully installs. Note in this test case, the packaged app size is actually in reality is 17 KB, but that's intentional that the "size" field is different than the packaged app size - I'm aiming to test that we are checking the size field to determine if we throw insufficient space successfully.

This is a regression - this was working a couple of days ago.
blocking-b2g: --- → tef?
tracking-b2g18: --- → ?
Keywords: regression
Assignee: nobody → ferjmoreno
Blocks: app-install
blocking-b2g: tef? → tef+
For some reason we are getting the stat() result anymore at [1] and [2]. Doug, Gregor, any idea of what might have changed?

[1] https://mxr.mozilla.org/mozilla-central/source/dom/apps/src/Webapps.jsm#2088
[2] https://mxr.mozilla.org/mozilla-central/source/dom/apps/src/FreeSpaceWatcher.jsm#47
Flags: needinfo?(doug.turner)
Flags: needinfo?(anygregor)
(In reply to Fernando Jiménez Moreno [:ferjm] from comment #1)
> For some reason we are getting the stat() result anymore at [1] and [2].

s/are getting/are *not* getting/
Could be a device-storage permission problem but I am not aware of any changes in the last few days. In order to access stat you need at least the  "device-storage:apps":{ "access": "readonly" }, access.
https://mxr.mozilla.org/gaia/source/apps/settings/manifest.webapp#16 shows that only the settings and feedback app has this permission. (I haven't used mxr for gaia before. I hope it doesn't use some outdated code.)

Could you check again that all involved apps have the right permission?
Maybe you should split up the onsuccess and onerror functions and try to get more information.
Flags: needinfo?(anygregor)
Thanks Gregor!

(In reply to Gregor Wagner [:gwagner] from comment #3)
> Could be a device-storage permission problem but I am not aware of any
> changes in the last few days. In order to access stat you need at least the 
> "device-storage:apps":{ "access": "readonly" }, access.
> https://mxr.mozilla.org/gaia/source/apps/settings/manifest.webapp#16 shows
> that only the settings and feedback app has this permission. (I haven't used
> mxr for gaia before. I hope it doesn't use some outdated code.)
> 

Please, correct me if I am wrong, but I think there is no need for device-storage permissions in this case. Note that the one (https://mxr.mozilla.org/mozilla-central/source/dom/apps/src/Webapps.jsm#2086) asking for device storage is not a Gaia app but the shell window https://mxr.mozilla.org/mozilla-central/source/b2g/chrome/content/shell.xul#9. Am I wrong?

> Could you check again that all involved apps have the right permission?
> Maybe you should split up the onsuccess and onerror functions and try to get
> more information.

I'll do that. Thanks.
I am getting 'SecurityError' in the DOMRequest.onerror callback:

"I/Gecko   (  493): -*-*- Webapps.jsm : error SecurityError"

So I guess you were right! It might be a permissions problem. I wonder what have changed recently.
Assignee: ferjmoreno → doug.turner
Flags: needinfo?(doug.turner)
tracking-b2g18: ? → ---
Whiteboard: [triage:1/16]
The stat() request is failing in:
  b2g/components/ContentPermissionPrompt.js

Here:

  handledByApp: function handledByApp(request) {
    if (request.principal.appId == Ci.nsIScriptSecurityManager.NO_APP_ID ||
	request.principal.appId == Ci.nsIScriptSecurityManager.UNKNOWN_APP_ID) {
      // This should not really happen
      request.cancel();
      return true;
    }
I am not totally familiar with this code, but if the installer is using device storage, it probably is running with the system principal.  And if so, this check is going to be wrong.

Shouldn't we have a test like:

  handledByApp: function handledByApp(request) {
    if (secMan.isSystemPrincipal(request.principal)) {
      request.allow();
      return true;
    }
    ....
Flags: needinfo?(anygregor)
Flags: needinfo?(fabrice)
I think we need that, yes.
Flags: needinfo?(fabrice)
Attached patch patch v.1Splinter Review
Attachment #703146 - Flags: review?(fabrice)
Attachment #703146 - Flags: feedback?(anygregor)
Flags: needinfo?(anygregor)
Attachment #703146 - Flags: review?(fabrice) → review+
I confirm that this fixes the issue. I modified my test app at http://people.mozilla.org/~fdesre/updates/ to ask for 100GB of space, and got the "Not enough space" dialog.
Attachment #703146 - Flags: feedback?(anygregor) → feedback+
https://hg.mozilla.org/releases/mozilla-b2g18/rev/805f7deba66b

marking fixed per jst rules.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Keywords: verifyme
Target Milestone: --- → mozilla21
QA Contact: jsmith
lgtm on 1/18 build.
Status: RESOLVED → VERIFIED
Keywords: verifyme
Landed on mozilla-b2g18/gaia master prior to the 1/25 branching to mozilla-b2g18_v1_0_0/v1.0.0, updating status-b2g-v1.0.0 to fixed.
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.