Bad packaged app upload blocks all future upload attempts

VERIFIED FIXED in Firefox 28

Status

()

Firefox
Developer Tools: WebIDE
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: jryans, Assigned: jryans)

Tracking

Trunk
Firefox 28
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

If you transfer a packaged app to the device, but it gets corrupted in transit, it's possible to get stuck in a state where the app can't ever be uploaded successfully anymore.

We currently skip moving the uploaded file (at /data/local/tmp/file-upload/package.zip) to the temporary application directory (at /data/local/tmp/b2g/APP_ID) if the directory already exists, but this is possible if a previous upload failed to complete correctly.
Created attachment 828498 [details] [diff] [review]
Always move uploaded package to installation dir

Try: https://tbpl.mozilla.org/?tree=Try&rev=ba95ac805075
Attachment #828498 - Flags: review?(poirot.alex)
Comment on attachment 828498 [details] [diff] [review]
Always move uploaded package to installation dir

Review of attachment 828498 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/devtools/server/actors/webapps.js
@@ +508,5 @@
> +        return { error: "badParameter",
> +                 message: "The uploaded file doesn't exist on device" };
> +      }
> +      appFile.moveTo(appDir, "application.zip");
> +    } else if (!aRequest.manifest && !aRequest.metadata) {

You should tweak this condition in order to not return this error 
if appDir && appDir.exists()

otherwise, you will just forbid app install via adb push.
Attachment #828498 - Flags: review?(poirot.alex) → review-
Created attachment 828887 [details] [diff] [review]
Always move uploaded package to installation dir v2

Try: https://tbpl.mozilla.org/?tree=Try&rev=9920bab8e082

(In reply to Alexandre Poirot (:ochameau) from comment #2)
> Comment on attachment 828498 [details] [diff] [review]
> Always move uploaded package to installation dir
> 
> Review of attachment 828498 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: toolkit/devtools/server/actors/webapps.js
> @@ +508,5 @@
> > +        return { error: "badParameter",
> > +                 message: "The uploaded file doesn't exist on device" };
> > +      }
> > +      appFile.moveTo(appDir, "application.zip");
> > +    } else if (!aRequest.manifest && !aRequest.metadata) {
> 
> You should tweak this condition in order to not return this error 
> if appDir && appDir.exists()
> 
> otherwise, you will just forbid app install via adb push.

Thanks for catching this, I didn't even know it was something people used! :)
Attachment #828498 - Attachment is obsolete: true
Attachment #828887 - Flags: review?(poirot.alex)
Attachment #828887 - Flags: review?(poirot.alex) → review+
https://hg.mozilla.org/integration/fx-team/rev/e7d6e224ba57
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/e7d6e224ba57
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 28

Updated

4 years ago
Keywords: verifyme
Verified that using latest Firefox 28 beta 4 the zip file is also moved to the temporary application directory (at /data/local/tmp/b2g) while with an oldest Nightly it is only moved to file-upload folder.

Is this verification enough to mark this bug as Verified or we should also reproduce the case with an app that gets corrupted while updating? If yes, please let us know how to reproduce this case. Thank you!
Flags: needinfo?(jryans)
Does an "oldest Nightly" mean Firefox 30?  Or something before Firefox 28...?

It's a bit a little while since this happened, but I believe you could test more thoroughly by trying to transfer a large app (which would take a while), and disconnecting the device in the middle of the installation process the first time.  That would simulate the "corrupted" situation.

Then, if you are able to successfully install anyway a second time, that should be enough to verify this.
Flags: needinfo?(jryans)
Thanks for the updates. I was using Firefox 28.0a1 (20131105030206).
On this version I couldn't reproduce the "corrupted situation" - the largest app I have is about 3MB and takes around 8 sec to install. 
I disconnected the device several times before actually let it be installed, but at the end it was installed and running.

The only thing I noticed is that the Update / Debug buttons remain grayed out in Apps tab after reconnecting the device. Switching to another app from list and returning will displayed the buttons correctly (happens on Fx 28 beta 6 too).

Should I try with a larger packaged app?
(In reply to Petruta Rasa [QA] [:petruta] from comment #8)
> Thanks for the updates. I was using Firefox 28.0a1 (20131105030206).
> On this version I couldn't reproduce the "corrupted situation" - the largest
> app I have is about 3MB and takes around 8 sec to install. 
> I disconnected the device several times before actually let it be installed,
> but at the end it was installed and running.
> 
> The only thing I noticed is that the Update / Debug buttons remain grayed
> out in Apps tab after reconnecting the device. Switching to another app from
> list and returning will displayed the buttons correctly (happens on Fx 28
> beta 6 too).
> 
> Should I try with a larger packaged app?

Just looked more closely at the patch again.  I had forgotten to say that this change is on device only, so the version of Firefox does not matter here.

If you wanted to reproduce the original problem, you would need a B2G build from before this patch landed.  Anyway, it's seems like this is working from what you've tested so far, so it seems fine to mark as verified to me.

Not sure if the debug buttons remaining grayed out after reconnection still applies to Nightly too, but if so it would be good to file a new bug for that issue (which is unrelated to this one).
Thanks Ryan!
I'll file a new bug for Update / Debug buttons.

I used an unagi device with B2G 1.2, platform version 26.0a2 and build identifier 20131014004003 and I was able to install the app after several disconnections.

I will mark this as verified based on previously results and Comment 9.
Assignee: jryans → petruta.rasa
Status: RESOLVED → VERIFIED
Keywords: verifyme
Assignee: petruta.rasa → jryans
QA Contact: petruta.rasa
You need to log in before you can comment on or make changes to this bug.