GNOME Bugzilla – Bug 344588
Incorrect unescaping of URLs in xmlIO
Last modified: 2006-10-11 20:14:37 UTC
The current code in __xmlOutputBufferCreateFilename only unescapes URIs that have a scheme, i.e. the absolute URLs. This is wrong because 1) the remote URLs must not be unescaped; 2) relative file URLs need unescaping. This is the cause of bug #337486 in libxslt. I propose a patch to unescape all local file URLs and leave others intact. Also, I found no unescaping code in xmlParserInputBufferCreateFilename which should behave symmetrically to the output counterpart. Is it intended?
Created attachment 67140 [details] [review] Patch for 2.6.26
Also, the code in this routine is too permissive in that it tries to create a file using the escaped URL as a file name if the attempt to create a file using the unescaped path fails. This creates an uncertainty with the actual output file path based on the state of the filesystem, and may present a security risk. I suggest making an if-else decision here bound to the logic of URI unescaping, which is corrected by the patch above. And maybe duplication of code in both branches is bad, since we know the unescaped case is only for the file: scheme.
w.r.t. patch in #1, this looks right, applied and commited, thanks ! w.r.t. comment in #2 I agree this is not perfect but unfortunately conversion from path -> URL -> XML processing -> URL -> path is a hazard, if there is '%20' in an input directory you will never be able to guess after the base computation using it whether the '%20' in the output filename should be interpreted as is or means ' '. Unfortunately there is no way to pick either one without checking the filesystem state, and that's basically what the code does. Daniel
This shows that promiscuity is bad. If we'd be able to demand that every file parameter is always treated as an URL, that ambiguity would not occur.
Well you can try to convert millions of Windows users to use / instead of \ as a first start ... good luck. Daniel