console: prevent constructing console methods by Hakerh400 · Pull Request #26096 · nodejs/node (original) (raw)

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation19 Commits1 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

Ref: #25987

This PR defined all console methods (except the ones intended for internal use) as methods rather than constructible functions. It prevents constructing methods of console instance, except for the global console (opened as separate PR as per #25987 (comment)).

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
commit message follows commit guidelines

Can you add a test?
Also, I don't think the current patch fixes the issue for the global console, as all console methods are wrapped using consoleCall unconditionally, so to fix the global console, the FunctionTemplate for that method in inspector::Initialize needs to use v8::ConstructorBehavior::kThrow.

env->SetMethod(target, "consoleCall", InspectorConsoleCall);
env->SetMethod(

Can you add a test?

Sure.

Also, I don't think the current patch fixes the issue for the global console

It's mentioned in the PR description: "It prevents constructing methods of console instance, except for the global console". Please read the relevant discussion in #25987. I will open separate PR for ConstructorBehavior::kThrow, since it affect many other things.

@Hakerh400

It's mentioned in the PR description: "It prevents constructing methods of console instance, except for the global console".

Sorry for not reading the PR description carefully *_*

I will open separate PR for ConstructorBehavior::kThrow, since it affect many other things.

https://github.com/Hakerh400/node/commit/f084076963c7cdb0dbdcf8ad281c557021f50fc1 affects many other things because the behavior change is done in env-inl.h which gets reused by other internal code - I don't think we can simply replace that wholesale though, that could break a lot of hidden use cases. You can fix the console call without affecting others by manually instantiating a FunctionTemplate in inspector::Initialize instead of using env-SetMethod

Thanks. Applied the suggestion & added a test.