Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bxBE4-0001oF-If for pgadmin-hackers@arkaria.postgresql.org; Thu, 20 Oct 2016 11:09:32 +0000 Received: from localhost ([127.0.0.1] helo=postgresql.org) by malur.postgresql.org with smtp (Exim 4.84_2) (envelope-from ) id 1bxBE3-0003d7-RS for pgadmin-hackers@arkaria.postgresql.org; Thu, 20 Oct 2016 11:09:31 +0000 Received: from makus.postgresql.org ([2001:4800:1501:1::229]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1bxBE3-0003d0-1x for pgadmin-hackers@postgresql.org; Thu, 20 Oct 2016 11:09:31 +0000 Received: from mail-it0-x230.google.com ([2607:f8b0:4001:c0b::230]) by makus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.84_2) (envelope-from ) id 1bxBDz-0003Gy-Ix for pgadmin-hackers@postgresql.org; Thu, 20 Oct 2016 11:09:29 +0000 Received: by mail-it0-x230.google.com with SMTP id 66so84395921itl.1 for ; Thu, 20 Oct 2016 04:09:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=enterprisedb-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=9/R6ngYauAufg0eolAzsGH5phkQPckSM91Fhv4lNylk=; b=BPgUMoo4Vw2XfGMN/Rd6iaCQdlqD46OlxTwsenlsOligPPbjv085XalDalA1e5ORm7 CsK4mE4llqo/b/1ZAVzsT0OE5yxrcepdYVMTLxNob5NmIKsboWQjj5BU0M2XThZ9qbmk ZdCo20FUCOuJq1Syg8CJsMfCmte0nG655mFKGsvDR2udKieGHAM05VVg+j1in9J+2MX5 F+3L6/HdzOuu30q8L6S6ybrwNgt+RPKRs2WY5KJsK1Qj1+HzeiIeo5GJnu23CwzDHoSR Y2Ac+m/zmbGGfe7S9TvZ96vt9LePYzAJlB6voBAqUqoapARDSzzCCc6+I/yWT7BYdCDp CLZw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=9/R6ngYauAufg0eolAzsGH5phkQPckSM91Fhv4lNylk=; b=Bz2l8FrI37BwjCOOssjMOiEiWKcm6w6xLGBDynoG8nGJPLlrJcZMlAF6WfsnXOl39r 3TSH0AfP23Mi4Qn++otMOPiZQUacnZGwC65vEA+Pk93NYHzXLZbMcy0fuz0TlmEAJZUw LND7FejUtuGaoQdfFwhyS26mSt06BXM4pn8jOAhQnYio5b/TB5vFzQI5Z5OIKzQNYCdB 00I4cIlo1YH77EuZiLuTiA3MrfnO3VhqnvesduSwpBUfx8AEZL3VsnOwK864daOnjRjq NjQfzCji1JSj4k99PpGmDPQzXEhUIRhBFkH5awTieL6lJEniqlZl1g6wFnENRlDSy6Ey 6Aug== X-Gm-Message-State: ABUngveN26d2DH/3CTJqw9DAyDf2jgKbVtcfy+qpSyp0EnLSPTsHbSCYMqKW+33ePpmB9ulx1ep92Fyg/mpBbR8e X-Received: by 10.107.58.70 with SMTP id h67mr84812ioa.115.1476961753000; Thu, 20 Oct 2016 04:09:13 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.25.139 with HTTP; Thu, 20 Oct 2016 04:08:50 -0700 (PDT) In-Reply-To: References: From: Ashesh Vashi Date: Thu, 20 Oct 2016 16:38:50 +0530 Message-ID: Subject: Re: [pgAdmin4][Patch]: Fixed RM 1603 & RM 1220 To: Khushboo Vashi Cc: Dave Page , pgadmin-hackers Content-Type: multipart/alternative; boundary=001a114a8ef6c3c41e053f49f4c0 X-Pg-Spam-Score: -2.6 (--) List-Archive: List-Help: List-ID: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: X-Mailing-List: pgadmin-hackers Precedence: bulk Sender: pgadmin-hackers-owner@postgresql.org --001a114a8ef6c3c41e053f49f4c0 Content-Type: text/plain; charset=UTF-8 On Thu, Oct 20, 2016 at 4:26 PM, Khushboo Vashi < khushboo.vashi@enterprisedb.com> wrote: > > > On Sat, Oct 15, 2016 at 11:52 AM, Dave Page wrote: > >> >> >> On Friday, October 14, 2016, Ashesh Vashi >> wrote: >> >>> On Sat, Oct 15, 2016 at 4:59 AM, Dave Page wrote: >>> >>>> Hi >>>> >>>> On Friday, October 14, 2016, Khushboo Vashi < >>>> khushboo.vashi@enterprisedb.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. >> >> > > The problem here is that, until we click the "Click for details here" link > and close the another details dialogue, the acknowledgement does not send > to the server. So, it keeps re-showing the alert. > > I think, we need to clearly mention the steps on the alertify notifier > itself, so the user can get the idea. > > Dave/Ashesh, > Any other suggestion? > We can give a acknowledge link along with 'Click here for details' link to delete the status, logs, when clicked. Dave? > > >> >>>> 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. >> > > I have replaced _. unescape with _label > > >> >>> 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 >> >> > --001a114a8ef6c3c41e053f49f4c0 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
On T= hu, Oct 20, 2016 at 4:26 PM, Khushboo Vashi <khushboo.vash= i@enterprisedb.com> wrote:
=


On Sat, Oct 15, 2016 at 11:52 AM, Dave Page <dpage@p= gadmin.org> wrote:


On Friday, = October 14, 2016, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote= :

On Sat, Oct 15, 2016 at 4:59 AM, Dave Pag= e <dpage@pgadmin.org> wrote:

<= /div>
Hi

On Friday, October 14, 2016, Khush= boo Vashi <khushboo.vashi@enterprisedb.com> wrote:
Hi,
Please find the attached patch to fix the below 2 bugs.

RM 1603:=C2=A0[Web Based] Export database failed if objec= t contains double quotes.
RM=C2=A01220: Backup database is not wo= rking with special characters

The issues which wer= e fixed:

1. Client side data were not unescaped
2. Required command line arguments were quoted twice=C2=A0

This is not working for me: I te= sted using Table Export as per Fahar's instructions. As I'm in desk= top 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 dire= ctory. If I fix that, then the job says it's created, but as far as I c= an see, nothing else happens.
hmm..=C2=A0

Yes, but please see my= followup message. There's clearly something funky going on with the pr= ocess tracking - for whatever reason it didn't pick up this process unt= il after a restart, and per the bug I escalated earlier (which I think is e= ssential to fix for 1.1 in a little over a week), it doesn't always det= ect completed processes and then keeps re-showing the alert.
=C2=A0
=C2=A0
The problem here is that, until we click the &qu= ot;Click for details here" link and close the another details dialogue= , the acknowledgement does not send to the server. So, it keeps re-showing = the alert.

I think, we need to clearly mention the= steps on the alertify notifier itself, so the user can get the idea.=C2=A0=

Dave/Ashesh,
Any other suggestion?
We can give a acknowledge link along w= ith 'Click here for details' link to delete the status, logs, when = clicked.
Dave?=C2=A0
=C2=A0

Secondly, this patch seems to push quoting responsibilty to the fr= ont end.
No - that's not the case, we're usi= ng _.escape(..) function on the node's label to fix the issue of=C2=A0<= span style=3D"font-size:12.8px;background-color:rgb(255,255,255)">XSS=C2=A0vulnerability on client side= .
Hence - during sending back the data, we're using _.unescap= e(..) function to return the same data coming sent by the server.

Ahh, OK - I see.
=C2=A0

Though - IIRC - we ha= ve a original label stored in another variable '_label', which we c= an use it instead of unescape it again.=C2=A0

Right, as we've done in many other pl= aces.
=C2=A0
I have replaced =C2=A0= _. unescape with _label

=C2=A0
This doesn't seem rig= ht, because we might want to use the RESTful APIs for another purpose in th= e future, which would mean needing to re-implement quoting if something els= e uses an affected API.
As I explained above, it won= t 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


--001a114a8ef6c3c41e053f49f4c0--