Skip to content

Commit ca036d6

Browse files
committed
fix: harden loadModule
1 parent ef1a245 commit ca036d6

3 files changed

Lines changed: 183 additions & 2 deletions

File tree

runtime/src/global_fns.rs

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -653,10 +653,23 @@ pub(crate) fn handle_read_text_file(
653653
throw_js_error(scope, "__nsReadTextFile(path) expects 1 argument");
654654
return;
655655
}
656-
let Some(path) = value_to_string(scope, args.get(0)) else {
656+
// Reject null/undefined explicitly. value_to_string would otherwise coerce them
657+
// to the literal strings "null"/"undefined", leading to a confusing "file not
658+
// found" deeper in the call. A bad module path (e.g. loadModule(null)) must throw
659+
// a clear JS error here rather than panic the runtime.
660+
let path_arg = args.get(0);
661+
if path_arg.is_null_or_undefined() {
662+
throw_js_error(scope, "__nsReadTextFile: path is null or undefined");
663+
return;
664+
}
665+
let Some(path) = value_to_string(scope, path_arg) else {
657666
throw_js_error(scope, "Unable to convert path argument to string");
658667
return;
659668
};
669+
if path.is_empty() {
670+
throw_js_error(scope, "__nsReadTextFile: path is empty");
671+
return;
672+
}
660673
match fs::read_to_string(Path::new(path.as_str())) {
661674
Ok(content) => {
662675
if let Some(value) = v8::String::new(scope, content.as_str()) {
@@ -779,10 +792,27 @@ pub(crate) fn handle_resolve_module_path(
779792
);
780793
return;
781794
}
782-
let Some(specifier) = value_to_string(scope, args.get(0)) else {
795+
// Reject null/undefined explicitly so a bad `loadModule(null)` surfaces as a clean
796+
// JS error instead of being coerced to the literal string "null" and then failing
797+
// deeper in the resolver. The module resolver returns null when it can't find a
798+
// registered module matching a path (e.g. an unregistered CSS file); passing that
799+
// null through must throw, never panic the runtime.
800+
let specifier_arg = args.get(0);
801+
if specifier_arg.is_null_or_undefined() {
802+
throw_js_error(
803+
scope,
804+
"__nsResolveModulePath: module specifier is null or undefined",
805+
);
806+
return;
807+
}
808+
let Some(specifier) = value_to_string(scope, specifier_arg) else {
783809
throw_js_error(scope, "Unable to convert module specifier to string");
784810
return;
785811
};
812+
if specifier.is_empty() {
813+
throw_js_error(scope, "__nsResolveModulePath: module specifier is empty");
814+
return;
815+
}
786816
let parent_path = if args.length() >= 2 {
787817
value_to_string(scope, args.get(1))
788818
} else {
@@ -3285,6 +3315,13 @@ const HELPER_SOURCE: &str = r#"
32853315
var cjsCache = new Map();
32863316
32873317
function resolveSpecifier(specifier, callerFile) {
3318+
// Guard non-string / empty specifiers (e.g. loadModule(null) when the
3319+
// module resolver can't find a registered module for a path). Throw a
3320+
// clear error instead of letting `specifier.charAt(0)` raise a cryptic
3321+
// TypeError or reaching native code with a coerced "null".
3322+
if (typeof specifier !== 'string' || specifier.length === 0) {
3323+
throw new Error('Cannot find module: ' + String(specifier));
3324+
}
32883325
if (typeof globalThis.__nsResolveModulePath !== 'function') return null;
32893326
var appRoot = (globalThis.__nsAppRoot || '').replace(/[\\/]+$/, '');
32903327

runtime/src/lib.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8182,6 +8182,9 @@ mod color_test;
81828182
#[cfg(test)]
81838183
mod error_handling_test;
81848184

8185+
#[cfg(test)]
8186+
mod module_load_test;
8187+
81858188
#[cfg(test)]
81868189
mod instance_cache_test;
81878190

runtime/src/module_load_test.rs

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
//! Regression tests for the module-loading path that backs `global.loadModule`.
2+
//!
3+
//! `global.loadModule` (from @nativescript/core) routes through the runtime's
4+
//! `require` shim and the native `__nsResolveModulePath` / `__nsReadTextFile`
5+
//! host functions. When the module resolver can't find a registered module for a
6+
//! path (e.g. an unregistered CSS file like `app.css`), it produces a null module
7+
//! name. Passing that null down the chain must surface as a *catchable JS error* —
8+
//! it must never panic / abort the Rust runtime.
9+
//!
10+
//! Each test runs a self-contained JS try/catch that returns a sentinel string.
11+
//! If the native layer panicked instead of throwing, the panic cannot be caught by
12+
//! the JS `catch` and would abort the test binary — so a passing test proves the
13+
//! path throws rather than panics.
14+
15+
use crate::Runtime;
16+
17+
/// Evaluate a JS expression and return its string result, or `<no result>` if the
18+
/// outer eval itself threw (it shouldn't — each snippet catches internally).
19+
fn eval(runtime: &mut Runtime, expr: &str) -> String {
20+
runtime
21+
.eval_script_to_string(expr)
22+
.unwrap_or_else(|| "<no result>".to_string())
23+
}
24+
25+
/// Wrap a call so a thrown error becomes `THREW:<message>` and a normal return
26+
/// becomes `NO_THROW:<value>`. A Rust panic in the call would abort the process.
27+
fn caught(call: &str) -> String {
28+
format!(
29+
r#"(function() {{
30+
try {{
31+
var r = {call};
32+
return 'NO_THROW:' + String(r);
33+
}} catch (e) {{
34+
return 'THREW:' + (e && e.message ? e.message : String(e));
35+
}}
36+
}})()"#
37+
)
38+
}
39+
40+
#[test]
41+
fn require_null_throws_not_panics() {
42+
let mut runtime = Runtime::new(".");
43+
let result = eval(&mut runtime, &caught("globalThis.require(null)"));
44+
assert!(
45+
result.starts_with("THREW:"),
46+
"require(null) must throw, got: {result}"
47+
);
48+
}
49+
50+
#[test]
51+
fn require_undefined_throws_not_panics() {
52+
let mut runtime = Runtime::new(".");
53+
let result = eval(&mut runtime, &caught("globalThis.require(undefined)"));
54+
assert!(
55+
result.starts_with("THREW:"),
56+
"require(undefined) must throw, got: {result}"
57+
);
58+
}
59+
60+
#[test]
61+
fn require_empty_string_throws_not_panics() {
62+
let mut runtime = Runtime::new(".");
63+
let result = eval(&mut runtime, &caught("globalThis.require('')"));
64+
assert!(
65+
result.starts_with("THREW:"),
66+
"require('') must throw, got: {result}"
67+
);
68+
}
69+
70+
/// Mirrors how @nativescript/core calls `global.loadModule(resolvedModuleName)`
71+
/// where `resolvedModuleName` is null because no registered webpack module matched
72+
/// the CSS path. The runtime-owned `loadModule` (aliased to `require` here) must
73+
/// throw cleanly.
74+
#[test]
75+
fn load_module_null_resolved_name_throws_not_panics() {
76+
let mut runtime = Runtime::new(".");
77+
let result = eval(
78+
&mut runtime,
79+
&caught(
80+
"(function() { \
81+
var loadModule = globalThis.loadModule || globalThis.require; \
82+
return loadModule(null); \
83+
})()",
84+
),
85+
);
86+
assert!(
87+
result.starts_with("THREW:"),
88+
"loadModule(null) must throw, got: {result}"
89+
);
90+
}
91+
92+
#[test]
93+
fn resolve_module_path_null_throws_not_panics() {
94+
let mut runtime = Runtime::new(".");
95+
let result = eval(&mut runtime, &caught("globalThis.__nsResolveModulePath(null)"));
96+
assert!(
97+
result.starts_with("THREW:"),
98+
"__nsResolveModulePath(null) must throw, got: {result}"
99+
);
100+
}
101+
102+
#[test]
103+
fn resolve_module_path_empty_throws_not_panics() {
104+
let mut runtime = Runtime::new(".");
105+
let result = eval(&mut runtime, &caught("globalThis.__nsResolveModulePath('')"));
106+
assert!(
107+
result.starts_with("THREW:"),
108+
"__nsResolveModulePath('') must throw, got: {result}"
109+
);
110+
}
111+
112+
#[test]
113+
fn read_text_file_null_throws_not_panics() {
114+
let mut runtime = Runtime::new(".");
115+
let result = eval(&mut runtime, &caught("globalThis.__nsReadTextFile(null)"));
116+
assert!(
117+
result.starts_with("THREW:"),
118+
"__nsReadTextFile(null) must throw, got: {result}"
119+
);
120+
}
121+
122+
#[test]
123+
fn read_text_file_empty_throws_not_panics() {
124+
let mut runtime = Runtime::new(".");
125+
let result = eval(&mut runtime, &caught("globalThis.__nsReadTextFile('')"));
126+
assert!(
127+
result.starts_with("THREW:"),
128+
"__nsReadTextFile('') must throw, got: {result}"
129+
);
130+
}
131+
132+
/// Hammer the bad-input path repeatedly: a latent panic / abort or memory-safety
133+
/// issue in the native handlers would surface as a crash rather than 50 clean throws.
134+
#[test]
135+
fn repeated_bad_module_loads_are_stable() {
136+
let mut runtime = Runtime::new(".");
137+
for _ in 0..50 {
138+
let result = eval(&mut runtime, &caught("globalThis.require(null)"));
139+
assert!(result.starts_with("THREW:"), "unexpected: {result}");
140+
}
141+
}

0 commit comments

Comments
 (0)