Closed Bug 306382 Opened 19 years ago Closed 19 years ago

Components.utils.evalInSandbox can fail silently

Categories

(Core :: XPConnect, defect)

x86
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: mrbkap, Assigned: mrbkap)

References

Details

(Keywords: fixed1.8, regression, Whiteboard: [needs review shaver])

Attachments

(1 file, 1 obsolete file)

When the fix for bug 303108 went in, I made the bad assumption that
JS_EvaluateUCScriptForPrincipals would always have a pending exception. This
isn't the case for OOM, where the function returns false, but doesn't set a
pending exception. In this case, evalInSandbox should still signal the error,
and not simply return NS_OK.
Attached patch catch OOM (obsolete) — Splinter Review
Brendan tells me that OOM is the only time when this happens, so I went with
the more specific return value.
Attachment #194248 - Flags: review?(shaver)
Comment on attachment 194248 [details] [diff] [review]
catch OOM

This is the wrong diff.
Attachment #194248 - Attachment is obsolete: true
Attachment #194248 - Flags: review?(shaver)
Attached patch correct diffSplinter Review
Attachment #194250 - Flags: review?(shaver)
This should probably block 1.8b4 since it's a new API and all that. This is also
an ultra-simple fix.
Status: NEW → ASSIGNED
Flags: blocking1.8b4?
Attachment #194250 - Flags: superreview?(brendan)
Comment on attachment 194250 [details] [diff] [review]
correct diff

Yeah, we need to fix this last detail.

/be
Attachment #194250 - Flags: superreview?(brendan)
Attachment #194250 - Flags: superreview+
Attachment #194250 - Flags: approval1.8b4+
Flags: blocking1.8b4? → blocking1.8b4+
Whiteboard: [needs review shaver]
Comment on attachment 194250 [details] [diff] [review]
correct diff

r=shaver
Attachment #194250 - Flags: review?(shaver) → review+
Fix checked into MOZILLA_1_8_BRANCH and trunk.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Keywords: fixed1.8
Resolution: --- → FIXED
There's a report about PAC being broken in the builds after this patch landed. 
Could this have regressed PAC?  Please see bug 306467.  Thanks!
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: