Sanitize user submitted content #195
Conversation
Utilize appendUntrusted() to safely escape any content from the user (e.g., malicious HTML or javascript).
|
Excellent! @nelsonic do you want me to make the updates? |
google-apps-script.js
Outdated
| // sanitize content from the user - trust no one | ||
| // ref: https://developers.google.com/apps-script/reference/html/html-output#appendUntrusted(String) | ||
| function cleanMe(theunclean) { | ||
| var output = HtmlService.createHtmlOutput(theunclean); |
There was a problem hiding this comment.
this line is not used.
google-apps-script.js
Outdated
| // for every key, concatenate an `<h4 />`/`<div />` pairing of the key name and its value, | ||
| // and append it to the `result` string created at the start. | ||
| } | ||
| return result; // once the looping is done, `result` will be one long string to put in the email body | ||
| } | ||
|
|
||
| // sanitize content from the user - trust no one | ||
| // ref: https://developers.google.com/apps-script/reference/html/html-output#appendUntrusted(String) | ||
| function cleanMe(theunclean) { |
There was a problem hiding this comment.
I agree with @nelsonic, let's try and clean up the names here. e.g. sanitizeFormResult(rawResult), or something along those lines
google-apps-script.js
Outdated
| // ref: https://developers.google.com/apps-script/reference/html/html-output#appendUntrusted(String) | ||
| function cleanMe(theunclean) { | ||
| var output = HtmlService.createHtmlOutput(theunclean); | ||
| var placeholder = HtmlService.createHtmlOutput(' '); |
There was a problem hiding this comment.
I would use an empty string here to "save space", granted that is probably negligible. Also, code convention in the repo is to use double quotes, i.e. ""
There was a problem hiding this comment.
Overall, I like the improvements made here. Thank you for the contribution!
As a note, this could break existing forms if people upgrade the server-side code. Namely, if they were using this feature to say, add images or custom HTML elements from the client and sending that to the server. I know of no one who has, but you could send all sorts of things, from images to possibly SVG visualizations with D3. shrug
The added security benefit outweighs this. Someone can "tear down the walls" or add exceptions if desired for their own projects. This processing could happen server-side too, which would be safer.
Overall, I think this is good to go after a few minor fixes I noted in-line. Feel free to make those @mhaddy , and this should be good to merge I think. Thanks!
Renamed function call, removed unused line, and aligned to project code standards.
|
@nelsonic @mckennapsean Please review the changes - I believe I've addressed all of your feedback. |
|
Thanks for cleaning that up! Will get it merged soon. |
|
Done, deployed in production. |
Utilize appendUntrusted() to safely escape any content from the user (e.g., malicious HTML or javascript).