Re: [PATCH] Fix crash when disabling auto commit

From: John Obaterspok <john(dot)obaterspok(at)gmail(dot)com>
To: Sanket Mehta <sanket(dot)mehta(at)enterprisedb(dot)com>
Cc: Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [PATCH] Fix crash when disabling auto commit
Date: 2015-06-22 18:33:06
Message-ID: CAOscVd+gSTXrC2uNhg8R-1QwKDipR3B0CxO8hbyqXQ0h3rZuLw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hello,

It still crashes on the wxIsalpha. Uncommenting the /* (size_t)wordlen <
queryLen && */ makes it not crash.

-- john

2015-06-18 11:24 GMT+02:00 Sanket Mehta <sanket(dot)mehta(at)enterprisedb(dot)com>:

> Hi,
>
> we had found some other issues in autocommit code and resolved it.
> Here is the patch attached with this mail.
>
>
> Regards,
> Sanket Mehta
> Sr Software engineer
> Enterprisedb
>
> On Tue, Jun 16, 2015 at 12:59 PM, John Obaterspok <
> john(dot)obaterspok(at)gmail(dot)com> wrote:
>
>> Hello,
>>
>> I forgot to mention that yesterday I did a search for GetChar() and there
>> was another place that also checked the length prior to calling GetChar().
>> But perhaps that case was different.
>>
>> Might be wx 3 related. (It has works fine except for an assert that pops
>> up after connecting to a db + the GetChar() issue)
>>
>> -- john
>>
>> 2015-06-16 8:40 GMT+02:00 Sanket Mehta <sanket(dot)mehta(at)enterprisedb(dot)com>:
>>
>>> Hi,
>>>
>>> We were using wxWidgets 2.8.12 so that may be the issue.
>>> I will now try with wxwidgets 3.0.2 and let you know.
>>>
>>> Btw apart from this issue, we have also resolved other issues in the
>>> code, i will soon send a new patch for the same.
>>>
>>> Regards,
>>> Sanket Mehta
>>> Sr Software engineer
>>> Enterprisedb
>>>
>>> On Mon, Jun 15, 2015 at 11:33 PM, John Obaterspok <
>>> john(dot)obaterspok(at)gmail(dot)com> wrote:
>>>
>>>> I'm using Visual Studio 2010 on Windows 7 x64. wxWidgets is wxMSW-3.0.2.
>>>> It might be an assert that triggers in wxwidgets, but ignoring it just
>>>> causes pgadmin to terminate.
>>>>
>>>> -- john
>>>>
>>>> 2015-06-14 18:44 GMT+02:00 Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com>
>>>> :
>>>>
>>>>> John,
>>>>>
>>>>> As I understand correctly, 8th character will be '\0' (null
>>>>> character). Hence - wxIsAlpha() will come out of the loop in general.
>>>>>
>>>>> But - it is possible every platform has different behaviour for
>>>>> wxWidgets. Can you please be specific about the operating system?
>>>>>
>>>>> --
>>>>> Thanks & Regards,
>>>>>
>>>>> Ashesh Vashi
>>>>> EnterpriseDB (Software Architect)
>>>>>
>>>>> [Sent through mobile]
>>>>> On Jun 14, 2015 2:41 PM, "Sanket Mehta" <sanket(dot)mehta(at)enterprisedb(dot)com>
>>>>> wrote:
>>>>>
>>>>>> Hi John,
>>>>>>
>>>>>> I have tried the same, I am not getting any out of the bounds error,
>>>>>> it simply comes out of the while loop.
>>>>>>
>>>>>> Regards,
>>>>>> Sanket Mehta
>>>>>> Sr Software engineer
>>>>>> Enterprisedb
>>>>>>
>>>>>> On Sun, Jun 14, 2015 at 2:00 PM, John Obaterspok <
>>>>>> john(dot)obaterspok(at)gmail(dot)com> wrote:
>>>>>>
>>>>>>> Hello Sanket,
>>>>>>>
>>>>>>> Just enter "rollback" and exec
>>>>>>>
>>>>>>> The
>>>>>>> while(wxIsalpha(query.GetChar(wordlen)))
>>>>>>> wordlen++;
>>>>>>>
>>>>>>> As the 'k' in rollback is a char it also tries the next character
>>>>>>> (worklen = 8) which causes out of bounds check.
>>>>>>>
>>>>>>> -- john
>>>>>>>
>>>>>>>
>>>>>>> 2015-06-12 13:25 GMT+02:00 Sanket Mehta <
>>>>>>> sanket(dot)mehta(at)enterprisedb(dot)com>:
>>>>>>>
>>>>>>>> Hi John,
>>>>>>>>
>>>>>>>> I have tried to reproduce the scenario but not able to reproduce
>>>>>>>> the crash in my system.
>>>>>>>> can you please provide your steps which causes crash on your
>>>>>>>> machine?
>>>>>>>>
>>>>>>>> Regards,
>>>>>>>> Sanket Mehta
>>>>>>>> Sr Software engineer
>>>>>>>> Enterprisedb
>>>>>>>>
>>>>>>>> On Fri, Jun 12, 2015 at 11:50 AM, Sanket Mehta <
>>>>>>>> sanket(dot)mehta(at)enterprisedb(dot)com> wrote:
>>>>>>>>
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> I am looking into the same and few other issues in code and will
>>>>>>>>> send the patch soon.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Regards,
>>>>>>>>> Sanket Mehta
>>>>>>>>> Sr Software engineer
>>>>>>>>> Enterprisedb
>>>>>>>>>
>>>>>>>>> On Fri, Jun 12, 2015 at 1:49 AM, John Obaterspok <
>>>>>>>>> john(dot)obaterspok(at)gmail(dot)com> wrote:
>>>>>>>>>
>>>>>>>>>> Fix crash when string has only alphas (like 'rollback')
>>>>>>>>>>
>>>>>>>>>> diff --git a/pgadmin/frm/frmQuery.cpp b/pgadmin/frm/frmQuery.cpp
>>>>>>>>>> index b5a2f56..110bbc7 100644
>>>>>>>>>> --- a/pgadmin/frm/frmQuery.cpp
>>>>>>>>>> +++ b/pgadmin/frm/frmQuery.cpp
>>>>>>>>>> @@ -2522,7 +2522,7 @@ bool frmQuery::isBeginNotRequired(wxString
>>>>>>>>>> query)
>>>>>>>>>> /*
>>>>>>>>>> * Check word length (since "beginx" is not "begin").
>>>>>>>>>> */
>>>>>>>>>> - while(wxIsalpha(query.GetChar(wordlen)))
>>>>>>>>>> + while(wordlen < query.Length() &&
>>>>>>>>>> wxIsalpha(query.GetChar(wordlen)))
>>>>>>>>>> wordlen++;
>>>>>>>>>>
>>>>>>>>>> /*
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>
>>>
>>
>

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Dave Page 2015-06-23 08:17:40 Re: [PATCH] Add Commit/Rollback toolbar action
Previous Message John Obaterspok 2015-06-22 18:02:33 Re: [PATCH] Add Commit/Rollback toolbar action