fix(importer-curl): keep --data-urlencode whole and survive a stray percent (#597)

plugins/importer-curl/src/index.ts

@@ -506,7 +506,17 @@ function importCommand(parseEntries: string[], workspaceId: string) {
       form: multipartFormDataFromRaw,
     };
   } else if (dataParameters.length > 0 && bodyAsGET) {
-    urlParameters.push(...dataParameters);
+    // `-G` moves the data into the query string, and Yaak encodes url
+    // parameters on send exactly as it encodes the form body below, so this
+    // needs the same decode -- otherwise a `--data-urlencode` value arrives
+    // here already encoded and goes out encoded twice.
+    urlParameters.push(
+      ...dataParameters.map((parameter) => ({
+        ...parameter,
+        name: decodePercentEncoding(parameter.name),
+        value: decodePercentEncoding(parameter.value),
+      })),
+    );
   } else if (
     dataParameters.length > 0 &&
     (mimeType == null || mimeType === "application/x-www-form-urlencoded")
@@ -515,8 +525,8 @@ function importCommand(parseEntries: string[], workspaceId: string) {
     body = {
       form: dataParameters.map((parameter) => ({
         ...parameter,
-        name: decodeURIComponent(parameter.name || ""),
-        value: decodeURIComponent(parameter.value || ""),
+        name: decodePercentEncoding(parameter.name),
+        value: decodePercentEncoding(parameter.value),
       })),
     };
     filteredHeaders.push({
@@ -593,6 +603,34 @@ interface DataParameter {
   enabled?: boolean;
 }
 
+/**
+ * Decode a percent-encoded form value, keeping it as-is when it is not one.
+ *
+ * Yaak's form editor holds decoded values and re-encodes them on send, so a
+ * `-d` value has to be decoded on the way in. But curl sends that value
+ * verbatim and does not require it to be valid percent-encoding: `a=100%` is
+ * an ordinary form value, and `decodeURIComponent` throws URIError on it,
+ * which failed the whole import rather than that one parameter.
+ */
+function decodePercentEncoding(value: string | undefined): string {
+  const text = value || "";
+  try {
+    return decodeURIComponent(text);
+  } catch {
+    // Mixed: some of it is percent-encoded and some of it is a stray `%`.
+    // Returning the whole string untouched would leave the encoded part to be
+    // encoded a second time on send, so decode each valid run on its own and
+    // leave the stray byte alone. A run rather than a single escape, because a
+    // non-ASCII character is several escapes that only decode together.
+    return text.replace(/(%[0-9A-Fa-f]{2})+/g, (run) => {
+      try {
+        return decodeURIComponent(run);
+      } catch {
+        return run;
+      }
+    });
+  }
+}
 function pairsToDataParameters(keyedPairs: FlagsByName): DataParameter[] {
   const dataParameters: DataParameter[] = [];
 
@@ -605,7 +643,11 @@ function pairsToDataParameters(keyedPairs: FlagsByName): DataParameter[] {
 
     for (const p of pairs) {
       if (typeof p !== "string") continue;
-      const params = p.split("&");
+      // `-d` content really is `&`-separated, so splitting it is right. But
+      // `--data-urlencode` encodes its whole argument — an `&` inside it is
+      // data curl percent-encodes, not a separator, so splitting there turned
+      // one parameter into several and changed what the request sends.
+      const params = flagName === "data-urlencode" ? [p] : p.split("&");
       for (const param of params) {
         const [name, value] = splitOnce(param, "=");
         if (param.startsWith("@")) {

plugins/importer-curl/tests/index.test.ts

@@ -244,6 +244,107 @@ describe("importer-curl", () => {
     });
   });
 
+  test("Keeps an --data-urlencode value whole", () => {
+    // curl encodes the whole argument, so the `&` and the second `=` are data it
+    // percent-encodes, not separators. Splitting on them made two parameters
+    // out of one, and Yaak then re-sent `q=a&b=c` where curl sends
+    // `q=a%26b%3Dc`. One parameter here re-encodes back to what curl sends.
+    expect(convertCurl(`curl --data-urlencode 'q=a&b=c' https://yaak.app`)).toEqual({
+      resources: {
+        workspaces: [baseWorkspace()],
+        httpRequests: [
+          baseRequest({
+            method: "POST",
+            url: "https://yaak.app",
+            bodyType: "application/x-www-form-urlencoded",
+            headers: [
+              {
+                name: "Content-Type",
+                value: "application/x-www-form-urlencoded",
+                enabled: true,
+              },
+            ],
+            body: {
+              form: [{ name: "q", value: "a&b=c", enabled: true }],
+            },
+          }),
+        ],
+      },
+    });
+  });
+
+  test("Imports a data value that is not valid percent-encoding", () => {
+    // curl sends a `-d` value verbatim and does not require it to decode, so
+    // a lone `%` is an ordinary form value. decodeURIComponent threw URIError
+    // on it and failed the whole import.
+    expect(convertCurl(`curl -d 'a=100%' https://yaak.app`)).toEqual({
+      resources: {
+        workspaces: [baseWorkspace()],
+        httpRequests: [
+          baseRequest({
+            method: "POST",
+            url: "https://yaak.app",
+            bodyType: "application/x-www-form-urlencoded",
+            headers: [
+              {
+                name: "Content-Type",
+                value: "application/x-www-form-urlencoded",
+                enabled: true,
+              },
+            ],
+            body: {
+              form: [{ name: "a", value: "100%", enabled: true }],
+            },
+          }),
+        ],
+      },
+    });
+  });
+
+  test("Keeps a valid escape decoded when the value also holds a stray percent", () => {
+    // Handing the whole value back untouched would leave `%25` to be encoded a
+    // second time on send, so each valid run decodes on its own.
+    expect(convertCurl(`curl -d 'a=50%25 and 100%' https://yaak.app`)).toEqual({
+      resources: {
+        workspaces: [baseWorkspace()],
+        httpRequests: [
+          baseRequest({
+            method: "POST",
+            url: "https://yaak.app",
+            bodyType: "application/x-www-form-urlencoded",
+            headers: [
+              {
+                name: "Content-Type",
+                value: "application/x-www-form-urlencoded",
+                enabled: true,
+              },
+            ],
+            body: {
+              form: [{ name: "a", value: "50% and 100%", enabled: true }],
+            },
+          }),
+        ],
+      },
+    });
+  });
+
+  test("Decodes -G --data-urlencode into the query string", () => {
+    // `-G` puts the data in the query string, which is encoded on send just
+    // like the form body, so the value has to arrive here decoded or it goes
+    // out encoded twice.
+    expect(convertCurl(`curl -G --data-urlencode 'q=a&b' https://yaak.app`)).toEqual({
+      resources: {
+        workspaces: [baseWorkspace()],
+        httpRequests: [
+          baseRequest({
+            url: "https://yaak.app",
+            urlParameters: [{ name: "q", value: "a&b", enabled: true }],
+          }),
+        ],
+      },
+    });
+  });
+
   test("Imports data params as text", () => {
     expect(
       convertCurl("curl -H Content-Type:text/plain -d a -d b -d c=ccc https://yaak.app"),