Fenix: [Bug] downloads that require a user to be logged in to a website fail

[kbrosnan]

• [x] https://bugzilla.mozilla.org/show_bug.cgi?id=1530022

Steps to reproduce

• Login to Bugzilla
• Open a restricted eg bug 1607647
  
  
  
  • anyone who needs access to this just ask. This is a test bug that has no confidential information. Its sole purpose is to have an attachment behind a login.
  
  
• Download Success.html attachment

Expected behavior

Success.html file is downloaded

Actual behavior

Access denied Bugzilla page is saved.

Device information

• Android device: Pixel 3
• Fenix version: Nightly

[lnicola]

This happens because Fenix downloads the file twice. See #7961 for details.

[st3fan]

@kbrosnan would you mind verifying this bug again? It sounds like #7961 fixed this?

[lnicola]

Not working for me in Build #20870608.

[lnicola]

@st3fan wait, how is #7961 supposed to have fixed this? It's a duplicate issue of it.

[st3fan]

Oh I see, I thought #7961 was closed with a PR, but it is indeed a dupe of this one.

[kbrosnan]

This still reproduces in Nightly and STR in comment 0.

[Amejia481]

This is GV bug https://bugzilla.mozilla.org/show_bug.cgi?id=1626335, it looks like we are not sharing some headers and for this reason we are getting the access denied.

[liuche]

Looks like a fix landed, but got backed out.

[lnicola]

@Amejia481, @liuche I'm not sure, did you see https://github.com/mozilla-mobile/fenix/issues/7961#issue-556553954? Fenix makes two HTTP requests. The first one is fine, but the second one has weird headers, and not only cookies.

[Amejia481]

@Amejia481, @liuche I'm not sure, did you see #7961 (comment)? Fenix makes two HTTP requests. The first one is fine, but the second one has weird headers, and not only cookies.

Didn't have the opportunity of seeing it. It looks like https://github.com/mozilla-mobile/fenix/issues/7961 is a different issue, we could reopen to take a look at it.

A fix for this issue from GeckoView is very close to land https://bugzilla.mozilla.org/show_bug.cgi?id=1626335

[pocmo]

GeckoView patch landed and latest AC Nightly should have that patch. So we should be able to verify it in the next Fenix Nightly.

[Amejia481]

It's working on nightly Saturday 7/11 @ 6:08 AM

Nightly 200711 06:00 (Build #21930609)
AC: 50.0.20200710130140, b54541be1
GV: 80.0a1-20200709093347
AS: 61.0.7

[kbrosnan]

@Amejia481 I still get the not logged in Bugzilla page testing in normal and private browsing. Seeing success.html is not enough to verify this you need to open the page an verify that you don't get the Access denied Bugzilla page.

Nightly 200712 06:47 (Build #21950658)
AC: 50.0.0200712130829, 22f032ca7
GV: 80.0a1-20200711092223
AS: 61.0.7

[Amejia481]

Unfortunately, you are right :( I'm getting the access denied Bugzilla page

[Amejia481]

@kbrosnan are you able to download in other authenticated sites? when I initially tested I was able to download my payment statement from https://workforcenow.adp.com

[AndiAJ]

Hi, I've just re-checked this on the latest Nightly Build 200714 from 7/14 and 79.0.0-beta.6 from 7/14 using a Google Pixel 3a (Android 10) & OnePlus A3 (Android 6.0.1) and it seems that the Access denied Bugzilla page is actually saved.

► Video

I'll remove the QA needed label until further notice

[st3fan]

I think we need a better test plan for this than just Bugzilla. Different sites do different things. Some redirect to a different domain, others ask the user to tap on a link to a different domain. I think it would be really good to have a bit of a test matrix for this one to make sure we cover all cases. (And explore what those cases/variants are)

[lnicola]

This might actually be #7961.

[st3fan]

I see #7961 mentions a different user agent - that can definitely get in the way of authentication. Some sites hash client attributes like that to make sure the download happens from the same browser. And will reject it if it does not look like it is coming from the same.

[lnicola]

@st3fan it's not just the user agent. The second request is missing the cookies.

[Amejia481]

I think we need a better test plan for this than just Bugzilla. Different sites do different things. Some redirect to a different domain, others ask the user to tap on a link to a different domain. I think it would be really good to have a bit of a test matrix for this one to make sure we cover all cases. (And explore what those cases/variants are)

I agree with @st3fan, different sites have different ways to allow to download authenticated content.

Sorry for all the confusion, I think I understood what is happening here. I thought this bug was the caused of not able to download any file from authenticated sites. But I was wrong, there were some authenticated sites that before this patch, we were able to download from, this was the case of https://workforcenow.adp.com. It looks like when I tested I was using an old cached dependency of geckoview and there https://workforcenow.adp.com was working, now with this patch I'm getting an HTTP 500 error and not able to download, when I omit the passing the headers I'm able to download from that site, unfortunately it looks like we are regressing with the patch.

[vesta0]

This is super high priority - please let's figure out a way to fix this as soon as possible.

[Amejia481]

This is GV bug https://bugzilla.mozilla.org/show_bug.cgi?id=1626335, it looks like we are not sharing some headers and for this reason we are getting the access denied.

The initial bugs wasn't the cause for this issue I open https://bugzilla.mozilla.org/show_bug.cgi?id=1652856 for checking the for Bugzilla attachments, I think it will be useful to have a list of sites ordered by priority where files that require authentication are failing, this way we can tackle the sites with highest priority

[Amejia481]

Maybe QA can help us with that

[AndiAJ]

Hi @Amejia481 , please review my findings after re-checking this matter on 79.0.0-beta.6 from 7/14 using the following devices:
• Google Pixel 3a (Android 10)
• OnePlus A3 (Android 6.0.1)

Signed in

• Google Drive - apk download
✔️ Successfully downloaded and installed the apk

• Gmail - gif download
✔️ Successfully downloaded and opened the gif

• Gmail -> WeTransfer link jpg, apk, pdf
✔️ Successfully downloaded
❓ Can't open the files from Fenix
✔️ Can open them from the file manager

• Udemy - zip file
✔️ Successfully downloaded
✔️ Can't open the files from Fenix
✔️ Can properly open them from the file manager

• Wizair - pdf file
✔️ Successfully downloaded and opened the pdf file

• Github - txt file
✔️ Successfully downloaded and opened the txt file

• Github link from #12297 - apk file
✔️ Successfully downloaded and opened the apk file

• Oracle - exe file
✔️ Successfully downloaded
✔️ Can't open the file (Win 10 exe file)

Not signed in

• Link from #11436 - pdf file
✔️ Successfully downloaded
❌ Can't open it from Fenix nor from the File manager
❗ Works on Chrome

• Link from #54064 - zip file
❌ Download failed instantaneously
❗ Works on Chrome

I'll remove the QA needed label until further notice.

[kglazko]

Hi @vesta0 sounds like the action item here is: " I think it will be useful to have a list of sites ordered by priority where files that require authentication are failing, this way we can tackle the sites with highest priority". It seems like this is working on some sites and not others.

@Amejia481 What do you think about me setting up a spreadsheet to keep track of sites where downloads are reported to work/fail?

[Amejia481]

That will be awesome ❤️

[avdv]

One more data point: I encountered this problem when trying to download my books from manning.com, it would successfully transfer the file, but it would have the HTML page in it, not the PDF or .epub contents.

[st3fan]

@kglazko great idea to setup a spreadsheet. I do want to make sure that we do not implement site-specific fixes. Instead we should understand how these websites present downloads to the browser at the HTTP level and properly deal with those cases. Or why our download request is rejected.

[Amejia481]

@kglazko great idea to setup a spreadsheet. I do want to make sure that we do not implement site-specific fixes. Instead we should understand how these websites present downloads to the browser at the HTTP level and properly deal with those cases. Or why our download request is rejected.

Agree, with the list we just want to identify which are the most common issues with authenticated downloads and address first the ones that are in the more popular sites. Something similar to what we did with fav icons :)

[chutten]

Can confirm this is still an issue for downloading the ebook of the month from tor.com on Nightly 2007220608 (get a "Download Failed" prompt. Works fine on Fennec, Chrome, Samsung Internet)

[liuche]

To summarize w/ the specific sites:

✅ Working for: Gmail, GDrive, Udemy, WizAir (file hosting), Github, Oracle
❌ Failing on: tor.com, dbextra, wowroms

May also be related to #7961.

[Amejia481]

We are block on this bugzilla bug https://bugzilla.mozilla.org/show_bug.cgi?id=1530022

[vesta0]

@liuche this is probably good enough right now (no longer urgent) but we should still fix the remaining use cases.

[liuche]

This looks like it landed in GV 2 days ago, so it should be in the next GV Nightly. I'll add a QA-needed label.

[victor-bayas]

I can now download invoices from my country's IRS equivalent but the download PDF file it's corrupted (0.01mb)

[lnicola]

@victor-bayas the PDF is likely an HTML page.

[victor-bayas]

@victor-bayas the PDF is likely an HTML page.

But Chromium can download it without problems, Fenix it's now Mozilla's production browser for Android

[Amejia481]

I'm integrating a new GV api on AC https://github.com/mozilla-mobile/android-components/issues/8312 that will likely fixed this and other download issues, it will be pretty helpful if the QA team can verify if this issue is reproduable on the GeckoView example app, as it's using the new GV download api.

[victor-bayas]

I'm integrating a new GV api on AC https://github.com/mozilla-mobile/android-components/issues/8312 that will likely fixed this and other download issues, it will be pretty helpful if the QA team can verify if this issue is reproduable on the GeckoView example app, as it's using the new GV download api.

Can you share the sample app to test?

[Amejia481]

You can download it from here

[victor-bayas]

You can download it from here

Can't download anything with it

[Amejia481]

I was able to download by clicking on the link public/build/geckoview_example.apk

[victor-bayas]

I was able to download by clicking on the link public/build/geckoview_example.apk

I'm speaking about being unable to download any files from the geckoview example browser

[Amejia481]

@owlishDeveloper can help us to identify how downloads are display on the GeckoView example app, as I'm unable to see a notification when I or dialog when starting a download.

[owlishDeveloper]

@Amejia481 GVE is a very simple app, it doesn't have any dialogs or notifications for downloads. When you download in it, you just wait and then you open the downloads folder and see if the file is there.

[Amejia481]

Thanks for the clarification. That will help the QA team to test :)

[abodea]

Re-tested on the latest Nightly build from 9/17 with Google Pixel 4 XL (Android 11).

| Website | Download | Open File from Fenix | Open File from FileManager |
| ------------- | :-------------: | :-------------: | :-------------: |
| Google Drive | ✔️ | ✔️ | ✔️ |
| Gmail GIF | ✔️ | ✔️ | ✔️ |
| Gmail -> WeTransfer link jpg , apk, pdf | ✔️ | ❓ | ✔️ |
| Udemy | ✔️ | ✔️ | ✔️ |
| Wizair | ✔️ | ✔️ | ✔️ |
| Github | ✔️ | ✔️ | ✔️ |
| Link from #11436 - pdf file| ✔️ | ❌ | ❌ |
| Link from #54064 - pdf file| ❌| N/A| N/A |

I do believe nothing really changed.
Removing the eng:qa:needed until further notice.

[liuche]

@kglazko seems like some pdf opening problems here too?

[Amejia481]

We recently merged this patch on AC, we got a new api from GV (special shout-out to @owlishDeveloper for the hard work), that will hopefully fix those issues :pray: It will be available in the next ac version.

[abodea]

Hello, @Amejia481
I re-tested on the latest Nightly for more details please check the following:
| Website | Download | Open File from Fenix | Open File from FileManager |
| ------------- | :-------------: | :-------------: | :-------------: |
| Google Drive | ✔️ | ✔️ | ✔️ |
| Gmail GIF | ✔️ | ✔️ | ✔️ |
| Gmail -> WeTransfer link jpg , apk, pdf | ✔️ | ❓ | ✔️ |
| Udemy | ✔️ | ✔️ | ✔️ |
| Wizair | ✔️ | ✔️ | ✔️ |
| Github | ✔️ | ✔️ | ✔️ |
| Link from #11436 - pdf file| ✔️ | ✔️ | ✔️ |
| Link from #54064 - pdf file| ✔️| ✔️| ✔️ |

The problem right now is still when trying to open any file downloaded from WeTransfer or any transfer link.
Maybe this is the only remaining issue when trying to open a file after it was downloaded from a transfer link? It's weird because I did other downloads and I had no issues when I tried to open the files.

Removing eng:qa:needed until further notice.

[victor-bayas]

@Amejia481 all working good on latest Nightly :D

Fenix used to fail to download PDF invoices from my country's companies house website but now it downloads and opens the invoice without problems.

[abodea]

Thank you @victor-bayas!
Depending on @Amejia481 input, it's possible to open a new issue or investigate in a new issue the transfer links.

[lnicola]

For me, the file name in the Content-Disposition header is not respected. The browser can't open the file after downloading it, the Files app shows the correct file with the wrong size (!), but in the file picker it shows as downloadfile.tar, where tar is the correct extension. Tested on Android 10.

[lnicola]

@Amejia481 the placeholder filename comes from https://github.com/mozilla-mobile/android-components/blob/449abd4010c159fa90dddac00d4db7cfa002aecd/components/support/utils/src/main/java/mozilla/components/support/utils/DownloadUtils.kt#L126. Here's a header that doesn't get parsed correctly: content-disposition: attachment; filename*=UTF-8''kedia2017mem%2Etar.

https://tools.ietf.org/html/rfc5987 has more details on that encoding, but I don't see why the code fails -- it looks like it should accept that specific value.

[Amejia481]

@Amejia481 the placeholder filename comes from https://github.com/mozilla-mobile/android-components/blob/449abd4010c159fa90dddac00d4db7cfa002aecd/components/support/utils/src/main/java/mozilla/components/support/utils/DownloadUtils.kt#L126. Here's a header that doesn't get parsed correctly: content-disposition: attachment; filename*=UTF-8''kedia2017mem%2Etar.

https://tools.ietf.org/html/rfc5987 has more details on that encoding, but I don't see why the code fails -- it looks like it should accept that specific value.

Thanks @lnicola for all the details, they are pretty helpful :), we have an opened issue for it https://github.com/mozilla-mobile/android-components/issues/8425

[Amejia481]

I think we can close this issue as it's fixed in nightly and we have opened issues for the remaining work, please feel free to reopen if needed!