-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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.
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
Conversation
|
Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
| categories.value = tags.map((tag: any) => ({ | ||
| id: tag.key, | ||
| title: tag.name, // 国际化 |
There was a problem hiding this comment.
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:
-
Format Consistency: Add commas after elements of arrays (
folderId.value = id,filterList.value = null) and properties of objects ({id: tag.key}). -
Remove Unnecessary Call: Move the
getList()call into theasync setup(...)block so it gets called when the component mounts naturally without needing an explicitcall. -
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
categoriesstate. - Adding a check for empty lists before proceeding further.
- Converting snake_case fields like 'description' to camelCase.
- Updating comments accordingly to maintain documentation clarity.
| } | ||
| } catch (e) { | ||
| MsgError(t('views.applicationWorkflow.nodes.mcpNode.mcpServerTip')) | ||
| return |
There was a problem hiding this comment.
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.
chore: refactor ToolStoreDialog.vue to streamline onBeforeMount usage
chore: remove unnecessary check for empty storeTools in ToolStoreDialog
fix: validate JSON object in form submission