Closed
Bug 1209107
Opened 9 years ago
Closed 9 years ago
Don't expose module environment objects
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla44
Tracking | Status | |
---|---|---|
firefox44 | --- | fixed |
People
(Reporter: jonco, Assigned: jonco)
References
Details
Attachments
(1 file)
(deleted),
patch
|
shu
:
review+
|
Details | Diff | Splinter Review |
ModuleEnvironmentObject is a type of scope object and is not meant to be exposed to JS in general. Currently it can be obtained by accessing the |environment| property on a ModuleObject and is used by the self-hosted parts of the module implementation and by the tests. There should be an intrinsic so that self-hosted JS can access this and testing functions so that tests can check its state, but the object itself should not be exposed like this. All other ModuleObject properties are either getters for immutable objects or idempotent methods.
Assignee | ||
Comment 3•9 years ago
|
||
Attachment #8666742 -
Flags: review?(shu)
Comment 4•9 years ago
|
||
Comment on attachment 8666742 [details] [diff] [review] bug1209107-environment Review of attachment 8666742 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/builtin/ModuleObject.cpp @@ +762,5 @@ > js::InitModuleClass(JSContext* cx, HandleObject obj) > { > static const JSPropertySpec protoAccessors[] = { > + //JS_PSG("initialEnvironment", ModuleObject_initialEnvironmentGetter, 0), > + //JS_PSG("environment", ModuleObject_environmentGetter, 0), Should delete instead of commenting out. ::: js/src/builtin/TestingFunctions.cpp @@ +2902,5 @@ > + array->initDenseElement(i, StringValue(JSID_TO_STRING(ids[i]))); > + > + args.rval().setObject(*array); > + return true; > +} You already wrote the code for GetModuleEnvironmentNames and GetModuleEnvironmentValue so I have no objections to leaving them in. I don't understand why they were needed though, couldn't you have left the tests the same and only changed uses of module.environment to use the GetModuleEnvironment testing function?
Attachment #8666742 -
Flags: review?(shu) → review+
Assignee | ||
Comment 5•9 years ago
|
||
(In reply to Shu-yu Guo [:shu] from comment #4) > couldn't you have left the > tests the same and only changed uses of module.environment to use the > GetModuleEnvironment testing function? The problem is that the fuzzers can then get hold of the scope object and manipulate it in ways we don't support (e.g. add or delete properties). I've lost the bug where this was happening now, but the point is that these objects are not meant to be exposed to arbitrary script anyway.
Can this land soon? The fuzzbugs for this (duped here) are appearing long enough to be on the verge of being fuzzblockers.
Flags: needinfo?(jcoppeard)
https://hg.mozilla.org/mozilla-central/rev/1ac68e528d12
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox44:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
Assignee | ||
Updated•9 years ago
|
Flags: needinfo?(jcoppeard)
You need to log in
before you can comment on or make changes to this bug.
Description
•