Subject: Re: [PATCH] send-email: add extra safetly in address sanitazion

From: Thomas Rast <[email-masked]>
Date: Sun, 5 Feb 2012 22:51:32 +0100

Felipe Contreras <[email-masked]> writes:

> 2012/2/5 Thomas Rast <[email-masked]>:
>> Felipe Contreras <[email-masked]> writes:
>>>
>>>   'Foo Bar <[email-masked]>' -> 'Foo Bar <[email-masked]>'
>>>   '"Foo Bar" <[email-masked]>' -> '"Foo Bar" <[email-masked]>'
>>>   [email-masked]' -> [email-masked]'
>>>   '<[email-masked]>' -> [email-masked]'
>>>   'Foo Bar' -> 'Foo Bar'
>>
>> Am I the only one who stared at this for ten seconds, only to then
>> realize that there is no sanitizing whatsoever going on here?
>
> There is: '<[email-masked]>' -> [email-masked]'

Indeed.

I still feel cheated as a reader though, you showed me four examples of
no change but let me figure that on my own.

>>>   'Foo Bar <[email-masked]>>' -> 'Foo Bar <[email-masked]>'
>>>   '"Foo Bar" <[email-masked]>>' -> '"Foo Bar" <[email-masked]>'
>>>   '<[email-masked]>>' -> [email-masked]'
>>
>> All of these are the same underlying issue.  Does your patch fix any
>> other malformed addresses, or just this particular type?
>
> See above.

Ok, I see I am falling into the same communication trap as Jonathan, so
let's be more explicit.

Your commit message first tells me you are going to sanitize something,
but starts out with examples of leaving the string unchanged.  Then it
continues with only the '>>' examples.

Today, and being someone who on average reads about half the mail that
comes through here, I know that this relates to the blame -e '>>' bug.
So today, I am wondering from the commit message why you narrowly focus
on that bug.  But you don't!  It's just that the commit message
insinuates it.

In a year, your reader (and bear in mind that this may very well be
yourself, at least if your memory is as good as mine) will wonder what
was so damn special about that '>>' string that it needs a specific fix
to send-email.

I see that you wrote in another thread:

> I have to write a peer-reviewed essay with an introduction for the
> people that are not familiar with the code in each of the patches

I'm not sure you meant it that literally, but the whole *point* is that
the message is for people who are not familiar with the code.  After
all, if I knew that your code did the right thing in the right way, I
would not be bothering with reading the message.  Today, I would just
send an Acked-by instead.  In a year, I'd scroll down for another
potential culprit for the bug I'm hunting.

What's especially striking me about your proposed messages of late: they
leave me with more open questions than I started with.  I tried to show
this above.  I'm not sure whether other contributors are better at
answering questions, or just better at not touching any topics that
might raise them.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch