Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

[Bug] Crash when downloading file with a period in the name #16443

Closed
mobd opened this issue Nov 7, 2020 · 15 comments
Closed

[Bug] Crash when downloading file with a period in the name #16443

mobd opened this issue Nov 7, 2020 · 15 comments
Assignees
Labels
b:crash Crashes Fenix: should link to Sentry, Crash-Stats or GPlay info 🐞 bug Crashes, Something isn't working, .. eng:qa:verified QA Verified Feature:Download S3 Blocks non-critical functionality and a work around exists

Comments

@mobd
Copy link

mobd commented Nov 7, 2020

Steps to reproduce

Download a file that has two consecutive periods as part of the filename

Expected behavior

The browser doesn't crash.

Actual behavior

The file is downloaded, but the browser crashes. Stacktrace:

d41c7934-4f47-436c-bf2e-eb29011bedce
java.lang.IllegalArgumentException: Invalid file path: /storage/emulated/0/Download/o..txt

java.lang.IllegalArgumentException: Invalid file path: /storage/emulated/0/Download/o..txt
at android.database.DatabaseUtils.readExceptionFromParcel(DatabaseUtils.java:170)
at android.database.DatabaseUtils.readExceptionFromParcel(DatabaseUtils.java:140)
at android.content.ContentProviderProxy.insert(ContentProviderNative.java:481)
at android.content.ContentResolver.insert(ContentResolver.java:1844)
at android.app.DownloadManager.addCompletedDownload(DownloadManager.java:1530)
at android.app.DownloadManager.addCompletedDownload(DownloadManager.java:1446)
at mozilla.components.feature.downloads.AbstractFetchDownloadService.updateDownloadNotification$feature_downloads_release(AbstractFetchDownloadService.kt:21)
at mozilla.components.feature.downloads.AbstractFetchDownloadService.access$updateDownloadNotification(AbstractFetchDownloadService.kt:5)
at mozilla.components.feature.downloads.AbstractFetchDownloadService$onStartCommand$2.invokeSuspend(AbstractFetchDownloadService.kt:7)
at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:3)
at kotlinx.coroutines.AwaitKt.resume(Await.kt:7)
at kotlinx.coroutines.CancellableContinuationImpl.dispatchResume(CancellableContinuationImpl.kt:18)
at kotlinx.coroutines.CancellableContinuationImpl.resumeImpl(CancellableContinuationImpl.kt:5)
at kotlinx.coroutines.CancellableContinuationImpl.resumeUndispatched(CancellableContinuationImpl.kt:2)
at kotlinx.coroutines.android.HandlerContext$scheduleResumeAfterDelay$$inlined$Runnable$1.run(Runnable.kt:1)
at android.os.Handler.handleCallback(Handler.java:900)
at android.os.Handler.dispatchMessage(Handler.java:103)
at android.os.Looper.loop(Looper.java:219)
at android.app.ActivityThread.main(ActivityThread.java:8347)
at java.lang.reflect.Method.invoke(Native Method)
at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:513)
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1055)

Device information

  • Android device: Huawei Nova 6 (Android 10)
  • Fenix version: Nightly 201106 17:01 (Build #2015774091)

┆Issue is synchronized with this Jira Task

@mobd mobd added the 🐞 bug Crashes, Something isn't working, .. label Nov 7, 2020
@github-actions github-actions bot added the needs:triage Issue needs triage label Nov 7, 2020
@mobd
Copy link
Author

mobd commented Nov 7, 2020

You can try it with this link https://send.vis.ee/download/9c75816f98eb53d0/#O2E5KMqIiE7NOdHOH0JIOQ, or if its expired upload any text file with a period to test it

@Amejia481 Amejia481 added Feature:Download and removed needs:triage Issue needs triage labels Nov 9, 2020
@Amejia481 Amejia481 self-assigned this Nov 9, 2020
@Amejia481
Copy link
Contributor

@mobd thanks for reporting, could you go to Settings -> About Firefox Nightly -> Crashes and share any entries there?

I wasn't able to replicate in nightly using pixel 3 with Android 11, possible a devices specific bug. QA team please help us to confirm.

Nightly 201108 17:01 (Build #2015774475)
AC: 66.0.20201106143126, cdc2400a0
GV: 84.0a1-20201106093443
AS: 63.0.0

@mobd
Copy link
Author

mobd commented Nov 9, 2020

@Amejia481 hi, I already posted the relevant stacktrace above. Or is there something else that would help?

@Amejia481
Copy link
Contributor

Sorry @mobd my bad, got mixed up with another bug, the stack trace above will be sufficient.

@kbrosnan kbrosnan added the b:crash Crashes Fenix: should link to Sentry, Crash-Stats or GPlay info label Nov 12, 2020
@Amejia481 Amejia481 added the eng:qa:needed QA Needed label Nov 12, 2020
@ebalazs-sv
Copy link

I had tested this issue on the latest Nightly 201117 05:03 (Build #2015776107) GV 84 from 11/17 with the following devices:

  • Huawei P9 Lite (Android 7);
  • Motorola Moto G6 (Android 8);
  • Google Pixel 2 (Android 9);

and I was not able to reproduce the crash, when downloading files from https://send.vis.ee/.

@mobd Hi! Is this issue still reproducible on the latest Nightly? What type of file you where downloading when the app crashed?
Could you please provide a video with all the steps? Thanks!

I will remove the qa:needed label for now.

@ebalazs-sv ebalazs-sv removed the eng:qa:needed QA Needed label Nov 17, 2020
@Amejia481
Copy link
Contributor

@ebalazs-sv thanks :)
As the issue is not reproducible anymore, I will proceed to close, please feel free to reopen if needed.

@mobd
Copy link
Author

mobd commented Nov 17, 2020

@Amejia481 hi, please reopen this, I don't think I have the permissions to do so. I can still reproduce with the latest nightly #2015776011.

@ebalazs-sv I did some more testing and it seems that it only happens when there are two consecutive periods anywhere in the filename, I'll edit the main issue too.

I can consistently reproduce this by creating a dummy text file with some content and downloading it from send.vis.ee. I've tried the following filenames and they all crash the browser.

..txt
1..txt
1..1.txt

It also happens with videos that have consecutive periods in the name so I don't think the filetype or website matters.

Video:
output_0

And sample file https://send.vis.ee/download/adddda3bf01aba79/#gatmoe2sQn8Qh5nBcFAdRg

Thank you both for looking at this!

@ebalazs-sv
Copy link

Thank you for the details!
I had tested this on the latest Nightly 201118 05:01 (Build #2015776297) GV 85 from 11/18 with Huawei P9 Lite (Android 7) and
Motorola Moto G6 (Android 8), with the sample file from comment #16443 (comment) and I was not able to reproduce the behavior.
20201118_091709

@Amejia481
Copy link
Contributor

Thanks @mobd, I was able to reproduce with an emulator with Android 10 looks like this is something specific about this version. I will take a look.

@Amejia481 Amejia481 reopened this Nov 18, 2020
@Amejia481
Copy link
Contributor

Thanks @ebalazs-sv for quickly retesting :)

@mobd
Copy link
Author

mobd commented Nov 18, 2020

@ebalazs-sv Actually I forgot to mention something 😅

If there already is a file with the same name in downloads, so that the downloaded file has a name like 1..(1).txt or whatever, I don't get a crash either. It's only if the name of the downloaded file is the same as the original that it happens.

Sorry for the bad explanation!

@Amejia481 Amejia481 added S3 Blocks non-critical functionality and a work around exists eng:qa:needed QA Needed labels Dec 2, 2020
@Amejia481
Copy link
Contributor

This is ready for QA review, @mobd could you confirm if the issue is fixed for you? :)

@mobd
Copy link
Author

mobd commented Dec 10, 2020

Hi @Amejia481, I can confirm the browser no longer crashes in Nightly 201208 17:06 (Build #2015780235). The sanitizing of the filename when uploading or downloading does seem like unexpected behaviour to me, chrome doesn't attempt to do this. But not having the browser crash is the most important part, thanks for fixing this!

@sflorean
Copy link
Contributor

Verified as fixed on Nightly 10/9 with Nokia 6(Android 7.1.1) and Pixel 3(Android 11).

@sflorean sflorean added eng:qa:verified QA Verified and removed eng:qa:needed QA Needed labels Dec 10, 2020
@Amejia481
Copy link
Contributor

Thanks @sflorean!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
b:crash Crashes Fenix: should link to Sentry, Crash-Stats or GPlay info 🐞 bug Crashes, Something isn't working, .. eng:qa:verified QA Verified Feature:Download S3 Blocks non-critical functionality and a work around exists
Projects
None yet
Development

No branches or pull requests

5 participants