bug fix: Fix unintended consequences of adding quotes to parameter va…

…lues
FNNDSC · Jun 14, 2024 · a3866e6 · a3866e6
1 parent f35bd00
commit a3866e6
Show file tree

Hide file tree

Showing 5 changed files with 60 additions and 24 deletions.
diff --git a/src/api/common.ts b/src/api/common.ts
@@ -10,6 +10,7 @@ import {
   PipelinePipingDefaultParameterList,
   ComputeResource,
 } from "@fnndsc/chrisapi";
+import { quote } from "shlex";
 
 export function useSafeDispatch(dispatch: any) {
   const mounted = React.useRef(false);
@@ -436,3 +437,19 @@ export async function fetchNote(feed?: Feed) {
 export const pluralize = (file: string, length: number) => {
   return length === 1 ? file : `${file}s`;
 };
+
+export function needsQuoting(value: string) {
+  // Check if the value is already properly quoted
+  const singleQuoted = value.startsWith("'") && value.endsWith("'");
+  const doubleQuoted = value.startsWith('"') && value.endsWith('"');
+  const isProperlyQuoted = singleQuoted || doubleQuoted;
+
+  // If already properly quoted, return false
+  if (isProperlyQuoted) {
+    return false;
+  }
+
+  // If not quoted, check if quoting is necessary
+  const quotedValue = quote(value);
+  return quotedValue !== value;
+}
diff --git a/src/components/AddNode/AddNode.tsx b/src/components/AddNode/AddNode.tsx
@@ -50,7 +50,7 @@ const AddNode = ({
   const { goToNextStep: onNextStep } = useWizardContext();
 
   const isDisabled =
-    params && Object.keys(requiredInput).length !== params["required"].length;
+    params && Object.keys(requiredInput).length !== params.required.length;
 
   useEffect(() => {
     window.addEventListener("keydown", (event) => {

diff --git a/src/components/AddNode/GuidedConfig.tsx b/src/components/AddNode/GuidedConfig.tsx
@@ -1,11 +1,10 @@
-import { PluginInstanceParameter } from "@fnndsc/chrisapi";
 import type { Plugin, PluginParameter } from "@fnndsc/chrisapi";
+import { PluginInstanceParameter } from "@fnndsc/chrisapi";
 import {
   Button,
   Card,
   CardBody,
   Checkbox,
-  ClipboardCopy,
   Dropdown,
   DropdownItem,
   DropdownList,
@@ -22,11 +21,11 @@ import {
   TextInput,
   Tooltip,
 } from "@patternfly/react-core";
-import React, { useEffect, useContext, useState } from "react";
+import React, { useContext, useEffect, useState } from "react";
 import { useDispatch } from "react-redux";
 import { quote } from "shlex";
 import { v4 } from "uuid";
-import { fetchResource } from "../../api/common";
+import { fetchResource, needsQuoting } from "../../api/common";
 import { useTypedSelector } from "../../store/hooks";
 import { getParams } from "../../store/plugin/actions";
 import { ClipboardCopyFixed, ErrorAlert } from "../Common";
@@ -102,7 +101,7 @@ const GuidedConfig = () => {
         return key;
       });
 
-      if (params && defaultComponentList.length < params["dropdown"].length) {
+      if (params && defaultComponentList.length < params.dropdown.length) {
         defaultComponentList = [...defaultComponentList, v4()];
       }
       nodeDispatch({
@@ -180,8 +179,8 @@ const GuidedConfig = () => {
     );
   };
 
-  const requiredLength = params && params["required"].length;
-  const dropdownLength = params && params["dropdown"].length;
+  const requiredLength = params?.required.length;
+  const dropdownLength = params?.dropdown.length;
 
   return (
     <div className="configuration">
@@ -226,8 +225,8 @@ const GuidedConfig = () => {
               />
             </span>
             {params &&
-              params["required"].length > 0 &&
-              renderRequiredParams(params["required"])}
+              params.required.length > 0 &&
+              renderRequiredParams(params.required)}
           </div>
           <div>
             <span>
@@ -297,36 +296,41 @@ const CheckboxComponent = () => {
       const dropdownInput: { [id: string]: InputIndex } = {};
 
       const paramsRequiredFetched = params?.required.reduce(
-        (acc, param) => ({
-          ...acc,
-          [param.data.name]: [param.data.id, param.data.flag],
-        }),
+        (acc: any, param) => {
+          acc[param.data.name] = [param.data.id, param.data.flag];
+          return acc;
+        },
         {},
       );
 
       const paramsDropdownFetched = params?.dropdown.reduce(
-        (acc, param) => ({
-          ...acc,
-          [param.data.name]: param.data.flag,
-        }),
+        (acc: any, param) => {
+          acc[param.data.name] = param.data.flag;
+          return acc;
+        },
         {},
       );
 
       for (let i = 0; i < pluginParameters.length; i++) {
         const parameter = pluginParameters[i];
         const { param_name, type, value } = parameter.data;
-        if (paramsRequiredFetched && paramsRequiredFetched[param_name]) {
+        if (paramsRequiredFetched?.[param_name]) {
+          const quotedValue =
+            type === "string" && needsQuoting(value) ? quote(value) : value;
           const [id, flag] = paramsRequiredFetched[param_name];
           requiredInput[id] = {
-            value: type === "string" ? quote(value) : value,
+            value: quotedValue,
             flag,
             type,
             placeholder: "",
           };
         } else if (paramsDropdownFetched) {
+          const quotedValue =
+            type === "string" && needsQuoting(value) ? quote(value) : value;
+
           const flag = paramsDropdownFetched[param_name];
           dropdownInput[v4()] = {
-            value: type === "string" ? quote(value) : value,
+            value: quotedValue,
             flag,
             type,
             placeholder: "",

diff --git a/src/components/CreateFeed/createFeedHelper.ts b/src/components/CreateFeed/createFeedHelper.ts
@@ -177,6 +177,17 @@ export const createFeedInstanceWithFS = async (
   }
 };
 
+const stripQuotes = (value: string) => {
+  if (typeof value === "string") {
+    const singleQuoted = value.startsWith("'") && value.endsWith("'");
+    const doubleQuoted = value.startsWith('"') && value.endsWith('"');
+    if (singleQuoted || doubleQuoted) {
+      return value.slice(1, -1);
+    }
+  }
+  return value;
+};
+
 export const getRequiredObject = async (
   dropdownInput: InputType,
   requiredInput: InputType,
@@ -219,7 +230,7 @@ export const getRequiredObject = async (
       const flag = params[i].data.flag;
       const defaultValue = params[i].data.default;
       if (Object.keys(nodeParameter).includes(flag)) {
-        let value: string | boolean = nodeParameter[flag].value;
+        let value: string | boolean = stripQuotes(nodeParameter[flag].value);
         const type = nodeParameter[flag].type;
 
         if (value === "" && type === "boolean") {

diff --git a/src/components/NodeDetails/NodeDetails.tsx b/src/components/NodeDetails/NodeDetails.tsx
@@ -13,6 +13,8 @@ import {
 import React, { Fragment, ReactNode } from "react";
 import { ErrorBoundary } from "react-error-boundary";
 import { useNavigate } from "react-router";
+import { quote } from "shlex";
+import { needsQuoting } from "../../api/common";
 import { useTypedSelector } from "../../store/hooks";
 import { SpinContainer } from "../Common";
 import { isPlVisualDataset } from "../DatasetRedirect/getDatasets";
@@ -24,7 +26,6 @@ import PluginTitle from "./PluginTitle";
 import Status from "./Status";
 import StatusTitle from "./StatusTitle";
 import { getErrorCodeMessage } from "./utils";
-import { quote } from "shlex";
 
 interface INodeState {
   plugin?: Plugin;
@@ -299,9 +300,12 @@ function getCommand(
         const isString = instanceParameters[i].data.type === "string";
         const value = instanceParameters[i].data.value;
 
+        const safeValue =
+          isString && needsQuoting(value) ? quote(value) : value;
+
         modifiedParams.push({
           name: pluginParameters[j].data.flag,
-          value: isBoolean ? " " : isString ? quote(value) : value,
+          value: isBoolean ? " " : isString ? safeValue : value,
         });
       }
     }