Closed
Bug 355025
Opened 18 years ago
Closed 18 years ago
Regression: Iterator() does not work any longer
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
VERIFIED
FIXED
People
(Reporter: igor, Assigned: igor)
References
Details
(Keywords: verified1.8.1, Whiteboard: [sg:nse] js1.7, regression from security bug)
Attachments
(2 files)
|
(deleted),
patch
|
brendan
:
review+
mtschrep
:
approval1.8.1+
|
Details | Diff | Splinter Review |
|
(deleted),
text/plain
|
Details |
My patch for security-restricted bug 354750 introduced a regression. Now Iterator(obj) would not construct a default iterator but throw an exception about value-is-not-a-function, only new Iterator(obj) would.
|
Assignee | |
Comment 1•18 years ago
|
||
The test case is trivial:
js> Iterator([])
Iterator([])
aaaa
typein:1: TypeError: Iterator([]) is not a function
js>
|
|
Assignee | |
Comment 2•18 years ago
|
||
A better solution would be to call js_ValueToIterator when Iterator is called as a function, but that would require to pass JSITER_ENUMERATE from the interpreter to js_ValueToIterator and increase the patch. So the cleanup should wait bug 354982.
Attachment #240814 -
Flags: review?(brendan)
|
||
Comment 3•18 years ago
|
||
Comment on attachment 240814 [details] [diff] [review]
Fix v1
> id = ATOM_TO_JSID(cx->runtime->atomState.iteratorAtom);
>- return JS_GetMethodById(cx, obj, id, &obj, &fval) &&
>- js_InternalCall(cx, obj, fval, argc - 1, argv + 1, rval);
>+ if (!JS_GetMethodById(cx, obj, id, &obj2, &fval))
I made the same mistake in the past, and should have caught this. FWIW, I'll review one more time, but my record isn't good.
>+ return JS_FALSE;
>+
>+ if (JSVAL_IS_VOID(fval)) {
>+ /* Fail over to the default enumerating native iterator. */
>+ keyonly = JS_FALSE;
>+ return js_ValueToBoolean(cx, argv[1], &keyonly) &&
>+ js_NewNativeIterator(cx, obj,
>+ keyonly ? 0 : JSITER_FOREACH,
>+ rval);
>+ }
The lack of Object.prototype.__iterator__ requires this, yeah. Is it needed anywhere else?
Drivers: this is needed as a < 1-day turnaround regression fix for 1.8.1.
/be
Attachment #240814 -
Flags: review?(brendan)
Attachment #240814 -
Flags: review+
Attachment #240814 -
Flags: approval1.8.1?
|
|
Assignee | |
Comment 4•18 years ago
|
||
(In reply to comment #3)
> The lack of Object.prototype.__iterator__ requires this, yeah. Is it needed
> anywhere else?
Well, after creating the initial patch for bug 354982 I think this is the only place as there are no other places besides js_NewNativeIterator that create iterator instances.
|
|
Assignee | |
Comment 5•18 years ago
|
||
I committed the patch from comment 2 to the trunk:
Checking in jsiter.c;
/cvsroot/mozilla/js/src/jsiter.c,v <-- jsiter.c
new revision: 3.46; previous revision: 3.45
done
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
|
||
Comment 6•18 years ago
|
||
Comment on attachment 240814 [details] [diff] [review]
Fix v1
Approved for RC2.
Attachment #240814 -
Flags: approval1.8.1? → approval1.8.1+
|
|
Assignee | |
Comment 7•18 years ago
|
||
I committed the patch from comment 2 to MOZILLA_1_8_BRANCH:
Checking in jsiter.c;
/cvsroot/mozilla/js/src/jsiter.c,v <-- jsiter.c
new revision: 3.17.2.21; previous revision: 3.17.2.20
done
Keywords: fixed1.8.1
|
||
Comment 8•18 years ago
|
||
|
|
||
Updated•18 years ago
|
Flags: in-testsuite+
|
||
Updated•18 years ago
|
Whiteboard: [sg:nse] regression from security bug
|
|
||
Comment 9•18 years ago
|
||
verified fixed 1.8 1.9 20061002 windows/linux 1.8 macppc
Status: RESOLVED → VERIFIED
Keywords: fixed1.8.1 → verified1.8.1
|
|
||
Updated•18 years ago
|
Whiteboard: [sg:nse] regression from security bug → [sg:nse] js1.7, regression from security bug
|
|
||
Updated•18 years ago
|
Group: security
|
|
||
Comment 10•18 years ago
|
||
/cvsroot/mozilla/js/tests/js1_7/iterable/regress-355025.js,v <-- regress-355025.js
You need to log in
before you can comment on or make changes to this bug.
Description
•