[pgpool-hackers: 3542] Re: [PATCH] Feature: Add support for an user/password input file to pg_md5

[Umar Hayat]

Any further updated needed for this patch from my end?

Regards
Umar Hayat

On Thu, Mar 5, 2020 at 1:45 AM Umar Hayat <m.umarkiani at gmail.com> wrote:

> Hi Usama,
> Thanks for feedback, updated patch is attached.
> Please see comments inline.
>
> Regards
> Umar Hayat
>
>
> On Wed, Mar 4, 2020 at 8:05 PM Muhammad Usama <m.usama at gmail.com> wrote:
>
>> Hi Umar,
>>
>> Here are some of the comments on your patch on top of Peng's comments.
>>
>> 1- The patch breaks the -f short option, by removing the : (colon) after
>> f from the optstring argument to getopt_long function
>>
>> i.e
>> while ((opt = getopt_long(argc, argv, "hpmfi:u:", long_options,
>> &optindex)) != -1)
>> should be
>> while ((opt = getopt_long(argc, argv, "hpmf:i:u:", long_options,
>> &optindex)) != -1)
>>
> Fixed.
>
>> 2- The input file has no option to specify escape characters. For
>> example, there is no way to specify a user name or password
>> in the input file that contains : (colon)
>>
> It does handle colon in password as it consider only left most colon as
> separator. For example for pair like 'user:pass:word', user name will be
> 'user' and password will be 'pass:word'
> I am not sure user name with colon is valid scenario? If its true, we can
> add a note otherwise IMO there could be ton of negative scenarios.
>
>> 3- Specifying the input file disregards the "-m" "--md5auth" option
>>
>> Fixed. That was purposefully disregarded  previously as mention
> in mentisbt-422, input-file mode as only for md5auth.
> Now It honour -m flag and in absence of this it will print md5 value
> stdout.
>
>> 4- The patch is not considering the length of username and password
>> buffers while copying into them
>>
>>
>> +               strncpy(username, buf, pch-buf);
>> +               strncpy(password, pch+1, strlen(pch+1));
>>
>>
>> The len (3rd) argument of strncpy is intended to guard against the buffer
>> overflow of the destination string.
>>
>> Fixed.
>
>> Thanks
>> Best regards
>> Muhammad Usama
>>
>>
>> On Wed, Mar 4, 2020 at 11:03 AM Umar Hayat <m.umarkiani at gmail.com> wrote:
>>
>>> Hi Bo Peng,
>>> Please find updated patch with following changes.
>>> 1. Trailing whitespaces warning removed
>>> 2. Documentation added in "doc/src/sgml/ref/pg_md5.sgml" with options
>>> update and example usage.
>>>
>>> Regards,
>>> Umar Hayat
>>>
>>> On Wed, Mar 4, 2020 at 7:40 AM Bo Peng <pengbo at sraoss.co.jp> wrote:
>>>
>>>> hello,
>>>>
>>>> On Tue, 3 Mar 2020 14:13:33 +0500
>>>> Umar Hayat <m.umarkiani at gmail.com> wrote:
>>>>
>>>> > Hi Bo Peng,
>>>> > Any feedback, If this looks ok to you? I should send the same for
>>>> pg_enc
>>>> > utility.
>>>>
>>>> Sorry for late response.
>>>>
>>>> First, when I apply this patch, I found some extra trailing spaces.
>>>>
>>>> -----------------
>>>> $ git apply pg_md5_input_file.diff
>>>> pg_md5_input_file.diff:85: trailing whitespace.
>>>> static void
>>>> warning: 1 line adds whitespace errors.
>>>> -----------------
>>>>
>>>> Could you add documentation in "doc/src/sgml/ref/pg_md5.sgml" to
>>>> explain how to use this option?
>>>>
>>>> >
>>>> > Regards
>>>> > Umar Hayat
>>>> > Principal Software Engineer
>>>> > EnterpriseDB: https://www.enterprisedb.com
>>>> >
>>>> > On Thu, Feb 13, 2020 at 9:54 AM Bo Peng <pengbo at sraoss.co.jp> wrote:
>>>> >
>>>> > > Thank you for your patch.
>>>> > > I will review your patch.
>>>> > >
>>>> > > On Tue, 11 Feb 2020 16:45:50 +0500
>>>> > > Umar Hayat <m.umarkiani at gmail.com> wrote:
>>>> > >
>>>> > > > Hello Hackers,
>>>> > > >
>>>> > > > I saw "Add support for an user/password input file to pg_md5"
>>>> enhancement
>>>> > > > in PgPool-II TODO list
>>>> > > > <
>>>> > >
>>>> https://pgpool.net/mediawiki/index.php/TODO#Add_support_for_an_user/password_input_file_to_pg_md5
>>>> > > >,
>>>> > > > (mentisbt-422 <https://www.pgpool.net/mantisbt/view.php?id=422>),
>>>> so I
>>>> > > > implemented this in pg_md5 utility. Please find attached patch for
>>>> > > > enhancement.
>>>> > > >
>>>> > > > As suggested, a new --input-file option added in pg_md5. Using
>>>> this
>>>> > > option,
>>>> > > > pg_md5 will parse the *user:password* pairs from provide file and
>>>> will
>>>> > > > create *user:md5xxxxxx* value in pool_passwd file.
>>>> > > >
>>>> > > > Regards,
>>>> > > > Umar Hayat
>>>> > >
>>>> > >
>>>> > > --
>>>> > > Bo Peng <pengbo at sraoss.co.jp>
>>>> > > SRA OSS, Inc. Japan
>>>> > >
>>>>
>>>>
>>>> --
>>>> Bo Peng <pengbo at sraoss.co.jp>
>>>> SRA OSS, Inc. Japan
>>>>
>>> _______________________________________________
>>> pgpool-hackers mailing list
>>> pgpool-hackers at pgpool.net
>>> http://www.pgpool.net/mailman/listinfo/pgpool-hackers
>>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.sraoss.jp/pipermail/pgpool-hackers/attachments/20200310/b7e8254b/attachment.html>