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

Revision history for this message
Adrien Cunin (adri2000) wrote :

Could you elaborate a bit on the race conditions?

For \n, well, you're right, that reminds me of multi-lines spam that sometimes affects DaD. I did some tests and managed to reproduce that in w3m, but I used ^M (Ctrl+V and Ctrl+M) which actually is \r not \n. The result was just... DoS: "Internal Server Error"; the code doesn't like when a line of the comments file doesn't have ':'. Anyway I will fix that, can you just confirm that I should strip out \r (and not \n) ?

And finally, yes, I've heard of cgi.escape but thought is was unnecessary to use it. The comments are always displayed in a text box, something like <input type="text" value="$the_comment_here" />, and never directly on the page where code could be interpreted. In this context, I can't think of, apart from the double quotes character and newline, a character that could break something or make it possible to get some code interpreted. If I'm missing something, please tell me! :)

« Back to merge proposal