Re: [pgAdmin4][Patch]: Fixed RM 1603 & RM 1220

From: Dave Page <dpage(at)pgadmin(dot)org>
To: Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com>
Cc: Khushboo Vashi <khushboo(dot)vashi(at)enterprisedb(dot)com>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgAdmin4][Patch]: Fixed RM 1603 & RM 1220
Date: 2016-10-15 06:22:06
Message-ID: CA+OCxoxOB6Yx-d8yKEkB3Gv6XAD=UAVBObxLB275m=nQuecpcQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

On Friday, October 14, 2016, Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com>
wrote:

> On Sat, Oct 15, 2016 at 4:59 AM, Dave Page <dpage(at)pgadmin(dot)org
> <javascript:_e(%7B%7D,'cvml','dpage(at)pgadmin(dot)org');>> wrote:
>
>> Hi
>>
>> On Friday, October 14, 2016, Khushboo Vashi <
>> khushboo(dot)vashi(at)enterprisedb(dot)com
>> <javascript:_e(%7B%7D,'cvml','khushboo(dot)vashi(at)enterprisedb(dot)com');>> wrote:
>>
>>> Hi,
>>>
>>> Please find the attached patch to fix the below 2 bugs.
>>>
>>> RM 1603: [Web Based] Export database failed if object contains double
>>> quotes.
>>> RM 1220: Backup database is not working with special characters
>>>
>>> The issues which were fixed:
>>>
>>> 1. Client side data were not unescaped
>>> 2. Required command line arguments were quoted twice
>>>
>>
>> This is not working for me: I tested using Table Export as per Fahar's
>> instructions. As I'm in desktop mode, the first problem was that we get an
>> error at line 210 of import_export/__init__.py, because
>> get_server_directory returned None for the directory. If I fix that, then
>> the job says it's created, but as far as I can see, nothing else happens.
>>
> hmm..
>

Yes, but please see my followup message. There's clearly something funky
going on with the process tracking - for whatever reason it didn't pick up
this process until after a restart, and per the bug I escalated earlier
(which I think is essential to fix for 1.1 in a little over a week), it
doesn't always detect completed processes and then keeps re-showing the
alert.

>
>> Secondly, this patch seems to push quoting responsibilty to the front end.
>>
> No - that's not the case, we're using _.escape(..) function on the node's
> label to fix the issue of XSS vulnerability on client side.
> Hence - during sending back the data, we're using _.unescape(..) function
> to return the same data coming sent by the server.
>

Ahh, OK - I see.

>
> Though - IIRC - we have a original label stored in another variable
> '_label', which we can use it instead of unescape it again.
>

Right, as we've done in many other places.

> This doesn't seem right, because we might want to use the RESTful APIs for
>> another purpose in the future, which would mean needing to re-implement
>> quoting if something else uses an affected API.
>>
> As I explained above, it wont affect the RESTful API.
>

Yep. Thanks for setting me straight.

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Dave Page 2016-10-15 15:44:56 pgAdmin 4 commit: Allow test servers to be enabled or disabled easily i
Previous Message Ashesh Vashi 2016-10-15 03:56:33 Re: [pgAdmin4][Patch]: Fixed RM 1603 & RM 1220