Skip to content
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 and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

vm.compileFunction throws an RangeError when invoked with options.lineOffset < 0 #43207

Closed
CathyKMeow opened this issue May 25, 2022 · 9 comments
Labels
vm Issues and PRs related to the vm subsystem.

Comments

@CathyKMeow
Copy link

CathyKMeow commented May 25, 2022

Version

v18.2.0

Platform

Linux localhost 5.17.0-rc7-asahi-next-20220310-5-2-ARCH #4 SMP PREEMPT Fri, 20 May 2022 00:28:08 +0000 aarch64 GNU/Linux

Subsystem

node:vm

What steps will reproduce the bug?

node -p 'vm.compileFunction("", [], { lineOffset: -1 })'

How often does it reproduce? Is there a required condition?

Always, without conditions.

What is the expected behavior?

user@localhost:~$ node -p 'vm.compileFunction("", [], { lineOffset: -1 })'
[Function (anonymous)]

What do you see instead?

user@localhost:~$ node -p 'vm.compileFunction("", [], { lineOffset: -1 })'
node:internal/validators:110
    throw new ERR_OUT_OF_RANGE(name, `>= ${min} && < 4294967296`, value);
    ^

RangeError [ERR_OUT_OF_RANGE]: The value of "options.lineOffset" is out of range. It must be >= 0 && < 4294967296. Received -1
    at Object.compileFunction (node:vm:329:3)
    at [eval]:1:4
    at Script.runInThisContext (node:vm:129:12)
    at Object.runInThisContext (node:vm:305:38)
    at node:internal/process/execution:76:19
    at [eval]-wrapper:6:22
    at evalScript (node:internal/process/execution:75:60)
    at node:internal/main/eval_string:27:3 {
  code: 'ERR_OUT_OF_RANGE'
}

Node.js v18.2.0

Additional information

script.runInContext, runInContext, etc. accept options.lineOffset < 0.

user@localhost:~$ cat /etc/os-release 
NAME="Arch Linux ARM"
PRETTY_NAME="Arch Linux ARM"
ID=archarm
ID_LIKE=arch
BUILD_ID=rolling
ANSI_COLOR="38;2;23;147;209"
HOME_URL="https://archlinuxarm.org/"
DOCUMENTATION_URL="https://archlinuxarm.org/wiki"
SUPPORT_URL="https://archlinuxarm.org/forum"
BUG_REPORT_URL="https://github.com/archlinuxarm/PKGBUILDs/issues"
LOGO=archlinux-logo
user@localhost:~$ date
Wed May 25 10:54:30 PDT 2022
user@localhost:~$ sudo pacman -Syu
:: Synchronizing package databases...
 asahi is up to date
 core is up to date
 extra is up to date
 community is up to date
 alarm is up to date
 aur is up to date
:: Starting full system upgrade...
warning: rust: local (1:1.61.0-1.1) is newer than extra (1:1.61.0-1)
warning: rust-src: local (1:1.61.0-1.1) is newer than extra (1:1.61.0-1)
 there is nothing to do
@devsnek
Copy link
Member

devsnek commented May 25, 2022

i think technically the bug here is that we allow negative numbers elsewhere.

@CathyKMeow
Copy link
Author

But IMO negative number should be allowed, actually.

Here's my use case:

await vm.compileFunction(
    "return (async () => {\n" +
        (await fsp.readFile("something")).toString() +
    "\n})();",
    [], {
    lineOffset: -1,
    contextExtensions: [{ something: something }]
})  ();

@devsnek
Copy link
Member

devsnek commented May 25, 2022

It seems like a better approach might be to support async function bodies in compileFunction instead?

@bnoordhuis
Copy link
Member

Orthogonal issues? V8 deliberately supports negative line and column offsets, ergo, it seems reasonable to support them in node as well.

@VoltrexKeyva VoltrexKeyva added the vm Issues and PRs related to the vm subsystem. label May 26, 2022
@CathyKMeow
Copy link
Author

It seems like a better approach might be to support async function bodies in compileFunction instead?

Yes, but anyway we should support negative lineOffset. In most circumstances we add wrappers around the code, not remove parts of the code.
You can't expect

hing")).toString() +
    "\n})();",
    [], {
    lineOffset: -1,
    contextExtensions: [{ somethi

to work.
And, in the native part of vm,

int line_offset = args[2].As<Int32>()->Value();

line_offset, // line offset

int resource_line_offset = 0,

lineOffset is a signed int32.

@sinkhaha
Copy link
Contributor

@bnoordhuis @devsnek So this question should support negative numbers? I'm happy to submit a pull request - internalCompileFunction method in the verification lineOffset parameter, validateUint32 to validateInt32 on the line, right?

@bnoordhuis
Copy link
Member

@sinkhaha yes, I believe so.

@sinkhaha
Copy link
Contributor

sinkhaha commented May 25, 2023

@bnoordhuis After testing that 'lineOffset' supports negative numbers, stack failed to print the correct line number. For example, the following example passed the test

assert.throws(() => {vm.compileFunction('throw new Error("Sample Error")',[],{ lineOffset: -1 })();
  }, {message: 'Sample Error',stack: 'Error: Sample Error\n    at <anonymous>'
  });

I'm not sure if this behavior meets expectations. If stack is changed to "Error: Sample Error\n at :0:7", the test will not pass. I'm not sure it makes sense to support negative numbers.

@fhinkel
Copy link
Member

fhinkel commented Jan 28, 2024

Closed via #49855

@fhinkel fhinkel closed this as completed Jan 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
vm Issues and PRs related to the vm subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants