-
Notifications
You must be signed in to change notification settings - Fork 902
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
fix: download responses not working #2205
fix: download responses not working #2205
Conversation
@gupta-piyush19 is attempting to deploy a commit to the formbricks Team on Vercel. A member of the Team first needs to authorize it. |
Thank you for following the naming conventions for pull request titles! 🙏 |
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.
@gupta-piyush19 thank you, looks great! 😊💪
Many thanks for the quick fix. I would suggest to make the function getResponsesFileName even more robust against path traversal attacks (e.g. a user could download files he should not have access to)?
|
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.
I would suggest to make the function getResponsesFileName even more robust against path traversal attacks by removing all characters from the filename that could cause issues and also limit the length of the name (to resolve OS specific issues with really long file names)
@@ -307,6 +307,8 @@ export const buildWhereClause = (filterCriteria?: TResponseFilterCriteria) => { | |||
}; | |||
|
|||
export const getResponsesFileName = (surveyName: string, extension: string) => { | |||
// replacing / with : to avoid url issues | |||
surveyName = surveyName.replaceAll("/", ":"); |
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.
// sanitize surveyName by removing all characters from the survey name that could cause issues & add max length
surveyName = surveyName.replace(/[^0-9a-zA-Z\-._]+/g, "_").substring(0, 255);
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.
Yes, it makes sense. Thank you for the suggestions. I'll update the PR with this new filename function.
What does this PR do?
fix download responses not working
Fixes #2199
How should this be tested?
/
in the survey nameChecklist
Required
pnpm build
console.logs
git pull origin main
Appreciated