control characters and Util::clean_text()

[David Miller]

Max Kanat-Alexander wrote on 12/21/05 11:59 AM:
> On Wed, 2005-12-21 at 14:09 +0100, Frédéric Buclin wrote:
>> So why 
>> not updating trim() to automatically remove such characters everywhere? 
>> This solution would be much less invasive.
> 
> 	I wouldn't object for the branches, but I'd definitely object for the
> tip. A developer expects a function called trim() to only remove
> whitespace. Functions should not have side effects.

I would think the other way around, because you're more likely to break 
people's customizations on the branches.  With a major version update, 
they expect to need to change things.

> 	I don't particularly see a pressing reason to remove control characters
> in most cases, anyhow -- if somebody was silly enough to put a control
> character into a field, perhaps they intended for it to appear there.
> (Unless, of course, displaying the control character has some security
> implication.)

There are security implications for any field which is included in email 
headers.  Allowing a linefeed lets you insert arbitrary email headers.

Of course, the least invasive (and probably most secure) way to fix this 
is to strip the control characters before putting things in the headers. :)

Technically, you're not allowed anything that's not US-ASCII in email 
headers, but that's another bug.

-- 
Dave Miller                                   http://www.justdave.net/
System Administrator, Mozilla Corporation      http://www.mozilla.com/
Project Leader, Bugzilla Bug Tracking System  http://www.bugzilla.org/