Closed
Bug 908440
Opened 12 years ago
Closed 11 years ago
[Session Restore] Add string type checks for values passed to Global, Tab, and Window value storage API
Categories
(Firefox :: Session Restore, defect)
Firefox
Session Restore
Tracking
()
RESOLVED
FIXED
Firefox 30
People
(Reporter: smacleod, Assigned: smacleod)
References
Details
Attachments
(1 file, 1 obsolete file)
2.71 KB,
patch
|
Details | Diff | Splinter Review |
The get*Value(...) and set*Value(...) (For Global, Tab, and Window) methods on SessionStore should check their parameters to ensure only strings are passed as values. These methods aren't protected by the idl type checks and consumers should not be able to pass in objects and maintain references to them.
Assignee | ||
Updated•12 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•11 years ago
|
||
Attachment #8375790 -
Flags: review?(ttaubert)
Updated•11 years ago
|
Attachment #8375790 -
Flags: review?(ttaubert) → review+
Assignee | ||
Comment 3•11 years ago
|
||
Attachment #8375790 -
Attachment is obsolete: true
Assignee | ||
Updated•11 years ago
|
Keywords: checkin-needed
Comment 4•11 years ago
|
||
Keywords: checkin-needed
Comment 5•11 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 30
Updated•11 years ago
|
QA Whiteboard: [qa-]
Comment 6•11 years ago
|
||
The MDN docs clearly state that "You may use any JavaScript object as the data." (https://developer.mozilla.org/en/docs/Session_store_API)
Hence, this patch causes unhandled exceptions in my add-on (KeeFox). Perhaps others are affected too (I doubt it's practical to automatically detect the scale of the issue).
Given the current focus on FF29/Australis by add-on authors, it would be good if this patch could be:
1) modified to output a warning instead of throwing an exception
2) delayed for a version or two
3) more widely announced to add-on authors, along with appropriate documentation modifications
In my case, I am actually setting a string most of the time but occasionally set "undefined". It's trivial for me to change this to an empty string but maybe the change for others could be more difficult.
Alternatively, if the API has never correctly stored and returned non-strings, could the set functions just check for undefined? (and null?)
I don't know if undefined and null count as references from the perspective of this API module. If they don't, presumably they could be permitted to be set as per current behaviour without any impact on GC?
You need to log in
before you can comment on or make changes to this bug.
Description
•