mirror of
https://github.com/RGBCube/serenity
synced 2025-07-24 21:27:44 +00:00
LibJS: Ensure function declarations don't leak outside function scopes
When using VM::set_variable() to put the created ScriptFunction onto a ScopeObject, we would previously unexpectedly reach the global object as set_variable() checks each traversed scope for an existing Variable with the given name - which would cause a leak of the inner function past the outer function (we even had a test expecting that behaviour!). Now we first declare functions (as DeclarationKind::Var) before setting them. This will need some more work to make hoisting across non-lexical scopes work, but it fixes this specific issue for now. Fixes #6766.
This commit is contained in:
parent
b221cad659
commit
a92dc4e30d
3 changed files with 31 additions and 10 deletions
|
@ -1,9 +1,11 @@
|
||||||
/*
|
/*
|
||||||
* Copyright (c) 2020, Andreas Kling <kling@serenityos.org>
|
* Copyright (c) 2020, Andreas Kling <kling@serenityos.org>
|
||||||
|
* Copyright (c) 2020-2021, Linus Groh <linusg@serenityos.org>
|
||||||
*
|
*
|
||||||
* SPDX-License-Identifier: BSD-2-Clause
|
* SPDX-License-Identifier: BSD-2-Clause
|
||||||
*/
|
*/
|
||||||
|
|
||||||
|
#include <AK/ScopeGuard.h>
|
||||||
#include <AK/StringBuilder.h>
|
#include <AK/StringBuilder.h>
|
||||||
#include <LibJS/AST.h>
|
#include <LibJS/AST.h>
|
||||||
#include <LibJS/Interpreter.h>
|
#include <LibJS/Interpreter.h>
|
||||||
|
@ -80,13 +82,17 @@ const GlobalObject& Interpreter::global_object() const
|
||||||
|
|
||||||
void Interpreter::enter_scope(const ScopeNode& scope_node, ScopeType scope_type, GlobalObject& global_object)
|
void Interpreter::enter_scope(const ScopeNode& scope_node, ScopeType scope_type, GlobalObject& global_object)
|
||||||
{
|
{
|
||||||
for (auto& declaration : scope_node.functions()) {
|
ScopeGuard guard([&] {
|
||||||
auto* function = ScriptFunction::create(global_object, declaration.name(), declaration.body(), declaration.parameters(), declaration.function_length(), current_scope(), declaration.is_strict_mode());
|
for (auto& declaration : scope_node.functions()) {
|
||||||
vm().set_variable(declaration.name(), function, global_object);
|
auto* function = ScriptFunction::create(global_object, declaration.name(), declaration.body(), declaration.parameters(), declaration.function_length(), current_scope(), declaration.is_strict_mode());
|
||||||
}
|
vm().set_variable(declaration.name(), function, global_object);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
if (scope_type == ScopeType::Function) {
|
if (scope_type == ScopeType::Function) {
|
||||||
push_scope({ scope_type, scope_node, false });
|
push_scope({ scope_type, scope_node, false });
|
||||||
|
for (auto& declaration : scope_node.functions())
|
||||||
|
current_scope()->put_to_scope(declaration.name(), { js_undefined(), DeclarationKind::Var });
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
15
Userland/Libraries/LibJS/Tests/functions/function-nesting.js
Normal file
15
Userland/Libraries/LibJS/Tests/functions/function-nesting.js
Normal file
|
@ -0,0 +1,15 @@
|
||||||
|
test("issue #6766, nested functions should not leak to global object", () => {
|
||||||
|
function foo() {
|
||||||
|
function bar() {
|
||||||
|
function baz() {
|
||||||
|
return 42;
|
||||||
|
}
|
||||||
|
return baz();
|
||||||
|
}
|
||||||
|
return bar();
|
||||||
|
}
|
||||||
|
expect(foo()).toBe(42);
|
||||||
|
expect(globalThis.foo).toBeUndefined();
|
||||||
|
expect(globalThis.bar).toBeUndefined();
|
||||||
|
expect(globalThis.baz).toBeUndefined();
|
||||||
|
});
|
|
@ -2,8 +2,8 @@ describe("function declarations in if statement clauses", () => {
|
||||||
test("if clause", () => {
|
test("if clause", () => {
|
||||||
if (true) function foo() {}
|
if (true) function foo() {}
|
||||||
if (false) function bar() {}
|
if (false) function bar() {}
|
||||||
expect(typeof globalThis.foo).toBe("function");
|
expect(typeof foo).toBe("function");
|
||||||
expect(typeof globalThis.bar).toBe("undefined");
|
expect(typeof bar).toBe("undefined");
|
||||||
});
|
});
|
||||||
|
|
||||||
test("else clause", () => {
|
test("else clause", () => {
|
||||||
|
@ -11,15 +11,15 @@ describe("function declarations in if statement clauses", () => {
|
||||||
else function foo() {}
|
else function foo() {}
|
||||||
if (true);
|
if (true);
|
||||||
else function bar() {}
|
else function bar() {}
|
||||||
expect(typeof globalThis.foo).toBe("function");
|
expect(typeof foo).toBe("function");
|
||||||
expect(typeof globalThis.bar).toBe("undefined");
|
expect(typeof bar).toBe("undefined");
|
||||||
});
|
});
|
||||||
|
|
||||||
test("if and else clause", () => {
|
test("if and else clause", () => {
|
||||||
if (true) function foo() {}
|
if (true) function foo() {}
|
||||||
else function bar() {}
|
else function bar() {}
|
||||||
expect(typeof globalThis.foo).toBe("function");
|
expect(typeof foo).toBe("function");
|
||||||
expect(typeof globalThis.bar).toBe("undefined");
|
expect(typeof bar).toBe("undefined");
|
||||||
});
|
});
|
||||||
|
|
||||||
test("syntax error in strict mode", () => {
|
test("syntax error in strict mode", () => {
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue