Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bxB1d-0000as-QS for pgadmin-hackers@arkaria.postgresql.org; Thu, 20 Oct 2016 10:56:41 +0000 Received: from localhost ([127.0.0.1] helo=postgresql.org) by malur.postgresql.org with smtp (Exim 4.84_2) (envelope-from ) id 1bxB1d-0003Lx-6h for pgadmin-hackers@arkaria.postgresql.org; Thu, 20 Oct 2016 10:56:41 +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 1bxB1P-00037g-Nc for pgadmin-hackers@postgresql.org; Thu, 20 Oct 2016 10:56:28 +0000 Received: from mail-lf0-x22c.google.com ([2a00:1450:4010:c07::22c]) by makus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.84_2) (envelope-from ) id 1bxB1M-00033q-7I for pgadmin-hackers@postgresql.org; Thu, 20 Oct 2016 10:56:26 +0000 Received: by mail-lf0-x22c.google.com with SMTP id l131so73465567lfl.2 for ; Thu, 20 Oct 2016 03:56:23 -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=QKg6MTe8LHyJ7twjGkCbg2BQo8AfsOp6pGr/5RtFNOQ=; b=H1d0HmJYt/lXp5NKCX6/e8Qhi1quuJ1ntae4PIJxxQlwGRteC/MlD6QAqzHSrKO5Rg CcjvF675a8q98TG3sqiExLBEiYZ116r7CtPgoAoeR8rbMdbrlg5HGmGU1pVCPzjeXwTf BzqprPjO7LgO23z95pHxjJYAXKoHG4HpX3LNRvkbLmh8ZPK7LLS3lGCiMpftWfIgASyk wiH/qmwMlP0XTthgr8iy00qh2eK1E05UyGyimCyM6KGrUBDYhb8Su676l1zoYdgiuTzo IOfAYLxTQEYnrDjwyx/vSUZQuJXmPufkmp7T0kX8MdlHXha4C2LnDASx0SHbLwWta8ze eCMQ== 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=QKg6MTe8LHyJ7twjGkCbg2BQo8AfsOp6pGr/5RtFNOQ=; b=D0DK0CZ3YpESQnuFr4RcgB174HrZLSLfTencFaXms5HFhLe+62+SSaVEszDcMy8QU+ rcVPhQk3T2A0paBE5pI7T+d49GMtlfgyL/PDbqU1H5B75H89EVHKjSflHSkh8KJp4ueR ThaL0/3FZyNnRgG2faCGnPj4sPl8HyPGZ9L+reaEVeLHtbPhbF4c542yBtoWBwaz91Bh 5M+CrI87yk32MoejllJR5XgaxNzbHa87rzikaT0v4+HCqqTzP1nlUa/Yuhcb0LViOD9W 2FCc3DrDWpVu6K/M3CYj/QJQEdqsxd2xYB/qhrgE4XWdM3lax4xzaMgcKYdw+b2Wog/G ZaOA== X-Gm-Message-State: AA6/9RmiVnGTxilQdV3XUTEFyvFXxmz5oMDrPGZJVNrNKmO7j5bGyKpLUNJypVjVJZnFoRAfm5s/PF9CeqEHrHT/ X-Received: by 10.25.35.6 with SMTP id j6mr644492lfj.147.1476960979579; Thu, 20 Oct 2016 03:56:19 -0700 (PDT) MIME-Version: 1.0 Received: by 10.25.87.3 with HTTP; Thu, 20 Oct 2016 03:56:18 -0700 (PDT) In-Reply-To: References: From: Khushboo Vashi Date: Thu, 20 Oct 2016 16:26:18 +0530 Message-ID: Subject: Re: [pgAdmin4][Patch]: Fixed RM 1603 & RM 1220 To: Dave Page Cc: Ashesh Vashi , pgadmin-hackers Content-Type: multipart/alternative; boundary=001a113ca762890714053f49c60b 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 --001a113ca762890714053f49c60b Content-Type: text/plain; charset=UTF-8 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? > >>> 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 > > --001a113ca762890714053f49c60b Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable


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


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

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

Hi
On Friday, October 14, 2016, Khushboo Vashi <khushboo.vashi@enterpri= sedb.com> wrote:
Hi,

Please find the attached pat= ch to fix the below 2 bugs.

RM 1603:=C2=A0[Web Bas= ed] Export database failed if object contains double quotes.
RM= =C2=A01220: Backup database is not working with special characters

The issues which were fixed:

1. C= lient side data were not unescaped
2. Required command line argum= ents were quoted twice=C2=A0

=
This is not working for me: I tested using Table Export as per Fahar&#= 39;s instructions. As I'm in desktop mode, the first problem was that w= e 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 say= s it's created, but as far as I can see, nothing else happens.
hmm..=C2=A0

Yes, but please see my followup message. There's clearly= something funky going on with the process tracking - for whatever reason i= t 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 ov= er a week), it doesn't always detect completed processes and then keeps= re-showing the alert.
=C2=A0
=
=C2=A0
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 men= tion the steps on the alertify notifier itself, so the user can get the ide= a.=C2=A0

Dave/Ashesh,
Any other suggesti= on?
=C2=A0
<= div dir=3D"ltr">

Secondly, = this patch seems to push quoting responsibilty to the front end.
No - that's not the case, we're using _.escape(..) fun= ction on the node's label to fix the issue of=C2=A0XSS=C2=A0vulnerability on client side.
Hence -= during sending back the data, we're using _.unescape(..) function to r= eturn the same data coming sent by the server.

Ahh, OK - I see.
=C2=A0
Though - IIRC - we have a original label stored in another var= iable '_label', which we can use it instead of unescape it again.= =C2=A0

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

=C2=A0
This doesn't seem right, becaus= e 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 t= he RESTful API.

<= div>Yep. Thanks for setting me straight.
<= div class=3D"gmail-h5">

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

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


--001a113ca762890714053f49c60b--