Обсуждение: some SCRAM read_any_attr() confusion

Поиск
Список
Период
Сортировка

some SCRAM read_any_attr() confusion

От
Peter Eisentraut
Дата:
I was a bit confused by some of the comments around the SCRAM function
read_any_attr(), used to skip over extensions.

The comment "Returns NULL if there is attribute.", besides being
strangely worded, appears to be wrong anyway, because the function never
returns NULL.

This lead me to wonder how this loop would terminate if there is no "p"
attribute in the message:

    /* ignore optional extensions */
    do
    {
        proof = p - 1;
        value = read_any_attr(&p, &attr);
    } while (attr != 'p');

What actually happens is

ERROR:  malformed SCRAM message
DETAIL:  Attribute expected, but found invalid character "0x00".

which serves the purpose but was probably not quite intended that way.

I propose the attached patch to clean this up a bit, with better
comments and a better error message.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Вложения

Re: some SCRAM read_any_attr() confusion

От
Michael Paquier
Дата:
On Sat, Aug 17, 2019 at 10:11:27AM +0200, Peter Eisentraut wrote:
> I was a bit confused by some of the comments around the SCRAM function
> read_any_attr(), used to skip over extensions.
>
> The comment "Returns NULL if there is attribute.", besides being
> strangely worded, appears to be wrong anyway, because the function never
> returns NULL.

This may have come from an incorrect merge with a past version when
this code has been developed.  +1 for me for your suggestions and your
patch.
--
Michael
Вложения

Re: some SCRAM read_any_attr() confusion

От
Peter Eisentraut
Дата:
On 2019-08-17 14:57, Michael Paquier wrote:
> On Sat, Aug 17, 2019 at 10:11:27AM +0200, Peter Eisentraut wrote:
>> I was a bit confused by some of the comments around the SCRAM function
>> read_any_attr(), used to skip over extensions.
>>
>> The comment "Returns NULL if there is attribute.", besides being
>> strangely worded, appears to be wrong anyway, because the function never
>> returns NULL.
> 
> This may have come from an incorrect merge with a past version when
> this code has been developed.  +1 for me for your suggestions and your
> patch.

committed

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services