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

From: Sanket Mehta <sanket(dot)mehta(at)enterprisedb(dot)com>
To: Akshay Joshi <akshay(dot)joshi(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-26 09:57:08
Message-ID: CA+yw=mO0VSJFXBPP_4z1ZriX+Fw2ZjJq26s0KEX+e8oz+nCGxA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers pgadmin-support

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.

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

Attachment Content-Type Size
variable_and_privileges_issue2.patch application/octet-stream 16.3 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Akshay Joshi 2014-11-26 12:11:16 Re: [pgadmin-hackers] potential invalid input field of pgAdmin New Column GUI
Previous Message Akshay Joshi 2014-11-25 09:27:24 Re: [pgadmin-hackers] potential invalid input field of pgAdmin New Column GUI

Browse pgadmin-support by date

  From Date Subject
Next Message Akshay Joshi 2014-11-26 12:11:16 Re: [pgadmin-hackers] potential invalid input field of pgAdmin New Column GUI
Previous Message Dave Page 2014-11-25 11:18:32 Re: RSS feed?