[Session Restore] Add string type checks for values passed to Global, Tab, and Window value storage API

RESOLVED FIXED in Firefox 30

Status

()

defect
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: smacleod, Assigned: smacleod)

Tracking

Trunk
Firefox 30
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

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.
Status: NEW → ASSIGNED
Duplicate of this bug: 961646
Attachment #8375790 - Flags: review?(ttaubert)
Attachment #8375790 - Flags: review?(ttaubert) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/aacb9513c0b5
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 30
Depends on: 961646
QA Whiteboard: [qa-]
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?
Depends on: 996053
You need to log in before you can comment on or make changes to this bug.