<div dir="ltr"><div>Hi Usama,</div><div>Thanks for feedback, updated patch is attached.</div><div>Please see comments inline.</div><div><br></div><div>Regards</div><div>Umar Hayat</div><div><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Mar 4, 2020 at 8:05 PM Muhammad Usama <<a href="mailto:m.usama@gmail.com">m.usama@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>Hi Umar,</div><div><br></div><div>Here are some of the comments on your patch on top of Peng's comments.</div><div><br></div><div>1- The patch breaks the -f short option, by removing the : (colon) after f from the optstring argument to getopt_long function</div><div><br></div>i.e<br>while ((opt = getopt_long(argc, argv, "hpmfi:u:", long_options, &optindex)) != -1)<br>should be<br>while ((opt = getopt_long(argc, argv, "hpmf:i:u:", long_options, &optindex)) != -1)</div></blockquote>Fixed.<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>2- The input file has no option to specify escape characters. For example, there is no way to specify a user name or password</div><div>in the input file that contains : (colon)</div></div></blockquote>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'</div><div class="gmail_quote">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. <br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>3- Specifying the input file disregards the "-m" "--md5auth" option</div><div><br></div></div></blockquote><div>Fixed. That was purposefully disregarded previously as mention in mentisbt-422, input-file mode as only for md5auth. </div><div>Now It honour -m flag and in absence of this it will print md5 value stdout.</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div></div><div>4- The patch is not considering the length of username and password buffers while copying into them</div><div><p style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;line-height:normal"><br></p><p style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;line-height:normal">+ strncpy(username, buf, pch-buf);<br>+ strncpy(password, pch+1, strlen(pch+1));<span style="color:rgb(34,34,34);font-family:Arial,Helvetica,sans-serif;font-size:small"> </span></p><p style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;line-height:normal"><span style="color:rgb(34,34,34);font-family:Arial,Helvetica,sans-serif;font-size:small"><br></span></p><p style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;line-height:normal"><span style="color:rgb(34,34,34);font-family:Arial,Helvetica,sans-serif;font-size:small">The len (3rd) argument of strncpy is intended to guard against the buffer overflow of the destination string.</span><br></p></div><div><br></div></div></blockquote><div>Fixed. </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div></div><div>Thanks</div><div>Best regards</div><div>Muhammad Usama</div><div><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Mar 4, 2020 at 11:03 AM Umar Hayat <<a href="mailto:m.umarkiani@gmail.com" target="_blank">m.umarkiani@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Hi Bo Peng,<div>Please find updated patch with following changes.</div><div>1. Trailing whitespaces warning removed</div><div>2. Documentation added in "doc/src/sgml/ref/pg_md5.sgml" with options update and example usage.</div><div><br></div><div>Regards,</div><div>Umar Hayat</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Mar 4, 2020 at 7:40 AM Bo Peng <<a href="mailto:pengbo@sraoss.co.jp" target="_blank">pengbo@sraoss.co.jp</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">hello,<br>
<br>
On Tue, 3 Mar 2020 14:13:33 +0500<br>
Umar Hayat <<a href="mailto:m.umarkiani@gmail.com" target="_blank">m.umarkiani@gmail.com</a>> wrote:<br>
<br>
> Hi Bo Peng,<br>
> Any feedback, If this looks ok to you? I should send the same for pg_enc<br>
> utility.<br>
<br>
Sorry for late response.<br>
<br>
First, when I apply this patch, I found some extra trailing spaces.<br>
<br>
-----------------<br>
$ git apply pg_md5_input_file.diff<br>
pg_md5_input_file.diff:85: trailing whitespace.<br>
static void <br>
warning: 1 line adds whitespace errors.<br>
-----------------<br>
<br>
Could you add documentation in "doc/src/sgml/ref/pg_md5.sgml" to <br>
explain how to use this option?<br>
<br>
> <br>
> Regards<br>
> Umar Hayat<br>
> Principal Software Engineer<br>
> EnterpriseDB: <a href="https://www.enterprisedb.com" rel="noreferrer" target="_blank">https://www.enterprisedb.com</a><br>
> <br>
> On Thu, Feb 13, 2020 at 9:54 AM Bo Peng <<a href="mailto:pengbo@sraoss.co.jp" target="_blank">pengbo@sraoss.co.jp</a>> wrote:<br>
> <br>
> > Thank you for your patch.<br>
> > I will review your patch.<br>
> ><br>
> > On Tue, 11 Feb 2020 16:45:50 +0500<br>
> > Umar Hayat <<a href="mailto:m.umarkiani@gmail.com" target="_blank">m.umarkiani@gmail.com</a>> wrote:<br>
> ><br>
> > > Hello Hackers,<br>
> > ><br>
> > > I saw "Add support for an user/password input file to pg_md5" enhancement<br>
> > > in PgPool-II TODO list<br>
> > > <<br>
> > <a href="https://pgpool.net/mediawiki/index.php/TODO#Add_support_for_an_user/password_input_file_to_pg_md5" rel="noreferrer" target="_blank">https://pgpool.net/mediawiki/index.php/TODO#Add_support_for_an_user/password_input_file_to_pg_md5</a><br>
> > >,<br>
> > > (mentisbt-422 <<a href="https://www.pgpool.net/mantisbt/view.php?id=422" rel="noreferrer" target="_blank">https://www.pgpool.net/mantisbt/view.php?id=422</a>>), so I<br>
> > > implemented this in pg_md5 utility. Please find attached patch for<br>
> > > enhancement.<br>
> > ><br>
> > > As suggested, a new --input-file option added in pg_md5. Using this<br>
> > option,<br>
> > > pg_md5 will parse the *user:password* pairs from provide file and will<br>
> > > create *user:md5xxxxxx* value in pool_passwd file.<br>
> > ><br>
> > > Regards,<br>
> > > Umar Hayat<br>
> ><br>
> ><br>
> > --<br>
> > Bo Peng <<a href="mailto:pengbo@sraoss.co.jp" target="_blank">pengbo@sraoss.co.jp</a>><br>
> > SRA OSS, Inc. Japan<br>
> ><br>
<br>
<br>
-- <br>
Bo Peng <<a href="mailto:pengbo@sraoss.co.jp" target="_blank">pengbo@sraoss.co.jp</a>><br>
SRA OSS, Inc. Japan<br>
</blockquote></div>
_______________________________________________<br>
pgpool-hackers mailing list<br>
<a href="mailto:pgpool-hackers@pgpool.net" target="_blank">pgpool-hackers@pgpool.net</a><br>
<a href="http://www.pgpool.net/mailman/listinfo/pgpool-hackers" rel="noreferrer" target="_blank">http://www.pgpool.net/mailman/listinfo/pgpool-hackers</a><br>
</blockquote></div></div>
</blockquote></div></div>