Обсуждение: [pgAdmin4][Patch][RM_2191] : Add support for the hostaddr connection parameter
Hi,
Please find attached patch for RM #2191 : Add support for hostaddr connection parameter
Thanks,
Atul
Вложения
Hi On Thu, Jun 22, 2017 at 11:05 AM, Atul Sharma <atul.sharma@enterprisedb.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
Re: [pgAdmin4][Patch][RM_2191] : Add support for the hostaddrconnection parameter
От
Atul Sharma
Дата:
Hi,
Please find updated patch.
Regards,
Atul
On Thu, Jun 22, 2017 at 5:02 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi
On Thu, Jun 22, 2017 at 11:05 AM, Atul Sharma
<atul.sharma@enterprisedb.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
Вложения
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.sharma@enterprisedb.com> wrote: > Hi, > > Please find updated patch. > > Regards, > Atul > > On Thu, Jun 22, 2017 at 5:02 PM, Dave Page <dpage@pgadmin.org> wrote: >> >> Hi >> >> On Thu, Jun 22, 2017 at 11:05 AM, Atul Sharma >> <atul.sharma@enterprisedb.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
Re: [pgAdmin4][Patch][RM_2191] : Add support for the hostaddrconnection parameter
От
Atul Sharma
Дата:
Hi Dave,
Please find updated version attached.
Regards,
Atul
On Fri, Jun 23, 2017 at 8:49 PM, Dave Page <dpage@pgadmin.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.sharma@enterprisedb.com> wrote:
> Hi,
>
> Please find updated patch.
>
> Regards,
> Atul
>
> On Thu, Jun 22, 2017 at 5:02 PM, Dave Page <dpage@pgadmin.org> wrote:
>>
>> Hi
>>
>> On Thu, Jun 22, 2017 at 11:05 AM, Atul Sharma
>> <atul.sharma@enterprisedb.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
Вложения
Hi
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
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.sharma@enterprisedb.com> wrote:
On Friday, June 23, 2017, Atul Sharma <atul.sharma@enterprisedb.com> wrote:
Hi Dave,Please find updated version attached.Regards,AtulOn Fri, Jun 23, 2017 at 8:49 PM, Dave Page <dpage@pgadmin.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.sharma@enterprisedb.com> wrote:
> Hi,
>
> Please find updated patch.
>
> Regards,
> Atul
>
> On Thu, Jun 22, 2017 at 5:02 PM, Dave Page <dpage@pgadmin.org> wrote:
>>
>> Hi
>>
>> On Thu, Jun 22, 2017 at 11:05 AM, Atul Sharma
>> <atul.sharma@enterprisedb.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
Вложения
Re: [pgAdmin4][Patch][RM_2191] : Add support for the hostaddrconnection parameter
От
Atul Sharma
Дата:
Hi Dave,
Yes, It makes sense to use the same approach at both ends. I have modified the server file to use the same regex approach.
Attached is the patch for the same. Please review.
Regards,
Atul
On Sun, Jun 25, 2017 at 6:32 PM, Dave Page <dpage@pgadmin.org> wrote:
HiThere 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.sharma@enterprisedb.com> wrote:Hi Dave,Please find updated version attached.Regards,AtulOn Fri, Jun 23, 2017 at 8:49 PM, Dave Page <dpage@pgadmin.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.sharma@enterprisedb.com> wrote:
> Hi,
>
> Please find updated patch.
>
> Regards,
> Atul
>
> On Thu, Jun 22, 2017 at 5:02 PM, Dave Page <dpage@pgadmin.org> wrote:
>>
>> Hi
>>
>> On Thu, Jun 22, 2017 at 11:05 AM, Atul Sharma
>> <atul.sharma@enterprisedb.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
Вложения
Hi Awesome - that works nicely. Patch committed. Congratulations on your first feature! Regards, Dave. On Mon, Jun 26, 2017 at 8:56 AM, Atul Sharma <atul.sharma@enterprisedb.com> wrote: > Hi Dave, > > Yes, It makes sense to use the same approach at both ends. I have modified > the server file to use the same regex approach. > Attached is the patch for the same. Please review. > > Regards, > Atul > > On Sun, Jun 25, 2017 at 6:32 PM, Dave Page <dpage@pgadmin.org> wrote: >> >> 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.sharma@enterprisedb.com> >> wrote: >>> >>> Hi Dave, >>> >>> Please find updated version attached. >>> >>> Regards, >>> Atul >>> >>> On Fri, Jun 23, 2017 at 8:49 PM, Dave Page <dpage@pgadmin.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.sharma@enterprisedb.com> wrote: >>>> > Hi, >>>> > >>>> > Please find updated patch. >>>> > >>>> > Regards, >>>> > Atul >>>> > >>>> > On Thu, Jun 22, 2017 at 5:02 PM, Dave Page <dpage@pgadmin.org> wrote: >>>> >> >>>> >> Hi >>>> >> >>>> >> On Thu, Jun 22, 2017 at 11:05 AM, Atul Sharma >>>> >> <atul.sharma@enterprisedb.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 >> > -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company