Closed Bug 1121722 Opened 7 years ago Closed 7 years ago
Chrome-only DOM File constructors don't use the file's modification time
The forms of File::CreateFromFile that take an nsIFile or path will use the actual file's last-modified time for the created DOM File, as one might expect. (This is what happens when chrome script sets the |value| property of a file input element to a non-empty string currently, but this has e10s/sandboxing problems; see bug 1068838.) However, CreateFromFile isn't exposed to script directly, and the chrome-exposed File constructors add an extra layer of MultipartFileImpl in which the mtime gets lost and replaced with the current time. In principle, the caller could use the |lastModified| option to the File constructor to work around this, but that's not implemented yet (bug 939509).
Investigating this issue a bit more, it seems that it's not a big problem to support lastModified attribute in the File ChromeOnly constructors. We cannot take the modification time of the nsIFile for security reasons: bug 403852. Maybe this is not valid anymore... but I don't want to break existing tests.
Attachment #8561522 - Flags: review?(bzbarsky)
I can add a mochitest, maybe :)
Comment on attachment 8561522 [details] [diff] [review] date.patch > We cannot take the modification time of the nsIFile for security reasons: > bug 403852 That bug has no security anything involved. What's the security issue? This patch is not removing the todo() in dom/base/test/test_bug403852.html that references this bug number. Why not?
Attachment #8561522 - Flags: review?(bzbarsky) → review-
Sorry bz, you are totally right. Here a new patch, with a mochitest.
Comment on attachment 8562107 [details] [diff] [review] date.patch >+ uint64_t subLastModified = blob->GetLastModified(error); Maybe call it "partLastModified"? >+ mLastModificationDate = >+ lastModified ? lastModified * PR_USEC_PER_MSEC : JS_Now(); Isn't 0 a fairly valid value? Just means a 40-year-old file.... Speaking of which, shouldn't our last-modified things be signed, so they can represent times before the epoch? Followup ok here. r=me
Attachment #8562107 - Flags: review?(bzbarsky) → review+
Assignee: nobody → amarchesini
You need to log in before you can comment on or make changes to this bug.