479627 - Allow SessionStart to work if true JSON strings are returned by _readStateFile() call

Status: RESOLVED FIXED

(Firefox :: Session Restore, defect)

Description

[Michael Kraft [:morac]]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b2) Gecko/20081201 Firefox/3.1b2 (.NET CLR 3.5.30729)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090221 Minefield/3.2a1pre (.NET CLR 3.5.30729)

As extensions can modify the value returned by the _readStateFile function in the SessionStart compoment, it would be a good idea to wrap the returned result in parenthesis before trying to call the evalInSandbox function.  Otherwise if an extension returns a true JSON string (one created by JSON.stringify and not wrapped in parenthesis) the evalinSandbox call with throw an exception.

Since the SessionStore component also reads the _iniString variable from SessionStart, either the _iniString variable itself should contain the parenthesis wrapped string or the evalinSandbox call in SessionStart and the _safeeval() call in SessionStore's init function should explicitly wrap it.


The main reason I ask for this is to keep things consistent: 

1.  Firefox's SessionStore component will wrap API function parameters before attempting to _safeeval() them so I think it SessionStart should as well.  

2. Making this change would keep consistency between Firefox and SeaMonkey 2.0a3.  Both Firefox and SeaMonkey allow extensions to modify the initial state by observing the "sessionstore-state-read" notification, but as SeaMonkey does a JSON.parse and Firefox does an evalInSandBox, an extension would need to return parenthesis wrapped JSON strings to Firefox and true JSON strings to SeaMonkey to prevent exceptions.  This requires adding extra browser detection code to extensions which would otherwise not be needed.


Reproducible: Always

Comment 1

[Simon Bünzli]

Attachment: fix

Dietrich: We'll want this two-liner on the 1.9.1 branch for future proofing our code (should we ever want to completely switch to JSON) and also for consistency with SeaMonkey (which has already made that switch). The patch is save, as adding parentheses when there's already a pair is a no-op for eval.

Comment 2

[Dietrich Ayala (:dietrich)]

Comment on attachment 363500 [details] [diff] [review]
fix

r=me

Comment 3

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/0bc0ee3c2964

Comment 4

[Michael Kraft [:morac]]

Am I right in assuming this won't make it into Firefox 3.1 since only the trunk got the fix?

Comment 5

[Simon Bünzli]

(In reply to comment #4)
The patch will land on the Firefox 3.1 branch as soon as we've got approval for doing so by the release drivers (note the approval1.9.1? flag).

And in case you're wondering: The Target Milestone refers to the next _Trunk_ milestone to contain the fix, branch milestones are tracked through keywords (fixed1.9.1, verified1.9.1, etc.).

Comment 6

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 363500 [details] [diff] [review]
fix

a191=beltzner

Comment 7

[Dão Gottwald [:dao]]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/552ed1cc4493