Escape or URL encode paths where necessary#159
Conversation
b5a218a to
dd279c6
Compare
When running in non-MultiViews mode path passed from browser is prematurely escaped for XML although it is still in use in PHP. This will result in two issues: * Broken paths are processed in PHP * Data returned to users is broken or double escaped This closes #155
7d5d4d0 to
e329b8c
Compare
| $vars['rev'] = $rev; | ||
| $vars['peg'] = $peg; | ||
| $vars['path'] = escape($ppath); | ||
| $vars['path'] = str_replace('%2F', '/', rawurlencode($ppath)); |
There was a problem hiding this comment.
This looks confusing to me: I would have expected path to be a plain path, not an encoded URL. What's the reason for doing so? Is path always used in some URL-context? Wouldn't it be better to distinguish the plain path from an encoded URL using different variable names, something like urlPath or pathAsUrl or alike? Pretty much like safepath currently.
There was a problem hiding this comment.
I can explain that. If you look at the tempates: websvn:path is solely used iwth href attributes which require URI encoding. Paths which appear as text are prefixed with safe since they aren't usable in href attributes. Thhe code did not properly address this and assumed that files would be 7 bit ASCII only.
There was a problem hiding this comment.
So how about renaming path to something mentioning it to be a URL instead? Too risky/too much work or simple global search&replace? safepath seems badly named as well and is more like a htmlPath.
There was a problem hiding this comment.
I agree with the second that safePath should have been htmlPath. I have just reused the given term.
As far as path is concerned, the name is too generic, I agree, but it can never be a URL because the base URL is provided through a var as well. Moreover, I would make the diff even bigger and affect all templates. I would consider this in a seoncd for for the sake of simplicity.
ams-tschoening
left a comment
There was a problem hiding this comment.
I won't have the time to test this in detail, so let's end naming discussions and use what works for you.
I did testing with all templates and affected PHP files with multi views and non-MV, I hope I haven't miss anything. I will leave the reporter a few more days to test and then will merge. |
When running in non-MultiViews mode path passed from browser is prematurely
escaped for XML although it is still in use in PHP. This will result in two
issues:
This closes #155