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

Felipe Contreras [off-list ref] writes:

On Sat, Feb 4, 2012 at 5:10 PM, Felipe Contreras
[off-list ref] wrote:

quoted

Otherwise, 'git send-email' would be happy to do:

 % git send-email --to '[off-list ref]>'

And use '[off-list ref]>' in the headers.

Er, actually that's not correct: '[off-list ref]>' will remain the
same, but 'Foo [off-list ref]>' will be sanitized.

I suspect that this "Er" is merely a sympotom of a larger issue in the
approach taken by this patch.  The code takes a potentially malformed
input, and applies a rewrite logic without telling the user what it is
doing.  If the rewrite logic is perfect, that may be OK, but if not, the
logic to rewrite may or may not trigger, or when it triggers it may or may
not produce a correct result, and it all depends on the nature of breakage
in the input.

Wouldn't a better approach to detect problem on the input side and reject
a wrong one by erroring out, so that the user has a chance to fix?

On Sun, Feb 5, 2012 at 11:12 PM, Junio C Hamano [off-list ref] wrote:

Felipe Contreras [off-list ref] writes:

quoted

On Sat, Feb 4, 2012 at 5:10 PM, Felipe Contreras
[off-list ref] wrote:

quoted

Otherwise, 'git send-email' would be happy to do:

 % git send-email --to '[off-list ref]>'

And use '[off-list ref]>' in the headers.

Er, actually that's not correct: '[off-list ref]>' will remain the
same, but 'Foo [off-list ref]>' will be sanitized.

I suspect that this "Er" is merely a sympotom of a larger issue in the
approach taken by this patch.  The code takes a potentially malformed
input, and applies a rewrite logic without telling the user what it is
doing.

That's what the function is doing already: sanitizing the address.

Wouldn't a better approach to detect problem on the input side and reject
a wrong one by erroring out, so that the user has a chance to fix?

Perhaps, but the code is not prepared for that. Anyway, feel free to
drop it, I am not interested in pursing this.

Cheers.

-- 
Felipe Contreras