Skip to content

Commit

Permalink
fix: more node-api fixes (#24220)
Browse files Browse the repository at this point in the history
- add fallback impls of external string apis which always copy. after
upstream changes to rusty_v8 we can support non-copying api as well.
- `napi_get_buffer_data` needs to work on all TypedArray instances.
  - Fixes: #24209
- `target_defaults.default_configuration` is used by some modules to
find the corresponding node file from node-gyp
- `node_api_get_module_filename` expects the filename to be a `file:`
url.
  • Loading branch information
devsnek authored Jun 19, 2024
1 parent 6c6ee02 commit 293a36f
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 30 deletions.
66 changes: 48 additions & 18 deletions cli/napi/js_native_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1180,27 +1180,57 @@ fn napi_create_string_utf16(
#[napi_sym]
fn node_api_create_external_string_latin1(
env_ptr: *mut Env,
_string: *const c_char,
_length: usize,
_nogc_finalize_callback: napi_finalize,
_finalize_hint: *mut c_void,
_result: *mut napi_value,
_copied: *mut bool,
string: *const c_char,
length: usize,
nogc_finalize_callback: Option<napi_finalize>,
finalize_hint: *mut c_void,
result: *mut napi_value,
copied: *mut bool,
) -> napi_status {
return napi_set_last_error(env_ptr, napi_generic_failure);
let status =
unsafe { napi_create_string_latin1(env_ptr, string, length, result) };

if status == napi_ok {
unsafe {
*copied = true;
}

if let Some(finalize) = nogc_finalize_callback {
unsafe {
finalize(env_ptr as napi_env, string as *mut c_void, finalize_hint);
}
}
}

status
}

#[napi_sym]
fn node_api_create_external_string_utf16(
env_ptr: *mut Env,
_string: *const u16,
_length: usize,
_nogc_finalize_callback: napi_finalize,
_finalize_hint: *mut c_void,
_result: *mut napi_value,
_copied: *mut bool,
string: *const u16,
length: usize,
nogc_finalize_callback: Option<napi_finalize>,
finalize_hint: *mut c_void,
result: *mut napi_value,
copied: *mut bool,
) -> napi_status {
return napi_set_last_error(env_ptr, napi_generic_failure);
let status =
unsafe { napi_create_string_utf16(env_ptr, string, length, result) };

if status == napi_ok {
unsafe {
*copied = true;
}

if let Some(finalize) = nogc_finalize_callback {
unsafe {
finalize(env_ptr as napi_env, string as *mut c_void, finalize_hint);
}
}
}

status
}

#[napi_sym]
Expand Down Expand Up @@ -2793,8 +2823,8 @@ fn napi_instanceof(
unsafe {
napi_throw_type_error(
env,
"ERR_NAPI_CONS_FUNCTION\0".as_ptr() as _,
"Constructor must be a function\0".as_ptr() as _,
c"ERR_NAPI_CONS_FUNCTION".as_ptr(),
c"Constructor must be a function".as_ptr(),
);
}
return napi_function_expected;
Expand Down Expand Up @@ -3147,8 +3177,8 @@ fn napi_create_dataview<'s>(
unsafe {
return napi_throw_range_error(
env,
"ERR_NAPI_INVALID_DATAVIEW_ARGS\0".as_ptr() as _,
"byte_offset + byte_length should be less than or equal to the size in bytes of the array passed in\0".as_ptr() as _,
c"ERR_NAPI_INVALID_DATAVIEW_ARGS".as_ptr(),
c"byte_offset + byte_length should be less than or equal to the size in bytes of the array passed in".as_ptr(),
);
}
}
Expand Down
12 changes: 2 additions & 10 deletions cli/napi/node_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -426,22 +426,14 @@ fn napi_get_buffer_info(
let env = check_env!(env);
check_arg!(env, value);

// NB: Any TypedArray instance seems to be accepted by this function
// in Node.js.
let Some(ta) =
value.and_then(|v| v8::Local::<v8::TypedArray>::try_from(v).ok())
else {
return napi_set_last_error(env, napi_invalid_arg);
};

let buffer_constructor =
v8::Local::new(&mut env.scope(), &env.buffer_constructor);

if !ta
.instance_of(&mut env.scope(), buffer_constructor.into())
.unwrap_or(false)
{
return napi_set_last_error(env, napi_invalid_arg);
}

if !data.is_null() {
unsafe {
*data = ta.data();
Expand Down
6 changes: 5 additions & 1 deletion ext/napi/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use core::ptr::NonNull;
use deno_core::error::type_error;
use deno_core::error::AnyError;
use deno_core::op2;
use deno_core::url::Url;
use deno_core::ExternalOpsTracker;
use deno_core::OpState;
use deno_core::V8CrossThreadTaskSpawner;
Expand Down Expand Up @@ -528,7 +529,10 @@ where
let type_tag = v8::Private::new(scope, Some(type_tag_name));
let type_tag = v8::Global::new(scope, type_tag);

let env_shared = EnvShared::new(napi_wrap, type_tag, path.clone());
let url_filename =
Url::from_file_path(&path).map_err(|_| type_error("Invalid path"))?;
let env_shared =
EnvShared::new(napi_wrap, type_tag, format!("{url_filename}\0"));

let ctx = scope.get_current_context();
let mut env = Env::new(
Expand Down
4 changes: 3 additions & 1 deletion ext/node/polyfills/process.ts
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,9 @@ Process.prototype.chdir = chdir;

/** https://nodejs.org/api/process.html#processconfig */
Process.prototype.config = {
target_defaults: {},
target_defaults: {
default_configuration: "Release",
},
variables: {},
};

Expand Down

0 comments on commit 293a36f

Please sign in to comment.