Closed Bug 303108 Opened 19 years ago Closed 19 years ago

Components.utils.evalInSandbox should throw good exceptions

Categories

(Core :: XPConnect, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: axel, Assigned: mrbkap)

References

Details

(Keywords: fixed1.8)

Attachments

(1 file, 2 obsolete files)

Right now, evalInSandbox just throws an NS_ERROR_FAILURE on errors, it should throw good exceptions. IMHO, using the source position of the caller is the right approach here, instead of the principal uri. I have a patch that does just that. I haven't gotten JSMSG_UNCAUGHT_EXCEPTION to work. That maps to JSEXN_NONE, and I can't tell if it will be caught or not in the eval'd statement. Components.utils.evalInSandbox("foo=Components.classes","about:blank") vs. Components.utils.evalInSandbox("try{foo=Components.classes}catch(e){}","about:blank") This may just be bug 66453, but I hope we can work around it.
Comment on attachment 191355 [details] [diff] [review] use caller source info, call SetExceptionWasThrown on error putting this on shaver's radar for feedback.
Attachment #191355 - Flags: review?(shaver)
Comment on attachment 191355 [details] [diff] [review] use caller source info, call SetExceptionWasThrown on error mrbkap: does this fit with your pending patch? Y'all should work together, team-like and stuff.
Attachment #191355 - Flags: review?(shaver) → review?(mrbkap)
Comment on attachment 191355 [details] [diff] [review] use caller source info, call SetExceptionWasThrown on error >Index: xpccomponents.cpp + if (frame) { + frame->GetFilename(getter_Copies(filename)); + frame->GetLineNumber(&lineno); + } If we can't get the stack frame, do we want to fall back to what we did before (using the codebase as the filename and 1 as the line number)?
Yes, I think we do.
We should get the evalInSandbox stuff locked down for 1.5. I'll take this and update the patch to my own review comment sometime today.
Assignee: dbradley → mrbkap
Flags: blocking1.8b4?
Attachment #191355 - Attachment is obsolete: true
Attachment #191355 - Flags: review?(mrbkap)
Flags: blocking1.8b4? → blocking1.8b4+
Attached patch handle uncaught exceptions too (obsolete) — Splinter Review
Uncaught exceptions were being a pain since they were trying to be reported as errors, but were unable to roundtrip back to being exceptions. With this patch, we push a fake stack frame onto the sandcx so that we don't try to report uncaught exceptions as errors.
Attachment #193984 - Flags: review?(shaver)
I moved the SetExceptionWasThrown call into the case where we actually set the pending exception since I think it belongs there.
Attachment #193984 - Attachment is obsolete: true
Attachment #193986 - Flags: review?(shaver)
Attachment #193984 - Flags: review?(shaver)
Comment on attachment 193986 [details] [diff] [review] one that actually compiles r=shaver. We'll fix this API yet!
Attachment #193986 - Flags: review?(shaver) → review+
Comment on attachment 193986 [details] [diff] [review] one that actually compiles We probably want this on the branch as well.
Attachment #193986 - Flags: approval1.8b4?
Attachment #193986 - Flags: approval1.8b4? → approval1.8b4+
Fix check into MOZILLA_1_8_BRANCH and trunk.
Status: NEW → RESOLVED
Closed: 19 years ago
Keywords: fixed1.8
Resolution: --- → FIXED
Depends on: 306382
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: