Closed Bug 794386 Opened 7 years ago Closed 7 years ago

do not store blob: URIs in global history

Categories

(Toolkit :: Places, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla19

People

(Reporter: freddyb, Assigned: baku)

Details

Attachments

(1 file, 1 obsolete file)

blob URIs are very volatile and supposed to be
a) bound to an origin, i.e. only accessible for the origin they were created in.
b) revoked after the document has been unloaded.

I can open blob URIs in other tabs, which creates a history entry. What use is it, to have this history entry?

My suggestion: don't store them in the first place (or remove them on revocation)


I guess this depends on bug 773132, implementing automatic revocation.
Bug 773132 is not related at all.
No longer depends on: 773132
Are you talking about global history here, or session history?
Sorry, I'm not sure about the distinction between global and session history. I'll try to clarify my bug description.

Steps to reproduce:
1) b = new Blob(['foo']); u = window.URL.createObjectURL(b);
2) Then I open the blob URL in a new window (be it with window.open or manually)
3) Restart Firefox
4) Notice that the tabs with the blob URL contain blank pages (blob URIs invalidated?)
5) Close the tabs
6) Open the history (e.g. CTRL+H)
7) Search for "blob:"
8) blob URIs are in the history, but point to something that the browser *knows* is not valid anymore


I hope this doesn't add any confusion.
That's global history.  

Gavin, do you recall whether docshell is supposed to filter URLs for that, or does Places do it?
Places does most of the filtering, see nsNavHistory::CanAddURI. I think you should just add blob: to its scheme blacklist.
Component: DOM → Places
Product: Core → Toolkit
Summary: do not store blob: URIs in history → do not store blob: URIs in global history
Attached patch patch 1 (obsolete) — Splinter Review
Is this enough?
Attachment #669105 - Flags: review?(bzbarsky)
Assignee: nobody → amarchesini
Comment on attachment 669105 [details] [diff] [review]
patch 1

Yes, thanks.
Attachment #669105 - Flags: review?(bzbarsky) → review+
Attached patch patch 1Splinter Review
added r=gavin.sharp
Attachment #669105 - Attachment is obsolete: true
Attachment #669153 - Flags: review+
Keywords: checkin-needed
Flags: in-testsuite+
We're going to need to add 'mediastream' and 'mediasource' to this list. Should we make it a whitelist instead of a blacklist?
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #10)
> Should we make it a whitelist instead of a blacklist?

Yes, I suggested this long time ago, I also think would be better.
FWIW, I agree :)
I agree too, file a bug.
https://hg.mozilla.org/mozilla-central/rev/8db471894559
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
You need to log in before you can comment on or make changes to this bug.