DownloadContentService: canWrite() and hasEnoughDiskSpace() can erroneously return false

RESOLVED FIXED in Firefox 46

Status

()

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: sebastian, Assigned: sebastian)

Tracking

unspecified
Firefox 46
All
Android
Points:
---

Firefox Tracking Flags

(firefox46 fixed)

Details

Attachments

(2 attachments)

(Assignee)

Description

3 years ago
On Android 6 with targetSdkVersion=23 and without storage permission canWrite() and hasEnoughDiskSpace() return false (temporaryFile and destinationFile).

Both files are stored in the application specific temporary and data directories. Those directories are writable without a permission. Nevertheless File.canWrite() and getUsableSpace() return values as if we do not have the needed permissions.
(Assignee)

Comment 1

3 years ago
This seems to be actually independent from the permission:
* File.canWrite() returns false if the file does not exist (even though we /could/ write to this destination)
* hasEnoughDiskSpace() returns 0 for a directory that does not exist yet
No longer blocks: 1216537
Summary: DLC: Without storage permission canWrite() and hasEnoughDiskSpace() returns false → DownloadContentService: canWrite() and hasEnoughDiskSpace() can erroneously return false
(Assignee)

Updated

3 years ago
Assignee: nobody → s.kaspari
Status: NEW → ASSIGNED
(Assignee)

Updated

3 years ago
Duplicate of this bug: 1236907
(Assignee)

Comment 3

3 years ago
Created attachment 8706493 [details]
MozReview Request: Bug 1237576 - DownloadAction.getDestinationFile(): Create directory if needed. r?rnewman

Review commit: https://reviewboard.mozilla.org/r/30355/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/30355/
Attachment #8706493 - Flags: review?(rnewman)
(Assignee)

Comment 4

3 years ago
Created attachment 8706495 [details]
MozReview Request: Bug 1237576 - DownloadAction: Remove canWrite() check. r?rnewman

We can't use File.canWrite() on files that do not exist yet. After all we are
going to create the file in a folder that we just created. So it is very
unlikely that writing to that folder is going to fail.

Review commit: https://reviewboard.mozilla.org/r/30357/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/30357/
Attachment #8706495 - Flags: review?(rnewman)
(Assignee)

Comment 5

3 years ago
> * hasEnoughDiskSpace() returns 0 for a directory that does not exist yet

If needed the directory is now created.

> * File.canWrite() returns false if the file does not exist (even though we
> /could/ write to this destination)

I removed this from the code: The only safe way to tell if the file is writable is to create it. But I do not want to create empty files or create and delete them. And as we are now creating the folder first: It's unlikely that we can't create a file in the folder we just created.
Comment on attachment 8706493 [details]
MozReview Request: Bug 1237576 - DownloadAction.getDestinationFile(): Create directory if needed. r?rnewman

https://reviewboard.mozilla.org/r/30355/#review27081
Attachment #8706493 - Flags: review?(rnewman) → review+
Comment on attachment 8706495 [details]
MozReview Request: Bug 1237576 - DownloadAction: Remove canWrite() check. r?rnewman

https://reviewboard.mozilla.org/r/30357/#review27079
Attachment #8706495 - Flags: review?(rnewman) → review+
(Assignee)

Comment 8

3 years ago
https://hg.mozilla.org/integration/fx-team/rev/c49219ebe24741df0615bac9bb7d368793b26b0a
Bug 1237576 - DownloadAction.getDestinationFile(): Create directory if needed. r=rnewman

https://hg.mozilla.org/integration/fx-team/rev/35c1ed58949d991030af52b8af23c879c8ccb364
Bug 1237576 - DownloadAction: Remove canWrite() check. r=rnewman

Comment 9

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/c49219ebe247
https://hg.mozilla.org/mozilla-central/rev/35c1ed58949d
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox46: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 46
You need to log in before you can comment on or make changes to this bug.