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

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.
> 3. dlgColumn.cpp
> - Remove comment //code removed for testing from constructor if
> testing is done.
>
> Above issues are resolved.
PFA the revised patch.

> 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*
>