Netocrat wrote:
On Mon, 28 Nov 2005 12:40:56 -0800, Brion Vibber
wrote:
Netocrat wrote:
[incorrectly patching
Title.php/getLocalURL() and getFullURL() to encode
ampersands]
This patch is incorrect, and will cause broken
URLs to be output
throughout the wiki.
Too hasty - I didn't notice that there were already escaped versions of
these functions and that unescaped versions were necessary.
As a general rule, escaping should be done as close as possible to the actual
use. It's part of the file format or communications protocol in your output, not
a part of your data, so you want to keep them separate to avoid unnecessarily
hobbling code that should be more flexible.
URLs may go to different outputs, not always HTML; for instance HTTP headers,
plaintext output, generated JavaScript code, etc, will all have different
requirements from XML/HTML text.
Early escaping is also dangerous, since additional processing or just forgetting
where you got some value can end up producing either corrupt data (such as
double-escaping) or security vulnerabilities from missing escaping (SQL
injection, HTML/JavaScript injection, etc).
Instead, you
should locate the individual *output* of the bad URL that
you found and patch *that* to properly HTML-encode its output.
I based part of the extension on code from BoardVote.php, which doesn't
use the escaped url function to generate the action of a form. So the
patch is not very significant anyhow but this is what it should have been:
[patch
snipped]
Thanks, I've applied it to CVS.
-- brion vibber (brion @
pobox.com)