🐛 (device-management-kit) [DSDK-1136]: Fix Android Manager API requests rejected with HTTP 400 due to trailing slash in query params (#1484)

Author: aussedatlo

.changeset/fix-rn-url-trailing-slash.md

@@ -0,0 +1,5 @@
+---
+"@ledgerhq/device-management-kit": patch
+---
+
+Fix React Native Manager API requests failing with HTTP 400 because the last query parameter value was suffixed with a stray `/` (e.g. `provider=1/`). `DmkNetworkClient` now builds and sends the request URL as a plain string instead of round-tripping through `URL`/`URL.toString()`, which corrupted URLs with a query string on affected React Native versions (facebook/react-native#54242).

packages/device-management-kit/src/api/network/DmkNetworkClient.test.ts

@@ -60,6 +60,23 @@ describe("DmkNetworkClient", () => {
       expect(url.searchParams.has("skip")).toBe(false);
       expect(url.searchParams.has("alsoSkip")).toBe(false);
     });
+
+    it("should pass a plain string URL to fetch with no trailing slash after the query (facebook/react-native#54242)", async () => {
+      const fetchMock = vi.fn().mockResolvedValue(jsonResponse({ ok: true }));
+      const client = new DmkNetworkClient({ fetch: fetchMock });
+
+      await client.get(
+        "https://manager.api.live.ledger.com/api/get_device_version",
+        { params: { target_id: 858783748, provider: 1 } },
+      );
+
+      const [calledUrl] = fetchMock.mock.calls[0]!;
+      expect(typeof calledUrl).toBe("string");
+      expect(calledUrl).toBe(
+        "https://manager.api.live.ledger.com/api/get_device_version?target_id=858783748&provider=1",
+      );
+      expect((calledUrl as string).endsWith("/")).toBe(false);
+    });
   });
 
   describe("headers", () => {

packages/device-management-kit/src/api/network/DmkNetworkClient.ts

@@ -187,7 +187,7 @@ export class DmkNetworkClient {
 
     let response: Response;
     try {
-      response = await this.getFetch()(url.toString(), {
+      response = await this.getFetch()(url, {
         method: config.method,
         headers,
         body,

packages/device-management-kit/src/api/network/DmkNetworkClientHelpers.test.ts

@@ -45,23 +45,23 @@ describe("DmkNetworkClientHelpers", () => {
   describe("buildUrl", () => {
     it("should keep an absolute URL as-is", () => {
       const url = buildUrl({ url: "https://api.example.com/items" });
-      expect(url.toString()).toBe("https://api.example.com/items");
+      expect(url).toBe("https://api.example.com/items");
     });
 
     it("should prepend the base URL to a relative path", () => {
       const url = buildUrl({
         url: "/items",
         baseUrl: "https://api.example.com/",
       });
-      expect(url.toString()).toBe("https://api.example.com/items");
+      expect(url).toBe("https://api.example.com/items");
     });
 
     it("should ignore the base URL when the input URL is absolute", () => {
       const url = buildUrl({
         url: "https://other.example.com/items",
         baseUrl: "https://api.example.com",
       });
-      expect(url.toString()).toBe("https://other.example.com/items");
+      expect(url).toBe("https://other.example.com/items");
     });
 
     it("should append query params and skip null/undefined entries", () => {
@@ -75,36 +75,56 @@ describe("DmkNetworkClientHelpers", () => {
           alsoSkip: undefined,
         },
       });
-      expect(url.searchParams.get("chain")).toBe("1");
-      expect(url.searchParams.get("contract")).toBe("0xabc");
-      expect(url.searchParams.get("active")).toBe("true");
-      expect(url.searchParams.has("skip")).toBe(false);
-      expect(url.searchParams.has("alsoSkip")).toBe(false);
+      expect(typeof url).toBe("string");
+      const parsed = new URL(url);
+      expect(parsed.searchParams.get("chain")).toBe("1");
+      expect(parsed.searchParams.get("contract")).toBe("0xabc");
+      expect(parsed.searchParams.get("active")).toBe("true");
+      expect(parsed.searchParams.has("skip")).toBe(false);
+      expect(parsed.searchParams.has("alsoSkip")).toBe(false);
     });
 
     it("should preserve a pre-existing query string in the input URL", () => {
       const url = buildUrl({
         url: "https://api.example.com/items?keep=1",
         params: { chain: 2 },
       });
-      expect(url.searchParams.get("keep")).toBe("1");
-      expect(url.searchParams.get("chain")).toBe("2");
+      const parsed = new URL(url);
+      expect(parsed.searchParams.get("keep")).toBe("1");
+      expect(parsed.searchParams.get("chain")).toBe("2");
     });
 
     it("should percent-encode keys and values", () => {
       const url = buildUrl({
         url: "https://api.example.com/items",
         params: { "a key": "a value&b" },
       });
-      expect(url.toString()).toBe(
-        "https://api.example.com/items?a%20key=a%20value%26b",
+      expect(url).toBe("https://api.example.com/items?a%20key=a%20value%26b");
+    });
+
+    it("should not append a trailing slash after the last query value (facebook/react-native#54242)", () => {
+      const url = buildUrl({
+        url: "https://manager.api.live.ledger.com/api/get_device_version",
+        params: { target_id: 858783748, provider: 1 },
+      });
+      expect(typeof url).toBe("string");
+      expect(url).toBe(
+        "https://manager.api.live.ledger.com/api/get_device_version?target_id=858783748&provider=1",
       );
+      expect(url.endsWith("/")).toBe(false);
+    });
+
+    it("should return a plain string (not a URL instance) so RN URL serialization is bypassed", () => {
+      const url = buildUrl({
+        url: "https://api.example.com/items",
+        params: { chain: 1 },
+      });
+      expect(typeof url).toBe("string");
+      expect(url).not.toBeInstanceOf(URL);
     });
 
     it("should still build the URL when URLSearchParams.set is unavailable (React Native regression)", () => {
       const original = URLSearchParams.prototype.set;
-      // Simulate a runtime (e.g. some React Native versions) where
-      // URLSearchParams exists but `set` is not implemented.
       URLSearchParams.prototype.set = function notImplemented(): never {
         throw new Error("URLSearchParams.set is not implemented");
       };
@@ -113,10 +133,8 @@ describe("DmkNetworkClientHelpers", () => {
           url: "https://api.example.com/items",
           params: { chain: 1, contract: "0xabc" },
         });
-        // Reading `searchParams.get` in jsdom does not call `set`, so it is
-        // safe to assert against the parsed URL here.
-        expect(url.toString()).toContain("chain=1");
-        expect(url.toString()).toContain("contract=0xabc");
+        expect(url).toContain("chain=1");
+        expect(url).toContain("contract=0xabc");
       } finally {
         URLSearchParams.prototype.set = original;
       }

packages/device-management-kit/src/api/network/DmkNetworkClientHelpers.ts

@@ -44,43 +44,30 @@ function serializeParams(params: DmkQueryParams): string {
 }
 
 /**
- * Builds the final request URL from an input URL (absolute or relative), an
- * optional base URL, and optional query params. `null`/`undefined` param
- * values are skipped.
+ * Builds the final request URL as a plain string. `null`/`undefined` params
+ * are skipped; params are appended (not merged) when `url` already has a
+ * query string; fragments are not supported alongside `params`.
  *
- * Query params are serialized manually (see {@link serializeParams}) instead
- * of via `URL.searchParams`, because some runtimes (notably React Native)
- * ship a `URLSearchParams` whose mutation methods (`set`, `append`, …) are
- * not implemented.
- *
- * @remarks
- * Fragments (`#...`) in the input URL are not supported when `params` is
- * provided: the query string is concatenated after the fragment, so it ends
- * up parsed as part of `.hash` rather than `.search`. Callers should strip
- * or avoid fragments on URLs passed to this function.
- *
- * @remarks
- * If `url` already contains a query string, `params` are appended to it.
- * Keys present in both the existing query string and `params` are not merged:
- * both occurrences will appear in the final URL. Callers relying on override
- * semantics should pre-merge their parameters.
+ * Returns a string (not a `URL`) to avoid React Native's `URL.toString()`,
+ * which appends a stray trailing `/` to URLs with a query string on affected
+ * versions (facebook/react-native#54242).
  */
 export function buildUrl(args: {
   url: string;
   params?: DmkQueryParams;
   baseUrl?: string;
-}): URL {
+}): string {
   const { url, params, baseUrl } = args;
   const isAbsolute = /^[a-z][a-z0-9+.-]*:\/\//i.test(url);
   const composed = isAbsolute ? url : baseUrl ? joinPath(baseUrl, url) : url;
 
   const query = params ? serializeParams(params) : "";
   if (query.length === 0) {
-    return new URL(composed);
+    return composed;
   }
 
   const separator = composed.includes("?") ? "&" : "?";
-  return new URL(`${composed}${separator}${query}`);
+  return `${composed}${separator}${query}`;
 }
 
 /**