Code review comment for lp:~adri2000/merge-o-matic/dev

Revision history for this message
Kees Cook (kees) wrote :

Well, for the race conditions, I mean, what happens if two updates attempt
to land at the same time? There's no file locking to prevent it, from what
I can see. One might end up with interleaved data, lost comments, etc.

For the \n-removal, I would drop all characters less than 0x20. (And for
evilness injection, I recommend the firefox plugin "TamperData".)

Escaping is needed, if not for safety, just for avoiding bugs (e.g. "&"
would attempt to be turned into an entity). Best to use an existing
function to handle it. I would note that you need to use cgi.escape(s,
quote=True) in this case, to catch the quotes. (But you're right, the
quote-escaping was sufficient -- I think I was pondering a textbox, not an
inputbox.) :)

« Back to merge proposal