All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Eric Sunshine <sunshine@sunshineco.com>
Cc: Jan Viktorin <viktorin@rehivetech.com>,
	Git List <git@vger.kernel.org>,
	"brian m. carlson" <sandals@crustytoothpaste.net>
Subject: Re: [PATCH v1] send-email: provide whitelist of SMTP AUTH mechanisms
Date: Sun, 02 Aug 2015 11:10:53 -0700	[thread overview]
Message-ID: <xmqq614xa7de.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <CAPig+cQwgYYYYsszaRdJDwFLLB0PmiDQ_WTa+Nzzoq0U1zuMiA@mail.gmail.com> (Eric Sunshine's message of "Sun, 2 Aug 2015 05:41:29 -0400")

Eric Sunshine <sunshine@sunshineco.com> writes:

> What I was really asking was whether this sort of checking really
> belongs in git-send-email or if it is better left to Net::SMTP (and
> Authen::SASL) to do so since they are in better positions to know what
> is valid and what is not. If the Perl module(s) generate suitable
> diagnostics for bad input, then it makes sense to leave the checking
> to them. If not, then I can understand your motivation for
> git-send-email doing the checking instead in order to emit
> user-friendly diagnostics.
>
> So, that's what I meant when I asked 'What are the consequences of not
> limiting the input to this "approved" list?'.
>
> The other reason I asked was that it increases maintenance costs for
> us to maintain a list of "approved" mechanisms, since the list needs
> to be updated when new ones are implemented (and, as brian pointed
> out, some may already exist which are not in your list).

Both are very good points.  I do not think we should be limiting the
user input; instead, we (1) either let the Perl module emit proper
diagnosis (e.g. it may say "There is no valid auth method in the
list you gave me, which was 'PLIAN LOGIN CROM-MD5'") and do nothing,
or (2) catch the error from the Perl module and then guess what
happened after the fact (e.g. the module may say in its die() message
something that is understandable by the program but not by the user,
and "eval { ... module call ... }; if ($@) { ... HERE ... }" would
massage what it can learn in HERE from $@ into what the end user
would understand.  It may be that it is sufficient to have something
as simple as this:

	my $msg = "$@"
	if ($smtp_auth is used to customize) {
		$msg .= "\nYour customized <$smtp_auth> might be misspelt?"
	}
	die $msg;

in "HERE" above.

> ...
> Maybe. This leads back to my original question of whether it's really
> git-send-email's responsibility to do validation or if that can be
> left to Net::SMTP/Authen::SASL. If the Perl module(s) emit suitable
> diagnostics for bad input, then validation can be omitted from
> git-send-email.

Yes, exactly.  What happens if we go the route of doing nothing
here?  What are the horrible diagnostic message the end user would
not understand?

  parent reply	other threads:[~2015-08-02 18:11 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-31 23:33 [PATCH v1] send-email: provide whitelist of SMTP AUTH mechanisms Jan Viktorin
2015-08-01  9:33 ` Eric Sunshine
2015-08-01 18:19   ` Jan Viktorin
2015-08-02  9:41     ` Eric Sunshine
2015-08-02 16:43       ` Jan Viktorin
2015-08-02 18:28         ` Junio C Hamano
2015-08-03 10:24           ` Jan Viktorin
2015-08-03 19:53             ` Junio C Hamano
2015-08-02 18:10       ` Junio C Hamano [this message]
2015-08-01 16:49 ` brian m. carlson
2015-08-01 18:21   ` Jan Viktorin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqq614xa7de.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=sandals@crustytoothpaste.net \
    --cc=sunshine@sunshineco.com \
    --cc=viktorin@rehivetech.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.