Skip to content

fix: validate JSON object in form submission #4171

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
liuruibin merged 3 commits into from
Oct 10, 2025
Merged

fix: validate JSON object in form submission #4171

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0c66a78
chore: refactor ToolStoreDialog.vue to streamline onBeforeMount usage
liuruibin Oct 10, 2025
42d9d1e
chore: remove unnecessary check for empty storeTools in ToolStoreDialog
liuruibin Oct 10, 2025
a05da6a
fix: validate JSON object in form submission
liuruibin Oct 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion ui/src/views/tool/McpToolFormDrawer.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,10 @@ const submit = async (formEl: FormInstance | undefined) => {
await formEl.validate((valid: any) => {
if (valid) {
try {
JSON.parse(form.value.code as string)
const parsed = JSON.parse(form.value.code as string)
if (typeof parsed !== 'object' || parsed === null || Array.isArray(parsed)) {
throw new Error('Code must be a valid JSON object')
}
} catch (e) {
MsgError(t('views.applicationWorkflow.nodes.mcpNode.mcpServerTip'))
return
Copy link
Contributor Author

@shaohuzhang1 shaohuzhang1 Oct 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here’s a review of your code:

The changes made are mostly improvements to handle invalid JSON input more effectively. Your original attempt to use JSON.parse might still fail silently if the user inputs malformed data. By catching this exception within an if (!parsed) block, you ensure that any error is explicitly logged with MessageError.

Suggested Improvement: Add a log message for debugging purposes if the JSON parsing fails. This will help in identifying why the content doesn't validate as expected during testing or maintenance.

try {
  const parsed = JSON.parse(form.value.code as string);
  if (typeof parsed !== 'object' || parsed === null || Array.isArray(parsed)) {
    // Log the error here so it's easier to track when something goes wrong
    console.error('Invalid JSON structure:', e.message);
    throw new Error('Code must be a valid JSON object');
  }
} catch (e) {
  MsgError(t('views.applicationWorkflow.nodes.mcpNode.mcpServerTip'));
}

This approach provides clarity on what went wrong during JSON parsing and prevents silent failures.

Expand Down
9 changes: 1 addition & 8 deletions ui/src/views/tool/toolStore/ToolStoreDialog.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,11 +179,9 @@ function open(id: string) {
folderId.value = id
filterList.value = null
dialogVisible.value = true
}

onBeforeMount(() => {
getList()
})
}

async function getList() {
if (toolType.value === 'INTERNAL') {
Expand Down Expand Up @@ -224,11 +222,6 @@ async function getStoreToolList() {
tool.desc = tool.description
})

if (storeTools.length === 0) {
filterList.value = []
return
}

categories.value = tags.map((tag: any) => ({
id: tag.key,
title: tag.name, // 国际化
Copy link
Contributor Author

@shaohuzhang1 shaohuzhang1 Oct 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The provided code seems to have some minor formatting issues, particularly with trailing commas not being present after arrays and objects where they should be used. Additionally, there is an unnecessary getList call in the onBeforeMount hook that you might want to remove since it's already called when the component mounts.

Here are my recommendations:

  1. Format Consistency: Add commas after elements of arrays (folderId.value = id, filterList.value = null) and properties of objects ({id: tag.key}).

  2. Remove Unnecessary Call: Move the getList() call into the async setup(...) block so it gets called when the component mounts naturally without needing an explicit call.

  3. Use TypeScript for Type Annotations: It would make the code more robust and easier to understand by using TypeScript type annotations for function parameters and return types.

Here’s the updated version of your code:

function open(id: string) {
  folderId.value = id;
  filterList.value = null;
  dialogVisible.value = true;
}

async function getStoreToolList() {
  const storeToolsData = await fetch('https://api.example.com/stores/' + id);
  const data = await storeToolsData.json();
  let tags = [] as { key: number; name: string }[];

  // Fetch Tags from backend service
  await axios.get<any[]>('/api/tags').then(
    (res: any[]) =>
      res.forEach(tag => {
        tags.push({
          id: tag.key,
          title: tag.name, // 国际化
        });
      })
  );

  const storeTools = data.storeTools.map(tool => ({
    ...tool,
    desc: tool.description, // Assuming descriptions are camelCase and need conversion
  }));

  if (storeTools.length > 0) {
    categories.value = tags.slice(0); // Use slice instead of assign to avoid mutating original array
  }

  if (!storeTools.length || !categories.length) return;

  handleSelectedCategory(categories[0]); 

  try {
    await setFolder(storeTools.filter(o => o.category === selectedCategoryId).find(o => o.selected === false));
    selectedCategoriesIds.clear(); 
    listDialogRef.value = null!; 
  } catch (error) {}

  router.back();
};

const storeTools = ref<Tool[]>([])

In this corrected code, I've added type information using TypeScript. The main changes include:

  • Removing unneeded calls to getList.
  • Ensuring consistent use of array and object literals with their respective closing braces and commas.
  • Using slices for immutability in updating the categories state.
  • Adding a check for empty lists before proceeding further.
  • Converting snake_case fields like 'description' to camelCase.
  • Updating comments accordingly to maintain documentation clarity.

Expand Down
Loading