feat: HMR robustness and additional tests

Author: NathanWalker

NativeScript/runtime/DevFlags.mm

@@ -82,10 +82,25 @@ bool IsHttpFetchUrlLogEnabled() {
 static bool s_allowRemoteModules = false;
 static std::vector<std::string> s_remoteModuleAllowlist;
 
-// 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 authorized by allowlist `entry`.
+//
+// This is intentionally stricter than a raw string-prefix test: after the
+// matched entry text, the next character in `url` must be a URL-component
+// boundary ('/', '?', or '#'), the URL must end exactly at the entry, or the
+// entry must itself end in '/'. That refuses lookalike-host and lookalike-port
+// bypasses — an entry of "https://cdn.example.com" must NOT authorize
+// "https://cdn.example.com.attacker.com/x.js" or
+// "https://cdn.example.com:9999/x.js". To allow a specific port, include it in
+// the allowlist entry (deny-by-default for anything not explicitly listed).
+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() {
@@ -147,9 +162,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 URL-component boundary.
+  for (const std::string& entry : s_remoteModuleAllowlist) {
+    if (RemoteUrlMatchesAllowlistEntry(url, entry)) {
       return true;
     }
   }

NativeScript/runtime/HMRSupport.mm

@@ -3209,6 +3209,31 @@ void InitializeHmrDevGlobals(v8::Isolate* isolate, v8::Local<v8::Context> contex
       InitializeHotDisposeRunner(isolate, context);
       InitializeHotPruneRunner(isolate, context);
       InitializeHotDeclinedHelper(isolate, context);
+
+      // Debug-only diagnostic: expose the HTTP canonical-key function to JS so
+      // the test harness can pin its identity behavior across cache-busters,
+      // boot/hmr tags, and versioned bridge endpoints. This is NOT part of the
+      // HMR client API surface and is never installed in release builds.
+      {
+        auto canonicalizeCb = [](const v8::FunctionCallbackInfo<v8::Value>& info) {
+          v8::Isolate* iso = info.GetIsolate();
+          if (info.Length() < 1 || !info[0]->IsString()) {
+            info.GetReturnValue().SetEmptyString();
+            return;
+          }
+          v8::String::Utf8Value u(iso, info[0]);
+          std::string key =
+              CanonicalizeHttpUrlKey(*u ? std::string(*u) : std::string());
+          info.GetReturnValue().Set(tns::ToV8String(iso, key.c_str()));
+        };
+        v8::Local<v8::Function> fn =
+            v8::Function::New(context, canonicalizeCb).ToLocalChecked();
+        context->Global()
+            ->CreateDataProperty(
+                context, tns::ToV8String(isolate, "__nsCanonicalizeHttpUrlKey"),
+                fn)
+            .Check();
+      }
     } catch (...) {
       // Don't crash if HMR setup fails
     }

NativeScript/runtime/ModuleInternalCallbacks.mm

@@ -409,33 +409,40 @@ static bool ShouldTraceRegistryKey(const std::string& rawKey, const std::string&
 
 void SetImportMap(const std::string& json) {
   g_importMap.clear();
-  // Minimal JSON parser for {"imports": {"key": "value", ...}} shape.
-  // We avoid pulling in a full JSON library; the import map is simple flat key-value.
-  size_t importsPos = json.find("\"imports\"");
-  if (importsPos == std::string::npos) return;
-  size_t braceOpen = json.find('{', importsPos + 9);
-  if (braceOpen == std::string::npos) return;
-  size_t braceClose = json.find('}', braceOpen + 1);
-  if (braceClose == std::string::npos) return;
-
-  std::string inner = json.substr(braceOpen + 1, braceClose - braceOpen - 1);
-  // Parse "key": "value" pairs
-  size_t pos = 0;
-  while (pos < inner.size()) {
-    size_t keyStart = inner.find('"', pos);
-    if (keyStart == std::string::npos) break;
-    size_t keyEnd = inner.find('"', keyStart + 1);
-    if (keyEnd == std::string::npos) break;
-    std::string key = inner.substr(keyStart + 1, keyEnd - keyStart - 1);
-
-    size_t valStart = inner.find('"', keyEnd + 1);
-    if (valStart == std::string::npos) break;
-    size_t valEnd = inner.find('"', valStart + 1);
-    if (valEnd == std::string::npos) break;
-    std::string val = inner.substr(valStart + 1, valEnd - valStart - 1);
-
-    g_importMap[key] = val;
-    pos = valEnd + 1;
+  // The import map is a small, flat {"imports": {"specifier": "target", ...}}
+  // object. Parse it with Foundation's JSON reader rather than a hand-rolled
+  // scanner so escapes, nesting, and malformed input are handled correctly and
+  // can't desync key/value pairing.
+  @autoreleasepool {
+    NSData* data = [NSData dataWithBytes:json.data() length:json.size()];
+    if (data == nil || data.length == 0) {
+      return;
+    }
+    NSError* err = nil;
+    id parsed = [NSJSONSerialization JSONObjectWithData:data
+                                                options:kNilOptions
+                                                  error:&err];
+    if (parsed == nil || ![parsed isKindOfClass:[NSDictionary class]]) {
+      if (IsScriptLoadingLogEnabled()) {
+        NSString* detail = err.localizedDescription ?: @"not an object";
+        Log(@"[import-map] parse failed: %s", [detail UTF8String] ?: "unknown");
+      }
+      return;
+    }
+    id imports = [(NSDictionary*)parsed objectForKey:@"imports"];
+    if (![imports isKindOfClass:[NSDictionary class]]) {
+      return;  // no "imports" object → empty map, same as the prior parser
+    }
+    for (id key in (NSDictionary*)imports) {
+      if (![key isKindOfClass:[NSString class]]) continue;
+      id value = [(NSDictionary*)imports objectForKey:key];
+      if (![value isKindOfClass:[NSString class]]) continue;  // skip non-string targets
+      const char* k = [(NSString*)key UTF8String];
+      const char* v = [(NSString*)value UTF8String];
+      if (k != nullptr && v != nullptr) {
+        g_importMap[std::string(k)] = std::string(v);
+      }
+    }
   }
   if (IsScriptLoadingLogEnabled()) {
     Log(@"[import-map] loaded %lu entries", (unsigned long)g_importMap.size());

NativeScript/runtime/Runtime.mm

@@ -32,8 +32,6 @@
 #include "URLPatternImpl.h"
 #include "URLSearchParamsImpl.h"
 #include <vector>
-#include "HMRSupport.h"
-#include "DevFlags.h"
 
 #define STRINGIZE(x) #x
 #define STRINGIZE_VALUE_OF(x) STRINGIZE(x)

TestRunner/app/tests/HttpEsmLoaderTests.js

@@ -502,4 +502,54 @@ describe("HTTP ESM Loader", function() {
     });
 });
 
+// Focused, deterministic coverage for the native HTTP canonical-key function.
+// These run only in debug builds, where HMRSupport.mm installs the
+// `__nsCanonicalizeHttpUrlKey` diagnostic global; in release they self-skip via
+// pending(). They require no HTTP host — they pin pure string identity behavior.
+describe("HTTP canonical key (native __nsCanonicalizeHttpUrlKey)", function () {
+    function getCanon() {
+        return (typeof global !== "undefined") ? global.__nsCanonicalizeHttpUrlKey : undefined;
+    }
+
+    function checkKey(input, expected) {
+        var canon = getCanon();
+        if (typeof canon !== "function") {
+            pending("__nsCanonicalizeHttpUrlKey not installed (release build)");
+            return;
+        }
+        expect(canon(input)).toBe(expected);
+    }
+
+    it("is installed as a function in debug builds", function () {
+        var canon = getCanon();
+        if (typeof canon !== "function") {
+            pending("__nsCanonicalizeHttpUrlKey not installed (release build)");
+            return;
+        }
+        expect(typeof canon).toBe("function");
+    });
+
+    it("drops dev cache-busters (t/v/import) but keeps real query params", function () {
+        checkKey("http://h/ns/core?p=x&t=123&v=9&import=1", "http://h/ns/core?p=x");
+    });
+
+    it("leaves public (non-dev, non-volatile) URLs untouched", function () {
+        checkKey("https://cdn.example.com/lib.js?token=abc", "https://cdn.example.com/lib.js?token=abc");
+    });
+
+    it("strips boot and hmr tags to the canonical /ns/m path", function () {
+        checkKey("http://h/ns/m/__ns_boot__/b1/foo.js", "http://h/ns/m/foo.js");
+        checkKey("http://h/ns/m/__ns_hmr__/v7/foo.js", "http://h/ns/m/foo.js");
+    });
+
+    it("collapses versioned bridge endpoints to their base path", function () {
+        checkKey("http://h/ns/rt/42", "http://h/ns/rt");
+        checkKey("http://h/ns/core/13", "http://h/ns/core");
+    });
+
+    it("ignores URL fragments for dev endpoints", function () {
+        checkKey("http://h/ns/m/foo.js#frag", "http://h/ns/m/foo.js");
+    });
+});
+
 console.log("HTTP ESM Loader tests loaded");