1121722 - Chrome-only DOM File constructors don't use the file's modification time

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

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).

Comment 1

[Andrea Marchesini [:baku]]

Attachment: date.patch

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.

Comment 2

[Andrea Marchesini [:baku]]

I can add a mochitest, maybe :)

Comment 3

[Boris Zbarsky [:bzbarsky]]

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?

Comment 4

[Andrea Marchesini [:baku]]

Attachment: date.patch

Sorry bz, you are totally right. Here a new patch, with a mochitest.

Comment 5

[Boris Zbarsky [:bzbarsky]]

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

Comment 6

[Andrea Marchesini [:baku]]

Attachment: date.patch

Comment 7

[Andrea Marchesini [:baku]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ed6136ac7ec6

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f0e720196653

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/f0e720196653