Re: [PATCH v2] send-email: provide whitelist of SMTP AUTH mechanisms

From: Eric Sunshine <sunshine@sunshineco.com>
To: Jan Viktorin <viktorin@rehivetech.com>
Cc: Git List <git@vger.kernel.org>,
	"brian m. carlson" <sandals@crustytoothpaste.net>,
	Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH v2] send-email: provide whitelist of SMTP AUTH mechanisms
Date: Mon, 10 Aug 2015 19:43:24 -0400	[thread overview]
Message-ID: <CAPig+cSU_rgRvfETfY7TiY6X0B6Tt6N0+GMVgSsYH-6zMteAgg@mail.gmail.com> (raw)
In-Reply-To: <20150810120642.2a0baac2@jvn>

On Mon, Aug 10, 2015 at 6:06 AM, Jan Viktorin <viktorin@rehivetech.com> wrote:
> On Sun, 9 Aug 2015 14:13:33 -0400
> Eric Sunshine <sunshine@sunshineco.com> wrote:
>> One possibility which comes to mind is to create a fake
>> Authen::SASL::Perl which merely dumps its input mechanisms to a file,
>> and arrange for the Perl search path to find the fake one instead. You
>> could then check the output file to see if it reflects your
>> expectations. However, this may be overkill and perhaps not worth the
>> effort (especially if you're not a Perl programmer).
>
> I think that Authen::SASL::Perl mock would not help. I wanted to create
> some fake sendmail (but this is impossible as stated above because
> then the perl modules are not used). So the only way would be to
> provide some fake socket with a static content on the other side. This
> is really an overkill to just test the few lines of code.

Agreed.

> So, what more can I do for this feature?

I don't have any further suggestions. Other than the unwanted
"Supported:" line in the documentation and the couple style issues[1],
the patch seems sufficiently complete, as-is. The validation regex
gets a "meh" from me merely because it's not clear how beneficial it
will be in practice, but that's not an outright objection; I don't
feel strongly about it either way.

[1]: http://article.gmane.org/gmane.comp.version-control.git/275150

> I think that the basic regex test is OK. It can accept lowercase
> letters and do an explicit uppercase call. I do not like to rely on
> internals of the SASL library. As you could see, the SASL::Perl does
> not check its inputs in a very good way and its code is quite unclear
> (strange for a library providing security features).