Re: [pgAdmin4][Patch][RM_2191] : Add support for the hostaddr connection parameter

Hi

There are still validation issues unfortunately; see the attached
screenshot.

I'm a little bit worried about the validation for the IP being so different
in Python vs. JS. We should really have both of course (Python for
defensive purposes, JS for usability), but it's clear they can lead to
different results. I think we should use the regexp approach in both cases
(because client-side tests against sockets are a) probably not possible,
and b) it's the server that matters - e.g. if the client doesn't support
v6, but the server does). Thoughts?

I've also attached an updated patch in which I tweaked a couple of things,
including docs. Please work from that.

Thanks!

On Friday, June 23, 2017, Atul Sharma <atul(dot)sharma(at)enterprisedb(dot)com> wrote:

> Hi Dave,
>
> Please find updated version attached.
>
> Regards,
> Atul
>
> On Fri, Jun 23, 2017 at 8:49 PM, Dave Page <dpage(at)pgadmin(dot)org
> <javascript:_e(%7B%7D,'cvml','dpage(at)pgadmin(dot)org');>> wrote:
>
>> Hi,
>>
>> I'm getting:
>>
>> (pgadmin4)piranha:pgadmin4 dpage$ git apply ~/Downloads/RM_2191_ver2.patch
>> error: cannot apply binary patch to
>> 'docs/en_US/images/server_advanced.png' without full index line
>> error: docs/en_US/images/server_advanced.png: patch does not apply
>>
>> when trying to apply. If memory serves, this normally happens if you
>> forget to use --binary when creating the diff.
>>
>> Can you send an updated version please?
>>
>> On Fri, Jun 23, 2017 at 3:59 PM, Atul Sharma
>> <atul(dot)sharma(at)enterprisedb(dot)com
>> <javascript:_e(%7B%7D,'cvml','atul(dot)sharma(at)enterprisedb(dot)com');>> wrote:
>> > Hi,
>> >
>> > Please find updated patch.
>> >
>> > Regards,
>> > Atul
>> >
>> > On Thu, Jun 22, 2017 at 5:02 PM, Dave Page <dpage(at)pgadmin(dot)org
>> <javascript:_e(%7B%7D,'cvml','dpage(at)pgadmin(dot)org');>> wrote:
>> >>
>> >> Hi
>> >>
>> >> On Thu, Jun 22, 2017 at 11:05 AM, Atul Sharma
>> >> <atul(dot)sharma(at)enterprisedb(dot)com
>> <javascript:_e(%7B%7D,'cvml','atul(dot)sharma(at)enterprisedb(dot)com');>> wrote:
>> >> > Hi,
>> >> >
>> >> > Please find attached patch for RM #2191 : Add support for hostaddr
>> >> > connection parameter
>> >>
>> >> Unfortunately there are a few issues with this patch:
>> >>
>> >> - It needs rebasing (blame Ashesh :-p )
>> >>
>> >> - It's missing the documentation update (and screenshot update)
>> >>
>> >> - I'm not sure the validation for a valid IPv6 address in
>> >> check_for_valid_ipv6 is correct. For example:
>> >>
>> >> /^(?:[A-F0-9]{1,4}:){7}[A-F0-9]{1,4}$/.test('::1');
>> >> false
>> >> /^(?:[A-F0-9]{1,4}:){7}[A-F0-9]{1,4}$/.test('fe80::187f:316f
>> :4bb8:8a3d');
>> >> false
>> >>
>> >> Both of those are valid addresses.
>> >>
>> >> Note that I've only eye-balled the patch so far, as I was unable to
>> >> apply it without manual work.
>> >>
>> >> Thanks!
>> >>
>> >> --
>> >> Dave Page
>> >> Blog: http://pgsnake.blogspot.com
>> >> Twitter: @pgsnake
>> >>
>> >> EnterpriseDB UK: http://www.enterprisedb.com
>> >> The Enterprise PostgreSQL Company
>> >
>> >
>>
>>
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>
>

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company