Re: [pgadmin-hackers] potential invalid input field of pgAdmin New Column GUI

From: Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com>
To: Sanket Mehta <sanket(dot)mehta(at)enterprisedb(dot)com>
Cc: Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>, pgadmin-support <pgadmin-support(at)postgresql(dot)org>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>, liuyuanyuan <liuyuanyuangogo(at)gmail(dot)com>
Subject: Re: [pgadmin-hackers] potential invalid input field of pgAdmin New Column GUI
Date: 2014-11-28 12:37:29
Message-ID: CAG7mmoycMEgoROyCOAKYM1FU8PO7xuk3t2OAs-O+S_fEm3D7tA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers pgadmin-support

Hi Sanket,

On Fri, Nov 28, 2014 at 3:57 PM, Sanket Mehta <sanket(dot)mehta(at)enterprisedb(dot)com
> wrote:

> Hi Ashesh,
>
> On Wed, Nov 26, 2014 at 7:35 PM, Ashesh Vashi <
> ashesh(dot)vashi(at)enterprisedb(dot)com> wrote:
>
>> Hi Sanket,
>>
>> On Wed, Nov 26, 2014 at 5:41 PM, Akshay Joshi <
>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>
>>> Hi Sanket
>>>
>>> Patch looks good to me. I have tested couple of scenarios and it works
>>> fine.
>>>
>>> On Wed, Nov 26, 2014 at 3:27 PM, Sanket Mehta <
>>> sanket(dot)mehta(at)enterprisedb(dot)com> wrote:
>>>
>>>> Hi Akshay,
>>>>
>>>>
>>>> On Tue, Nov 25, 2014 at 2:57 PM, Akshay Joshi <
>>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>>
>>>>> Hi Sanket
>>>>>
>>>>> On Mon, Nov 24, 2014 at 5:18 PM, Sanket Mehta <
>>>>> sanket(dot)mehta(at)enterprisedb(dot)com> wrote:
>>>>>
>>>>>> Hi Akshay,
>>>>>>
>>>>>>
>>>>>> On Wed, Nov 19, 2014 at 4:39 PM, Akshay Joshi <
>>>>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>>>>
>>>>>>> Hi Sanket
>>>>>>>
>>>>>>> Below are the file wise review comments
>>>>>>>
>>>>>>>
>>>>>>> 1. ctlSeclabelPanel.cpp and ctlSeclabelPanel.h
>>>>>>> - Correct the function name "GetCUrrentProviderLabelArray"
>>>>>>> according to camel case.
>>>>>>> - Use trim before check the empty condition of txtProvider
>>>>>>> and txtSeclabel used in ctlSeclabelPanel::OnChange().
>>>>>>> 2. dlgTable.cpp
>>>>>>> - Security label sql not removed from SQL pane of dlgTable
>>>>>>> even though I have removed the column.
>>>>>>> - Security label sql gets overwritten when I add more then
>>>>>>> one column.
>>>>>>> - Changes are overwritten in the SQL pane of dlgTable when I
>>>>>>> have changed the attributes of the column more then once by clicking on
>>>>>>> "Change" button.
>>>>>>>
>>>>>>> Above Issue is still reproducible.
>>>>>
>>>> All the reproducible issues are resolved.
>>>>
>>>>>
>>>>>>> 1. dlgColumn.cpp
>>>>>>> - Remove comment //code removed for testing from constructor
>>>>>>> if testing is done.
>>>>>>>
>>>>>>> Above issues are resolved.
>>>>>> PFA the revised patch.
>>>>>>
>>>>>
>>>>> I have found one more issue when we add security label while
>>>>> creating a new table it throws syntax error.
>>>>>
>>>> This issue is also resolved.
>>>>
>>>> PFA the revised patch.
>>>>
>>> I can see the memory leak, while closing the table dialog.
>> While changing the column, we do create column2 for preserving the data,
>> but it is never been released.
>>
>> Can you please cross check?
>>
>> *Resolved.*
>>
> *PFA the revised patch.*
>
Thanks for the patch.
It's looking good now to me.

Committed after some testing done.

--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company
<http://www.enterprisedb.com>

*http://www.linkedin.com/in/asheshvashi*
<http://www.linkedin.com/in/asheshvashi>

>
>>
> --
>>
>> Thanks & Regards,
>>
>> Ashesh Vashi
>> EnterpriseDB INDIA: Enterprise PostgreSQL Company
>> <http://www.enterprisedb.com>
>>
>>
>> *http://www.linkedin.com/in/asheshvashi*
>> <http://www.linkedin.com/in/asheshvashi>
>>
>>>
>>>>
>>>>>
>>>>>>
>>>>>>> On Tue, Nov 18, 2014 at 6:10 PM, Sanket Mehta <
>>>>>>> sanket(dot)mehta(at)enterprisedb(dot)com> wrote:
>>>>>>>
>>>>>>> Hi,
>>>>>>>>
>>>>>>>> Below are the thing I have taken care in this patch:
>>>>>>>>
>>>>>>>> 1. While creating the new table using table wizard, when any
>>>>>>>> variables or security labels are specified for certain column, those
>>>>>>>> were not visible on *sql tab* of table wizard.
>>>>>>>> 2. when new/existing column dialog is open from table dialog then
>>>>>>>> privileges and security labels in column dialog are not being persistent
>>>>>>>> for new changes.
>>>>>>>>
>>>>>>>>
>>>>>>>> Regards,
>>>>>>>> Sanket Mehta
>>>>>>>> Sr Software engineer
>>>>>>>> Enterprisedb
>>>>>>>>
>>>>>>>> On Tue, Nov 18, 2014 at 5:54 PM, Sanket Mehta <
>>>>>>>> sanket(dot)mehta(at)enterprisedb(dot)com> wrote:
>>>>>>>>
>>>>>>>>> Hi Akshay,
>>>>>>>>>
>>>>>>>>> PFA the patch.
>>>>>>>>> Please review it and let me know if anything is missing.
>>>>>>>>>
>>>>>>>>> Regards,
>>>>>>>>> Sanket Mehta
>>>>>>>>> Sr Software engineer
>>>>>>>>> Enterprisedb
>>>>>>>>>
>>>>>>>>> On Tue, Nov 11, 2014 at 7:21 PM, Ashesh Vashi <
>>>>>>>>> ashesh(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>>>>>
>>>>>>>>>> Hi Sanket,
>>>>>>>>>>
>>>>>>>>>> Apart from variable persistence issue taken care in your patch,
>>>>>>>>>> I've also observed the data persistence issue with:
>>>>>>>>>> (When reloading the existing/new column in column dialog from
>>>>>>>>>> table dialog)
>>>>>>>>>> 1. Priviledges
>>>>>>>>>> 2. Security Lables
>>>>>>>>>>
>>>>>>>>>> I also observed, when I remove some privileges from an existing
>>>>>>>>>> column, it generates SQL like, it needs to remove that column first, and
>>>>>>>>>> then add that column, and the modify the new privileges.
>>>>>>>>>> Can you also look into it?
>>>>>>>>>>
>>>>>>>>>> --
>>>>>>>>>>
>>>>>>>>>> Thanks & Regards,
>>>>>>>>>>
>>>>>>>>>> Ashesh Vashi
>>>>>>>>>> EnterpriseDB INDIA: Enterprise PostgreSQL Company
>>>>>>>>>> <http://www.enterprisedb.com>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> *http://www.linkedin.com/in/asheshvashi*
>>>>>>>>>> <http://www.linkedin.com/in/asheshvashi>
>>>>>>>>>>
>>>>>>>>>> On Tue, Nov 11, 2014 at 7:06 PM, Ashesh Vashi <
>>>>>>>>>> ashesh(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>>>>>>
>>>>>>>>>>> Hi Sanket,
>>>>>>>>>>>
>>>>>>>>>>> Quick review suggests that, you're not following the consistent
>>>>>>>>>>> name convention in new functions as per pgAdmin3 coding standard.
>>>>>>>>>>> I also observed a white-space warning, while apply the patch.
>>>>>>>>>>>
>>>>>>>>>>> Please resend the patch after resolving these issues.
>>>>>>>>>>>
>>>>>>>>>>> --
>>>>>>>>>>>
>>>>>>>>>>> Thanks & Regards,
>>>>>>>>>>>
>>>>>>>>>>> Ashesh Vashi
>>>>>>>>>>> EnterpriseDB INDIA: Enterprise PostgreSQL Company
>>>>>>>>>>> <http://www.enterprisedb.com>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> *http://www.linkedin.com/in/asheshvashi*
>>>>>>>>>>> <http://www.linkedin.com/in/asheshvashi>
>>>>>>>>>>>
>>>>>>>>>>> On Tue, Nov 11, 2014 at 5:25 PM, Sanket Mehta <
>>>>>>>>>>> sanket(dot)mehta(at)enterprisedb(dot)com> wrote:
>>>>>>>>>>>
>>>>>>>>>>>> Hi,
>>>>>>>>>>>>
>>>>>>>>>>>> Issue was occurring as mentioned below:
>>>>>>>>>>>>
>>>>>>>>>>>> While creating the new table using table wizard, when any
>>>>>>>>>>>> variables or security labels are specified for certain column then in
>>>>>>>>>>>> *dlgTable* class, it was not fetching those variable or
>>>>>>>>>>>> security labels from *dlgColumn *class. So those were not
>>>>>>>>>>>> visible on *sql tab* of new table wizard.
>>>>>>>>>>>>
>>>>>>>>>>>> I have resolved that issue and created the patch for the same.
>>>>>>>>>>>> Patch is attached with this mail. Please review it and if it
>>>>>>>>>>>> looks good, please commit the code.
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Regards,
>>>>>>>>>>>> Sanket Mehta
>>>>>>>>>>>> Sr Software engineer
>>>>>>>>>>>> Enterprisedb
>>>>>>>>>>>>
>>>>>>>>>>>> On Tue, Nov 4, 2014 at 1:16 PM, Sanket Mehta <
>>>>>>>>>>>> sanket(dot)mehta(at)enterprisedb(dot)com> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>> Sure Ashesh,
>>>>>>>>>>>>>
>>>>>>>>>>>>> I will check and get back.
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Regards,
>>>>>>>>>>>>> Sanket Mehta
>>>>>>>>>>>>> Sr Software engineer
>>>>>>>>>>>>> Enterprisedb
>>>>>>>>>>>>>
>>>>>>>>>>>>> On Tue, Nov 4, 2014 at 1:03 PM, Ashesh Vashi <
>>>>>>>>>>>>> ashesh(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>>> Sanket,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Can you take a look at it?
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> --
>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Ashesh Vashi
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On 4 Nov 2014 12:54, "liuyuanyuan" <liuyuanyuangogo(at)gmail(dot)com>
>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>> >
>>>>>>>>>>>>>> >
>>>>>>>>>>>>>> >
>>>>>>>>>>>>>> > Hi , hackers!
>>>>>>>>>>>>>> >
>>>>>>>>>>>>>> > Currently I test some part of pgadmin GUI, and I found some
>>>>>>>>>>>>>> potential invalid input field of New Column GUI,
>>>>>>>>>>>>>> >
>>>>>>>>>>>>>> > Like Variables and Security Labels.
>>>>>>>>>>>>>> Could you please have a look on this issue?
>>>>>>>>>>>>>> >
>>>>>>>>>>>>>> >
>>>>>>>>>>>>>> >
>>>>>>>>>>>>>> > For example:
>>>>>>>>>>>>>> >
>>>>>>>>>>>>>> > OS: WIN7 64bit
>>>>>>>>>>>>>> >
>>>>>>>>>>>>>> > PostgreSQL 9.3
>>>>>>>>>>>>>> >
>>>>>>>>>>>>>> > pgAdmin III:Version 1.18.1
>>>>>>>>>>>>>> >
>>>>>>>>>>>>>> > I use GUI of pgadmin to create table, and add column to
>>>>>>>>>>>>>> this table (just as follow ). When I add a new column,
>>>>>>>>>>>>>> >
>>>>>>>>>>>>>> > I can add Variables (or Security Label )to this column, but
>>>>>>>>>>>>>> finally in the tab SQL of New Table Interface I find nothing
>>>>>>>>>>>>>> >
>>>>>>>>>>>>>> > of the Variables (or Security Label ) I’ve add. It seems
>>>>>>>>>>>>>> that the Variables (or Security Label ) does not work.
>>>>>>>>>>>>>> >
>>>>>>>>>>>>>> >
>>>>>>>>>>>>>> >
>>>>>>>>>>>>>> > Best Wishes!
>>>>>>>>>>>>>> >
>>>>>>>>>>>>>> >
>>>>>>>>>>>>>> >
>>>>>>>>>>>>>> > Yours
>>>>>>>>>>>>>> >
>>>>>>>>>>>>>> > Jasmine Liu
>>>>>>>>>>>>>> >
>>>>>>>>>>>>>> >
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> *Akshay Joshi*
>>>>>>> *Principal Software Engineer *
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> *Phone: +91 20-3058-9517Mobile: +91 976-788-8246*
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> *Akshay Joshi*
>>>>> *Principal Software Engineer *
>>>>>
>>>>>
>>>>>
>>>>> *Phone: +91 20-3058-9517Mobile: +91 976-788-8246*
>>>>>
>>>>
>>>>
>>>
>>>
>>> --
>>> *Akshay Joshi*
>>> *Principal Software Engineer *
>>>
>>>
>>>
>>> *Phone: +91 20-3058-9517Mobile: +91 976-788-8246*
>>>
>>
>>
>

In response to

Browse pgadmin-hackers by date

  From Date Subject
Next Message Ashesh Vashi 2014-11-28 14:13:36 Re: Bad string from the translation point of view
Previous Message Ashesh Vashi 2014-11-28 12:36:13 pgAdmin III commit: Fixed preservation of the modified data for the col

Browse pgadmin-support by date

  From Date Subject
Next Message Davide Ronchi 2014-12-01 17:19:59 Does PGAdmin support the PGPASSFILE environment variable?
Previous Message Sanket Mehta 2014-11-28 10:27:29 Re: [pgadmin-hackers] potential invalid input field of pgAdmin New Column GUI