This bug was filed from the Socorro interface and is report bp-d2018f19-aba7-4164-bcac-365032160822. ============================================================= Steps to reproduce: 1. Click on "Attach file" in this bug. 2. Open Finder, pick a file, and drag it over the "Browse..." button. Crash.

Hi Michael, is this related to bug 1296041? Any more ideas? Thanks!

A bit different STR: - win10 - open 'add an attachment' page in bugzilla - drop a file on the [ Browse ] button (in my case a txt file) I believe it started with https://hg.mozilla.org/mozilla-central/rev/194fe275b4e60ded2af6b25173eec421f0dba8ad

:mstange contacted me about this tomorrow, but I wasn't able to work on it as I was out of the office. I've got a pretty good idea of what's going on, so it shouldn't be too bad to fix.

Pushlog: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=7d3bc3efebe0730a8f8d560ca5c07eb2501d74ad&tochange=96766f088fd33489b6b8011cc685eb09fabb0d19 Also reproducible on Windows.

Comment on attachment 8784091 [details] Bug 1297186 - Correctly propagate principals when getting the files list in DataTransfer, Adding Neil as a reviewer to see if we can speed up getting this patch landed.

Comment on attachment 8784091 [details] Bug 1297186 - Correctly propagate principals when getting the files list in DataTransfer, https://reviewboard.mozilla.org/r/73654/#review72848 ::: dom/events/DataTransferItemList.cpp:208 (Diff revision 2) > -FileList* > -DataTransferItemList::Files() > +already_AddRefed<FileList> > +DataTransferItemList::Files(nsIPrincipal* aPrincipal) > { > + RefPtr<FileList> files; > + if (nsContentUtils::IsSystemPrincipal(aPrincipal)) { > + NS_WARNING("Accessing files from System Principal"); Why is there a warning here? ::: dom/events/DataTransferItemList.cpp:220 (Diff revision 2) > mFiles = new FileList(static_cast<nsIDOMDataTransfer*>(mDataTransfer)); > + mFilesPrincipal = aPrincipal; > RegenerateFiles(); > } > > - return mFiles; > + if (!aPrincipal->Subsumes(mFilesPrincipal)) { This is a strange setup and needs more comments in the code. Does it work if a chrome caller accesses dataTransfer.files then a caller on the content page later does so? ::: dom/events/DataTransferItemList.cpp:302 (Diff revision 2) > nsIPrincipal* aPrincipal, > bool aInsertOnly, > bool aHidden, > ErrorResult& aRv) > { > + if (!nsContentUtils::IsSystemPrincipal(aPrincipal)) { This is called during FillInExternalData where these checks should not be performed.

(In reply to Neil Deakin from comment #8) > Comment on attachment 8784091 [details] > Bug 1297186 - Correctly propagate principals when getting the files list in > DataTransfer, > > https://reviewboard.mozilla.org/r/73654/#review72848 > > ::: dom/events/DataTransferItemList.cpp:208 > (Diff revision 2) > > -FileList* > > -DataTransferItemList::Files() > > +already_AddRefed<FileList> > > +DataTransferItemList::Files(nsIPrincipal* aPrincipal) > > { > > + RefPtr<FileList> files; > > + if (nsContentUtils::IsSystemPrincipal(aPrincipal)) { > > + NS_WARNING("Accessing files from System Principal"); > > Why is there a warning here? Accidental leftover from debugging. Removed. > ::: dom/events/DataTransferItemList.cpp:220 > (Diff revision 2) > > mFiles = new FileList(static_cast<nsIDOMDataTransfer*>(mDataTransfer)); > > + mFilesPrincipal = aPrincipal; > > RegenerateFiles(); > > } > > > > - return mFiles; > > + if (!aPrincipal->Subsumes(mFilesPrincipal)) { > > This is a strange setup and needs more comments in the code. > > Does it work if a chrome caller accesses dataTransfer.files then a caller on > the content page later does so? Yes, I added some comments. > ::: dom/events/DataTransferItemList.cpp:302 > (Diff revision 2) > > nsIPrincipal* aPrincipal, > > bool aInsertOnly, > > bool aHidden, > > ErrorResult& aRv) > > { > > + if (!nsContentUtils::IsSystemPrincipal(aPrincipal)) { > > This is called during FillInExternalData where these checks should not be > performed. I don't think it is. FillInExternalData is defined on the DataTransferItem now, and as far as I know never calls into DataTransferItemList::SetDataWithPrincipal anymore. If you're talking about CacheExternalData (which adds the placeholder DataTransferItem instances), that method is always called with the system principal as aPrincipal, so it shouldn't be an issue as far as I know.

The patch seems ok, but when I drag an image to the desktop on Mac I get a file shortcut instead of the real image. The filename is something like file-///directory/happy.png.fileloc rather than just the filename. The data appears to be missing the file promise type (as well as application/x-moz-nativeimage)

This patch moves the check out of SetDataWithPrincipal, and instead explicitly checks at the call sites which need to check.

Pushed by michael@thelayzells.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/04010fbe1f10 Correctly propagate principals when getting the files list in DataTransfer, r=enndeakin