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

[Umar Hayat]

Hi Usama,
Any feedback ?

On Thu, Mar 19, 2020 at 7:07 AM Bo Peng <pengbo at sraoss.co.jp> wrote:

> Hello,
>
> On Wed, 18 Mar 2020 11:51:05 +0500
> Umar Hayat <m.umarkiani at gmail.com> wrote:
>
> > Hi,
> > Please updated patch based on feedback from Tatsu Ishii on pg_enc patch.
>
> I tested your patch and it works fine.
>
> Usama,
>
> If you don't have any feedback, I am going to update
> the patch based on Ishii-san's feedback and commit it.
>
> > Regards
> > Umar Hayat
> >
> > On Tue, Mar 10, 2020 at 3:39 PM Umar Hayat <m.umarkiani at gmail.com>
> wrote:
> >
> > > 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
> > >>>>
> > >>>
>
>
> --
> Bo Peng <pengbo at sraoss.co.jp>
> SRA OSS, Inc. Japan
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.sraoss.jp/pipermail/pgpool-hackers/attachments/20200330/3c5234c8/attachment.html>