Closed Bug 379140 Opened 14 years ago Closed 14 years ago

FUEL 0.1: Application global contructor is accessible to content

Categories

(Firefox :: General, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 3 alpha7

People

(Reporter: Gavin, Assigned: mfinkle)

References

Details

(Whiteboard: [sg:want])

Attachments

(2 files, 1 obsolete file)

Something else I forgot to raise in my review comments: We still haven't found a solution to the problem of Application being accessible to content. I believe this shipped in a4 for the Mac, but not Windows and Linux due to bug 379139. I think we need to fix this for a5.
Flags: blocking-firefox3?
Attached patch add privileged properties (obsolete) — Splinter Review
The easiest way is to just add a new category for global properties that are chrome only. This patch adds a new category type which adds properties that can only be accessed from chrome callers. I think that's probably a better approach than any privileged caller. Not exactly sure if this is right security wise, but an attempt to access 'Application' from chrome works, whereas an attempt from elsewhere just throws 'Application is not defined'. Thoughts?
Assignee: nobody → enndeakin
Status: NEW → ASSIGNED
I think this is a very nice way to limit access to global properties. It certainly would work for FUEL.

Who are good reviewers?
(In reply to comment #2)
> I think this is a very nice way to limit access to global properties. It
> certainly would work for FUEL.
> 
> Who are good reviewers?
> 

jst, mrbkap, sicking
The patch doesn't define JAVASCRIPT_GLOBAL_PRIVILEGED_PROPERTY_CATEGORY, unless I'm missing something.
Attached patch add missing fileSplinter Review
Attachment #264503 - Attachment is obsolete: true
Attachment #264628 - Flags: superreview?(jst)
Attachment #264628 - Flags: review?(jst)
Comment on attachment 264628 [details] [diff] [review]
add missing file

r+sr=jst
Attachment #264628 - Flags: superreview?(jst)
Attachment #264628 - Flags: superreview+
Attachment #264628 - Flags: review?(jst)
Attachment #264628 - Flags: review+
Comment on attachment 264628 [details] [diff] [review]
add missing file

Mark, is this change still what is desired?
Attachment #264628 - Flags: review?(mark.finkle)
Comment on attachment 264628 [details] [diff] [review]
add missing file

let's wait until after 1.9a5 to commit this. I need to change the FUEL tests to chrome. This patch will break the current content based tests.
Attachment #264628 - Flags: review?(mark.finkle) → review+
Flags: blocking-firefox3? → blocking-firefox3+
Whiteboard: [sg:want]
moving to a6 per Mark's comment
Target Milestone: Firefox 3 alpha5 → Firefox 3 alpha6
Reassigning to Mark for the tests as mentioned above.
Assignee: enndeakin → mark.finkle
Status: ASSIGNED → NEW
Blocks: 380813
Actually, I'll just check this in without the fuelApplication.js change.
Status: NEW → ASSIGNED
(In reply to comment #11)
> Actually, I'll just check this in without the fuelApplication.js change.
> 

Good enough. I can make the small change to FUEL when I land it
Moving this to b1 for Mark's piece to land.
Target Milestone: Firefox 3 alpha6 → Firefox 3 beta1
Depends on: 387470
This patch changes the FUELs Application JS global property to be accessible by privileged code only.
Attachment #272372 - Flags: review?(gavin.sharp)
Comment on attachment 272372 [details] [diff] [review]
make FUEL only available to chrome JS

r=mano
Attachment #272372 - Flags: review?(gavin.sharp) → review+
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Flags: in-testsuite?
You need to log in before you can comment on or make changes to this bug.