On 05/25/2010 10:54 AM, Daniel P. Berrange wrote: > On Tue, May 25, 2010 at 08:42:31AM -0600, Eric Blake wrote: >> On 05/24/2010 12:52 PM, Cole Robinson wrote: >>> + >>> + /* Need to sanitize: >>> + * // -> // >>> + * /// -> / >>> + * /../foo -> /../foo >>> + * /.//foo -> /foo >>> + * /foo///bar/ -> /foo/bar >>> + * ./foo/./. -> /foo >>> + */ >>> + >> >> For my second attempt at a valid review, I actually compiled the >> function, and threw the above inputs at it. /../foo -> /./foo (oops, >> didn't match documentation), and ./foo/./. -> /foo (oops, matched >> documentation, but turned a relative path into absolute), so we do need >> a v4, but not for the original reasons in my first NAK where I mis-read >> the do-while loop. > > This function is crying out for a real test case to be written and put > under tests/, feeding it all sorts of evil input. > Agreed, however I'm backtracking a bit on this, my next post is only going to sanitize spurious /, like my original posting (but with the original comments addressed). A user can accidentally add an extra slash to a pool target, and its reasonable to expect it won't cause cause problems, but the same can't be said for relative path characters. This function is becoming impossible to review (the current posting goes into infinite loop with a path like /foo./bar), and most of the logic is not helping solve a real world problem. This patch is going to be backported to a few places: less complex the better. Oh, and I'm lazy :) - Cole -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list