Re: [PATCH] Fixed malformed error message on malformed SCRAM message.

On 06/08/2017 03:07 AM, Michael Paquier wrote:
> On Wed, Jun 7, 2017 at 11:48 PM, Heikki Linnakangas <hlinnaka(at)iki(dot)fi> wrote:
>> On 06/02/2017 09:32 AM, Noah Misch wrote:
>>>> BTW, since you mention COMMERROR uses in auth.c, isn't the usage at
>>>> line 687 wrong? It sure looks like the author supposed that that
>>>> ereport call wouldn't return, but it will. Adjacent similar calls
>>>> clean up and return NULL.
>>>
>>> Probably, though one could argue for proceeding with the short password.
>>> Deserves a comment if log-only is intentional.
>>
>> Let's turn it into an ERROR.
>
> Shouldn't that portion be back-patched?

Yeah, perhaps. But since it's not actively broken, nothing particularly
bad happens with the code as it is, I think I'm not going to bother.

>>> The lack of an exit after COMMERROR "client selected an invalid SASL
>>> authentication mechanism" looks like a bug.
>>
>> Yes. That was fixed in commit 505b5d2f86 already.
>>
>> Taking all the comments in this thread into account, and a few more things
>> that I spotted while looking at the error messages, I came up with the
>> attached patch. It includes the changes from Michael's patch upthread to use
>> errdetail() in the SCRAM errors, and it turns the protocol violation errors
>> in auth.c from COMMERROR into ERROR. See commit message for more details.
>> Barring objections, I'll push this tomorrow.
>
> Thanks for the new version. No additional comments from me.

Ok, committed, thanks!

- Heikki