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

From: Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
To: Sanket Mehta <sanket(dot)mehta(at)enterprisedb(dot)com>
Cc: Ashesh Vashi <ashesh(dot)vashi(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-25 09:27:24
Message-ID: CANxoLDdHqzKfDeH=gUO0kQNiGMQPrrkeR8PpOgkqu4Ku2vJJvA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers pgadmin-support

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.

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

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

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Sanket Mehta 2014-11-26 09:57:08 Re: [pgadmin-hackers] potential invalid input field of pgAdmin New Column GUI
Previous Message Sanket Mehta 2014-11-24 11:48:00 Re: [pgadmin-hackers] potential invalid input field of pgAdmin New Column GUI

Browse pgadmin-support by date

  From Date Subject
Next Message hushthatbush 2014-11-25 10:07:19 RSS feed?
Previous Message Ronny Kurniawan 2014-11-25 08:18:04 Bug Report: Length incorrectly specified when creating columns of 'time without time zone'