This fixes a race condition in the watchdog timer used for vm timeouts.
The condition would terminate the main stack's execution instead of the
code running under the sandbox.
PR-URL: https://github.com/nodejs/node/pull/13074
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Currently, when in strict mode, function
declarations are copied on the sandbox by
CopyProperties(), which is not necessary
and will break when CP is removed.
This change maintains current behavior,
letting GlobalPropertySetterCallback
copy functions on the sandbox instead
of using CP to do the task.
PR-URL: https://github.com/nodejs/node/pull/12051
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
In the implementation of the vm module,
if a property is successfully deleted
on the sandbox, we also need to delete it
on the global_proxy object. Therefore, we
must not call args.GetReturnValue().Set().
We only intercept, i.e., call
args.GetReturnValue().Set(), in the
DeleterCallback, if Delete() failed, e.g. because
the property was read only.
PR-URL: https://github.com/nodejs/node/pull/11266
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Fix a warning that was introduced in commit 67af1ad ("src: refactor
CopyProperties to remove JS") from a few days ago. This particular
change was suggested by me, mea culpa.
Fixes the following warning:
../src/node_contextify.cc:151:13: warning: ignoring return
value of function declared with warn_unused_result attribute
[-Wunused-result]
sandbox_obj->DefineProperty(context, key, *desc);
PR-URL: https://github.com/nodejs/node/pull/11197
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
CopyProperties() is refactored to use
the V8 5.5 DefineProperty() API call.
The change does not alter current behaviour.
It is a step prior to removing the function
CopyProperties, which becomes reduntant
after fixes of V8 SetNamedPropertyHandler
in 5.5. V8.
Strings used as property attributes
(value, enumerable etc) and accessors
are defined as persistent strings
in src/env.h
PR-URL: https://github.com/nodejs/node/pull/11102
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Check that the property doesn't have the read-only flag set before
overwriting it.
This is Ben Noordhuis previous commit, but keeping
is_contextual_store. is_contextual_store describes whether
this.foo = 42 or foo = 42 was called. The second is contextual
and will fail in strict mode if foo is used without
declaration. Therefore only do an early return if it is
a contextual store. In particular, don't do an early
return for Object.defineProperty(this, ...).
Fixes: https://github.com/nodejs/node/issues/10223
Refs: https://github.com/nodejs/node/pull/10227
PR-URL: https://github.com/nodejs/node/pull/11109
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Refactor various functions that read values from the contextify
options object. Rather than passing args and the index, pass the
value at that index.
We use env->isolate() rather than args.GetIsolate(), but since env
was constructed from args, this is the same isolate.
PR-URL: https://github.com/nodejs/node/pull/8850
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
When a vm script aborted after a timeout/signal interruption, test
whether the local timeout/signal watchdog was responsible for
terminating the execution.
Without this, when a shorter timer from an outer `vm.run*` invocation
fires before an inner timeout, the inner timeout would throw an error
instead of the outer one, but because it did not witness the timeout
itself, it would assume the termination was the result of a signal
interruption.
PR-URL: https://github.com/nodejs/node/pull/7373
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
- Adds the `breakEvalOnSigint` option to `vm.runIn(This)Context`.
This uses a watchdog thread to wait for SIGINT and generally works
just like the existing `timeout` option.
- Adds a method to the existing timer-based watchdog to check if it
stopped regularly or by running into the timeout. This is used to
tell a SIGINT abort from a timer-based one.
- Adds (internal) `process._{start,stop}SigintWatchdog` methods to
start/stop the watchdog thread used by the above option manually.
This will be used in the REPL to set up SIGINT handling before
entering terminal raw mode, so that there is no time window in
which Ctrl+C fully aborts the process.
PR-URL: https://github.com/nodejs/node/pull/6635
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
v8::Object::GetAlignedPointerFromInternalField() returns a random value
if Wrap() hasn't been run on the object handle. Causing v8 to abort if
certain getters are accessed. It's possible to access these getters and
functions during class construction through the AsyncWrap init()
callback, and also possible in a subset of those scenarios while running
the persistent handle visitor.
Mitigate this issue by manually setting the internal aligned pointer
field to nullptr in the BaseObject constructor and add necessary logic
to return appropriate values when nullptr is encountered.
PR-URL: https://github.com/nodejs/node/pull/6184
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Simplify how node_contextify was keeping a weak reference to the
sandbox object in order to prepare for new style phantom weakness V8
API. It is simpler (and more robust) for the context to hold a
reference to the sandbox in an embedder data field. Doing otherwise
meant that the sandbox could become weak while the context was still
alive. This wasn't a problem because we would make the reference
strong at that point.
Since the sandbox must live at least as long as the context, it
would be better for the context to hold onto the sandbox.
PR-URL: https://github.com/nodejs/node/pull/5392
Reviewed-By: Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl>
Cleanup how node_contextify keeps weak references in order to prepare
for new style phantom weakness API. We didn't need to keep a weak
reference to the context's global proxy, as the context holds it.
PR-URL: https://github.com/nodejs/node/pull/5392
Reviewed-By: Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl>
Fix segmentation faults when compiling the same code with
`produceCachedData` option. V8 ignores the option when the code is in
its compilation cache and does not return cached data. Added
`cachedDataProduced` property to `v8.Script` to denote whether the
cached data is produced successfully.
PR-URL: https://github.com/nodejs/node/pull/5343
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Stop using the deprecated `GetHiddenValue()` and `SetHiddenValue()`
methods, start using `GetPrivate()` and `SetPrivate()` instead.
This commit turns some of the entries in the per-isolate string table
into private symbols.
PR-URL: https://github.com/nodejs/node/pull/5045
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Do not blindly take data from underlying `ArrayBuffer`, use
`ByteOffset`/`ByteLength` of `Uint8Array` itself.
Additionally, fix tests that weren't actually properly running because
of V8's internal code cache. The code should be different, otherwise the
cached data won't be used at all.
Fix: #4939
PR-URL: https://github.com/nodejs/node/pull/4947
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
The vm module's displayErrors option attaches error arrow
messages as a hidden property. Later, core JavaScript code
can optionally decorate the error stack with the arrow message.
However, when user code catches an error, it has no way to
access the arrow message. This commit changes the behavior of
displayErrors to mean "decorate the error stack if an error
occurs."
Fixes: https://github.com/nodejs/node/issues/4835
PR-URL: https://github.com/nodejs/node/pull/4874
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Introduce `cachedData`/`produceCachedData` options for `v8.Script`.
Could be used to consume/produce V8's code cache for speeding up
compilation of known code.
PR-URL: https://github.com/nodejs/node/pull/4777
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Because Node modules are wrapped, errors on the first line
of a file leak the wrapper to the user and report the wrong
column number. This commit adds a line break to the module
wrapper so that the first line is treated the same as all
other lines. To compensate for the additional line, a line
offset of -1 is also applied to errors.
Fixes: https://github.com/nodejs/node/issues/2860
PR-URL: https://github.com/nodejs/node/pull/2867
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Marcel Laverdet