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

[Umar Hayat]

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/20200305/f1029b73/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pg_md5_input_file_with_docs_plus_fixes.diff
Type: application/octet-stream
Size: 7964 bytes
Desc: not available
URL: <http://www.sraoss.jp/pipermail/pgpool-hackers/attachments/20200305/f1029b73/attachment.obj>