| From: | Heikki Linnakangas <hlinnaka(at)iki(dot)fi> | 
|---|---|
| To: | Noah Misch <noah(at)leadboat(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> | 
| Cc: | Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Daniele Varrazzo <daniele(dot)varrazzo(at)gmail(dot)com>, pgsql-bugs <pgsql-bugs(at)postgresql(dot)org> | 
| Subject: | Re: [PATCH] Fixed malformed error message on malformed SCRAM message. | 
| Date: | 2017-06-07 14:48:23 | 
| Message-ID: | 3cc82687-ee65-d0f0-efc2-ddad60ab206f@iki.fi | 
| Views: | Whole Thread | Raw Message | Download mbox | Resend email | 
| Thread: | |
| Lists: | pgsql-bugs pgsql-hackers | 
On 06/02/2017 09:32 AM, Noah Misch wrote:
> On Fri, Jun 02, 2017 at 02:20:00AM -0400, Tom Lane 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.
> 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.
- Heikki
| Attachment | Content-Type | Size | 
|---|---|---|
| 0001-Improve-authentication-error-messages.patch | application/x-download | 10.7 KB | 
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Joe Conway | 2017-06-07 15:45:20 | Re: BUG #14682: row level security not work with partitioned table | 
| Previous Message | David G. Johnston | 2017-06-07 14:36:45 | Re: BUG #14693: create materialized view forces btrim | 
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Amit Khandekar | 2017-06-07 14:49:00 | Re: UPDATE of partition key | 
| Previous Message | Robert Haas | 2017-06-07 14:43:16 | Re: Use of non-restart-safe storage by temp_tablespaces |