Closed
Bug 751858
Opened 12 years ago
Closed 12 years ago
location's PUNCTURE policy throws without setting an exception
Categories
(Core :: XPConnect, defect)
Tracking
()
RESOLVED
FIXED
mozilla16
People
(Reporter: mrbkap, Assigned: mrbkap)
References
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
bholley
:
review+
mrbkap
:
checkin+
|
Details | Diff | Splinter Review |
I don't have a testcase right now, but I ran into this working on another bug. Basically, Policy::check functions are responsible for setting their own exceptions if they deny access.
Assignee | ||
Comment 1•12 years ago
|
||
I think the change to AccessCheck.cpp makes the control flow a little clearer (and it nukes a couple of braces). It's a little unfortunate that this patch actually makes LocationPolicy::check less clear (since the exception for PUNCTURE is "hidden" in a larger boolean expression) but it's the easiest way to share the code that throws the exception.
Attachment #621016 -
Flags: review?(bobbyholley+bmo)
Comment 2•12 years ago
|
||
Comment on attachment 621016 [details] [diff] [review]
Proposed fix
>--- a/js/xpconnect/wrappers/AccessCheck.cpp
>+++ b/js/xpconnect/wrappers/AccessCheck.cpp
>@@ -494,12 +494,10 @@ ExposedPropertiesOnly::check(JSContext *cx, JSObject *wrapper, jsid id, Wrapper:
> perm = PermitObjectAccess;
> return true;
> }
>- if (act == Wrapper::PUNCTURE) {
>- perm = DenyAccess;
>- return false;
>- }
>
> perm = DenyAccess;
>+ if (act == Wrapper::PUNCTURE)
>+ return false;
Doesn't this have the same issue?
Assignee | ||
Comment 3•12 years ago
|
||
Oops, yes.
Attachment #621016 -
Attachment is obsolete: true
Attachment #621016 -
Flags: review?(bobbyholley+bmo)
Attachment #621024 -
Flags: review?(bobbyholley+bmo)
Comment 4•12 years ago
|
||
Comment on attachment 621024 [details] [diff] [review]
Proposed fix v2
Can we add some tests for this? I'm about to touch this code in a couple of ways (fixing __exposedProps__, and removing universalXPConnect), and this seems like the sort of thing I might regress.
Attachment #621024 -
Flags: review?(bobbyholley+bmo) → review+
So ... can we land this?
Assignee | ||
Comment 7•12 years ago
|
||
Comment on attachment 621024 [details] [diff] [review]
Proposed fix v2
https://hg.mozilla.org/integration/mozilla-inbound/rev/8d752d1b621b
Attachment #621024 -
Flags: checkin+
Assignee | ||
Updated•12 years ago
|
Keywords: checkin-needed
Updated•12 years ago
|
Target Milestone: --- → mozilla16
Comment 8•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•