feat: HMR robustness and additional tests

Author: NathanWalker

test-app/app/src/main/assets/app/mainpage.js

@@ -78,4 +78,5 @@ require("./tests/testConcurrentAccess");
 
 require("./tests/testESModules.mjs");
 require("./tests/testHmrHotDataExt.mjs");
+require("./tests/testHttpCanonicalKey.mjs");
 require("./tests/testNodeBuiltinsAndOptionalModules.mjs");

test-app/app/src/main/assets/app/tests/testHttpCanonicalKey.mjs

@@ -0,0 +1,52 @@
+// HTTP canonical-key identity tests.
+//
+// Pins the behavior of the native CanonicalizeHttpUrlKey (the loader/registry
+// key) via the debug-only __nsCanonicalizeHttpUrlKey diagnostic global. Pure
+// string logic — no dev server required. Android does NOT collapse the
+// __ns_boot__/__ns_hmr__ virtual prefixes here (that is canonicalHotKey's job),
+// which these specs assert explicitly.
+
+describe("HTTP canonical key", function () {
+    function canon(url) {
+        return globalThis.__nsCanonicalizeHttpUrlKey(url);
+    }
+
+    it("is available in dev builds", function () {
+        if (typeof globalThis.__nsCanonicalizeHttpUrlKey !== "function") {
+            pending("__nsCanonicalizeHttpUrlKey not available (release build?)");
+            return;
+        }
+        expect(typeof globalThis.__nsCanonicalizeHttpUrlKey).toBe("function");
+    });
+
+    it("drops the fragment and unwraps file://http wrappers", function () {
+        if (typeof globalThis.__nsCanonicalizeHttpUrlKey !== "function") { pending("release"); return; }
+        expect(canon("http://h/ns/m/foo.js#frag")).toBe("http://h/ns/m/foo.js");
+        expect(canon("file://http://h/x.js")).toBe("http://h/x.js");
+    });
+
+    it("normalizes versioned bridge endpoints but not deeper paths", function () {
+        if (typeof globalThis.__nsCanonicalizeHttpUrlKey !== "function") { pending("release"); return; }
+        expect(canon("http://h/ns/rt/42")).toBe("http://h/ns/rt");
+        expect(canon("http://h/ns/core/13")).toBe("http://h/ns/core");
+        expect(canon("http://h/ns/rt/42/x.js")).toBe("http://h/ns/rt/42/x.js");
+    });
+
+    it("does NOT collapse __ns_hmr__/__ns_boot__ prefixes (Android loader key)", function () {
+        if (typeof globalThis.__nsCanonicalizeHttpUrlKey !== "function") { pending("release"); return; }
+        expect(canon("http://h/ns/m/__ns_hmr__/v7/foo.js"))
+            .toBe("http://h/ns/m/__ns_hmr__/v7/foo.js");
+    });
+
+    it("strips ?import and sorts remaining query params", function () {
+        if (typeof globalThis.__nsCanonicalizeHttpUrlKey !== "function") { pending("release"); return; }
+        expect(canon("http://h/a?import=1&b=2&a=3")).toBe("http://h/a?a=3&b=2");
+        expect(canon("http://h/a?b=2&a=1")).toBe("http://h/a?a=1&b=2");
+        expect(canon("http://h/ns/core?import=1")).toBe("http://h/ns/core");
+    });
+
+    it("leaves a non-http(s) specifier unchanged", function () {
+        if (typeof globalThis.__nsCanonicalizeHttpUrlKey !== "function") { pending("release"); return; }
+        expect(canon("~/local/foo.js")).toBe("~/local/foo.js");
+    });
+});

test-app/runtime/src/main/cpp/DevFlags.cpp

@@ -106,10 +106,25 @@ static bool s_allowRemoteModules = false;
 static std::vector<std::string> s_remoteModuleAllowlist;
 static bool s_isDebuggable = false;
 
-// Helper to check if a URL starts with a given prefix
-static bool UrlStartsWith(const std::string& url, const std::string& prefix) {
-  if (prefix.size() > url.size()) return false;
-  return url.compare(0, prefix.size(), prefix) == 0;
+// Returns true when `url` is covered by allowlist `entry`, matching only on
+// URL component boundaries so lookalike-host and lookalike-port values are
+// refused: an entry of "https://cdn.example.com" does NOT authorize
+// "https://cdn.example.com.attacker.com/x.js" or
+// "https://cdn.example.com:9999/x.js". The character after the matched entry
+// text must be a path/query/fragment boundary ('/', '?', '#'), or the URL must
+// end exactly at the entry, or the entry must already end in '/'.
+//
+// Deny-by-default: an entry without an explicit port does not match a URL
+// that adds one. To allow a specific port, include it in the entry.
+static bool RemoteUrlMatchesAllowlistEntry(const std::string& url,
+                                           const std::string& entry) {
+  if (entry.empty()) return false;
+  if (url.size() < entry.size()) return false;
+  if (url.compare(0, entry.size(), entry) != 0) return false;
+  if (url.size() == entry.size()) return true;   // exact match
+  if (entry.back() == '/') return true;           // entry ended at a boundary
+  const char next = url[entry.size()];
+  return next == '/' || next == '?' || next == '#';
 }
 
 void InitializeSecurityConfig() {
@@ -195,9 +210,9 @@ bool IsRemoteUrlAllowed(const std::string& url) {
     return true;
   }
 
-  // Check if URL matches any allowlist prefix
-  for (const std::string& prefix : s_remoteModuleAllowlist) {
-    if (UrlStartsWith(url, prefix)) {
+  // Check if URL matches any allowlist entry on a component boundary
+  for (const std::string& entry : s_remoteModuleAllowlist) {
+    if (RemoteUrlMatchesAllowlistEntry(url, entry)) {
       return true;
     }
   }

test-app/runtime/src/main/cpp/HMRSupport.cpp

@@ -31,7 +31,7 @@ namespace tns {
 // definitions live in ModuleInternalCallbacks.cpp; this header-style
 // forward block lets HMRSupport.cpp call them without pulling the
 // resolver header in (avoids a circular include).
-void SetImportMap(const std::string& json);
+void SetImportMapEntries(const std::vector<std::pair<std::string, std::string>>& entries);
 void SetVolatilePatterns(const std::vector<std::string>& patterns);
 std::vector<std::string> GetLoadedModuleUrls();
 
@@ -178,10 +178,53 @@ static bool IsSupportedDevSessionPlatform(const std::string& platform) {
   return platform == "android";
 }
 
+// Parse an import-map value (a JSON string OR a JS object of shape
+// `{ imports: { "<key>": "<url>", ... } }`) into flat (key → URL) entries using
+// V8's own JSON/object model. Returns true if it found an `imports` object
+// (even if empty); false if the value is unusable. Only flat string key→URL
+// mappings are honored; non-string import values are skipped.
+static bool ReadImportMapEntries(v8::Isolate* isolate,
+                                 v8::Local<v8::Context> context,
+                                 v8::Local<v8::Value> importMapValue,
+                                 std::vector<std::pair<std::string, std::string>>* out) {
+  v8::Local<v8::Value> mapVal = importMapValue;
+  if (mapVal->IsString()) {
+    v8::Local<v8::Value> parsed;
+    if (!v8::JSON::Parse(context, mapVal.As<v8::String>()).ToLocal(&parsed)) {
+      return false;
+    }
+    mapVal = parsed;
+  }
+  if (!mapVal->IsObject()) return false;
+  v8::Local<v8::Object> mapObj = mapVal.As<v8::Object>();
+
+  v8::Local<v8::Value> importsVal;
+  if (!mapObj->Get(context, ToV8String(isolate, "imports")).ToLocal(&importsVal) ||
+      !importsVal->IsObject()) {
+    return false;
+  }
+  v8::Local<v8::Object> imports = importsVal.As<v8::Object>();
+  v8::Local<v8::Array> keys;
+  if (!imports->GetOwnPropertyNames(context).ToLocal(&keys)) return false;
+
+  for (uint32_t i = 0; i < keys->Length(); ++i) {
+    v8::Local<v8::Value> keyVal;
+    if (!keys->Get(context, i).ToLocal(&keyVal)) continue;
+    v8::Local<v8::Value> valVal;
+    if (!imports->Get(context, keyVal).ToLocal(&valVal) || !valVal->IsString()) continue;
+    v8::String::Utf8Value keyUtf8(isolate, keyVal);
+    v8::String::Utf8Value valUtf8(isolate, valVal);
+    if (*keyUtf8 && *valUtf8) {
+      out->emplace_back(std::string(*keyUtf8), std::string(*valUtf8));
+    }
+  }
+  return true;
+}
+
 // Apply the v8::Object payload of `__nsConfigureDevRuntime`: re-validate the
-// `importMap` shape and serialize it back to JSON for `SetImportMap`. Parsing
-// runs entirely in V8 (via `ConfigureDevRuntimeCallback`), so this is a thin
-// wrapper over that shared validation.
+// `importMap` shape and read its entries via `ReadImportMapEntries` (V8-based
+// parsing) into the process-wide map. Mirrors the import-map handling in
+// `ConfigureDevRuntimeCallback`.
 static bool ApplyDevRuntimeConfigObject(v8::Isolate* isolate,
                                         v8::Local<v8::Context> context,
                                         v8::Local<v8::Object> payload,
@@ -202,49 +245,20 @@ static bool ApplyDevRuntimeConfigObject(v8::Isolate* isolate,
     return false;
   }
 
-  // Use JSON.stringify on the importMap object — keeps the on-disk format
-  // identical to what `__nsConfigureRuntime` already accepts.
-  v8::Local<v8::Object> jsonObj;
-  v8::Local<v8::Value> globalJson;
-  if (!context->Global()->Get(context, ToV8String(isolate, "JSON")).ToLocal(&globalJson) ||
-      !globalJson->IsObject()) {
-    if (errorMessage != nullptr) {
-      *errorMessage = "[__nsStartDevSession] JSON global unavailable";
-    }
-    return false;
-  }
-  jsonObj = globalJson.As<v8::Object>();
-
-  v8::Local<v8::Value> stringifyFnVal;
-  if (!jsonObj->Get(context, ToV8String(isolate, "stringify")).ToLocal(&stringifyFnVal) ||
-      !stringifyFnVal->IsFunction()) {
-    if (errorMessage != nullptr) {
-      *errorMessage = "[__nsStartDevSession] JSON.stringify unavailable";
-    }
-    return false;
-  }
-
-  v8::Local<v8::Function> stringifyFn = stringifyFnVal.As<v8::Function>();
-  v8::Local<v8::Value> args[] = {importMapValue};
-  v8::MaybeLocal<v8::Value> maybeJson = stringifyFn->Call(context, jsonObj, 1, args);
-  v8::Local<v8::Value> jsonVal;
-  if (!maybeJson.ToLocal(&jsonVal) || !jsonVal->IsString()) {
+  std::vector<std::pair<std::string, std::string>> importEntries;
+  if (!ReadImportMapEntries(isolate, context, importMapValue, &importEntries)) {
     if (errorMessage != nullptr) {
-      *errorMessage = "[__nsStartDevSession] failed to serialize importMap";
+      *errorMessage = "[__nsStartDevSession] failed to read importMap";
     }
     return false;
   }
-
-  v8::String::Utf8Value jsonUtf8(isolate, jsonVal);
-  std::string importMapJson = *jsonUtf8 ? *jsonUtf8 : "";
-  if (importMapJson.empty()) {
+  if (importEntries.empty()) {
     if (errorMessage != nullptr) {
       *errorMessage = "[__nsStartDevSession] runtime config importMap was empty";
     }
     return false;
   }
-
-  SetImportMap(importMapJson);
+  SetImportMapEntries(importEntries);
 
   std::vector<std::string> patterns;
   v8::Local<v8::Value> volatilePatternsValue;
@@ -447,14 +461,10 @@ bool ApplyDevRuntimeConfigFromUrl(const std::string& url,
   return true;
 }
 
-// Native-side mirror of `__NS_HMR_BOOT_COMPLETE__`. Read by the
-// runloop pump in `MaybePumpJSThreadDuringBoot` so its gate is a
-// single relaxed atomic load on the HMR-time hot path.
-static std::atomic<bool> g_devSessionBootComplete{false};
-
-static inline bool IsDevSessionBootComplete() {
-  return g_devSessionBootComplete.load(std::memory_order_relaxed);
-}
+// The live "dev-session boot complete" signal is the JS global
+// __NS_HMR_BOOT_COMPLETE__, set by ApplyDevSessionGlobals /
+// SetDevSessionBootComplete below. (There is no native runloop pump on
+// Android — the main NativeScript isolate runs JS on the UI thread.)
 
 void ApplyDevSessionGlobals(v8::Isolate* isolate,
                             v8::Local<v8::Context> context,
@@ -464,7 +474,6 @@ void ApplyDevSessionGlobals(v8::Isolate* isolate,
   SetBooleanGlobal(isolate, context, "__NS_HMR_BOOT_COMPLETE__", false);
   SetBooleanGlobal(isolate, context, "__NS_HMR_CLIENT_ACTIVE__", false);
   SetBooleanGlobal(isolate, context, "__NS_HMR_BROWSER_RUNTIME_CLIENT_ACTIVE__", false);
-  g_devSessionBootComplete.store(false, std::memory_order_relaxed);
   if (IsScriptLoadingLogEnabled()) {
     DEBUG_WRITE("[dev-session] globals applied session=%s origin=%s ws=%s bootComplete=false",
                 session.sessionId.c_str(), session.origin.c_str(),
@@ -476,7 +485,6 @@ void SetDevSessionBootComplete(v8::Isolate* isolate,
                                v8::Local<v8::Context> context,
                                bool value) {
   SetBooleanGlobal(isolate, context, "__NS_HMR_BOOT_COMPLETE__", value);
-  g_devSessionBootComplete.store(value, std::memory_order_relaxed);
   if (IsScriptLoadingLogEnabled()) {
     DEBUG_WRITE("[dev-session] __NS_HMR_BOOT_COMPLETE__=%s",
                 value ? "true" : "false");
@@ -2439,32 +2447,12 @@ void ConfigureDevRuntimeCallback(const v8::FunctionCallbackInfo<v8::Value>& info
   v8::Local<v8::Value> importMapVal;
   if (config->Get(ctx, ToV8String(isolate, "importMap")).ToLocal(&importMapVal) &&
       !importMapVal->IsUndefined() && !importMapVal->IsNull()) {
-    std::string jsonStr;
-    if (importMapVal->IsString()) {
-      v8::String::Utf8Value utf8(isolate, importMapVal);
-      if (*utf8) jsonStr = *utf8;
-    } else if (importMapVal->IsObject()) {
-      v8::Local<v8::Value> jsonGlobalVal;
-      if (ctx->Global()->Get(ctx, ToV8String(isolate, "JSON")).ToLocal(&jsonGlobalVal) &&
-          jsonGlobalVal->IsObject()) {
-        v8::Local<v8::Object> jsonObj = jsonGlobalVal.As<v8::Object>();
-        v8::Local<v8::Value> stringifyVal;
-        if (jsonObj->Get(ctx, ToV8String(isolate, "stringify")).ToLocal(&stringifyVal) &&
-            stringifyVal->IsFunction()) {
-          v8::Local<v8::Function> stringify = stringifyVal.As<v8::Function>();
-          v8::Local<v8::Value> args[] = {importMapVal};
-          v8::Local<v8::Value> result;
-          if (stringify->Call(ctx, jsonObj, 1, args).ToLocal(&result) && result->IsString()) {
-            v8::String::Utf8Value utf8(isolate, result);
-            if (*utf8) jsonStr = *utf8;
-          }
-        }
-      }
-    }
-    if (!jsonStr.empty()) {
-      SetImportMap(jsonStr);
+    std::vector<std::pair<std::string, std::string>> importEntries;
+    if (ReadImportMapEntries(isolate, ctx, importMapVal, &importEntries) &&
+        !importEntries.empty()) {
+      SetImportMapEntries(importEntries);
       if (logScriptLoading) {
-        DEBUG_WRITE("[__nsConfigureRuntime] import map set (%zu bytes)", jsonStr.size());
+        DEBUG_WRITE("[__nsConfigureRuntime] import map set (%zu entries)", importEntries.size());
       }
     }
   }
@@ -2909,6 +2897,22 @@ void ApplyStyleUpdateCallback(const v8::FunctionCallbackInfo<v8::Value>& info) {
   }
 }
 
+// Debug-only diagnostic: expose CanonicalizeHttpUrlKey to JS so the test harness
+// can pin its identity behavior. Not part of the @nativescript/vite client API.
+// The whole installer is gated on isDebuggable at the call site, so this never
+// ships in release.
+void CanonicalizeHttpUrlKeyCallback(const v8::FunctionCallbackInfo<v8::Value>& info) {
+  v8::Isolate* isolate = info.GetIsolate();
+  v8::HandleScope scope(isolate);
+  if (info.Length() < 1 || !info[0]->IsString()) {
+    info.GetReturnValue().SetEmptyString();
+    return;
+  }
+  v8::String::Utf8Value u(isolate, info[0]);
+  std::string key = CanonicalizeHttpUrlKey(*u ? std::string(*u) : std::string());
+  info.GetReturnValue().Set(ToV8String(isolate, key.c_str()));
+}
+
 void GetLoadedModuleUrlsCallback(const v8::FunctionCallbackInfo<v8::Value>& info) {
   v8::Isolate* isolate = info.GetIsolate();
   v8::HandleScope scope(isolate);
@@ -2962,6 +2966,7 @@ void InitializeHmrDevGlobals(v8::Isolate* isolate, v8::Local<v8::Context> contex
   InstallGlobalFunction(isolate, context, "__nsReloadDevApp", ReloadDevAppCallback);
   InstallGlobalFunction(isolate, context, "__nsApplyStyleUpdate", ApplyStyleUpdateCallback);
   InstallGlobalFunction(isolate, context, "__nsGetLoadedModuleUrls", GetLoadedModuleUrlsCallback);
+  InstallGlobalFunction(isolate, context, "__nsCanonicalizeHttpUrlKey", CanonicalizeHttpUrlKeyCallback);
 }
 
 void CleanupHMRGlobals() {

test-app/runtime/src/main/cpp/HMRSupport.h

@@ -167,9 +167,10 @@ void ClearHttpModulePrefetchCache();
 // synchronous network turn so the caller can pump its own runloop (e.g. the
 // JS-thread runloop so a placeholder UI can repaint during cold-boot).
 //
-// Default: a built-in pump that no-ops outside the JS thread / after the
-// dev-session boot completes (see `MaybePumpJSThreadDuringBoot` in
-// HMRSupport.cpp).
+// Default: a no-op (`NoopHttpFetchYield`). Android's main NativeScript
+// isolate runs JS on the UI thread, so there is no separate JS-thread
+// runloop to pump here; a host that drives its own loop can install a real
+// pump (e.g. one calling ALooper_pollOnce(0)) via this hook.
 //
 // Pass `nullptr` to disable any yielding (used by hosts that drive their own
 // run loop or by tests that want bit-for-bit deterministic fetch timing).