Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bvIMQ-0000Pb-1N for pgadmin-hackers@arkaria.postgresql.org; Sat, 15 Oct 2016 06:22:22 +0000 Received: from localhost ([127.0.0.1] helo=postgresql.org) by malur.postgresql.org with smtp (Exim 4.84_2) (envelope-from ) id 1bvIMP-0004Hw-Kf for pgadmin-hackers@arkaria.postgresql.org; Sat, 15 Oct 2016 06:22:21 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1bvIMN-0004Hl-W3 for pgadmin-hackers@postgresql.org; Sat, 15 Oct 2016 06:22:20 +0000 Received: from mail-it0-x233.google.com ([2607:f8b0:4001:c0b::233]) by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.84_2) (envelope-from ) id 1bvIMD-0000Tx-PG for pgadmin-hackers@postgresql.org; Sat, 15 Oct 2016 06:22:18 +0000 Received: by mail-it0-x233.google.com with SMTP id m138so9280614itm.0 for ; Fri, 14 Oct 2016 23:22:08 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pgadmin-org.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=ZAB7NO4iDi28Uqm1yae3JE2UrsoArG9MDLSYQ1hrXF8=; b=Q3yDwnScisZKCzmqAkeS8OO5Qip4/16c8ItJ0CMHxHBfY9s70ohOHRDwW676EfS0Iw FFHQs9hbmm1XMSCMhX0/iEk2/N/dgJp3EstdtKrZKWAfJVgYcNV7XTgfChsW8mCVSJ3e VrTSdQZrSEQU+up3xyCEpamlRJKvy81Ahicem0Rs2Fpbe7azxFb+QB+/7aOyZs4UHaev VsCAHuzy5WLISjUKw/aMYP2IotY3bA/FRbOzFDAr57dwgew+n/TImW8PwqHfUf1jwgwy j5RAnnTSMZzjZDl3UO/9LYOQAGgOcgOvcAnzl4D3iPoRK2GBv6qJmLxrTv5itnVYPDOE 7siQ== 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=ZAB7NO4iDi28Uqm1yae3JE2UrsoArG9MDLSYQ1hrXF8=; b=Skjy7SLRtEzcW7Z2P6PbRcBTag/6//61vUkN3AbEtXNTdKtYV4XNTL9t39sht6W1zY CYRe/rG7rpcAAq9t+2wFnO9OHbnSnR9+UuJVam6EPr5Dgggnc0l2ce1YjhmFb17J4Pmc wpFB+VoAs8R7+7N1F9dZhmvRpakMJEK6zqrqNek9BFRx2C3WT8D6kA8H/SH9lTbgxFej 9PEwHrgdoSGjLD8N8ssNd+1hhzJ+A25Gh70i/G7277A8qrTRxYyPp42Lvr8QT8wb4yhd E+5+kQ0AKMCtx5nCwYizKLHnIL4JP/Lsv3HLT/BfhP95ky4ZIvB9pgiqVuglmc/ozYes /AaQ== X-Gm-Message-State: AA6/9RkyD+kTYcOPWvIZAODvvUAhcRyzaeyvDTQRBqbb44W7+rqEZlqTJZGQ3JQ8h5+yTn4knYqn+B+9wczPpg== X-Received: by 10.36.202.131 with SMTP id k125mr805088itg.100.1476512527427; Fri, 14 Oct 2016 23:22:07 -0700 (PDT) MIME-Version: 1.0 Received: by 10.64.146.135 with HTTP; Fri, 14 Oct 2016 23:22:06 -0700 (PDT) In-Reply-To: References: From: Dave Page Date: Sat, 15 Oct 2016 07:22:06 +0100 Message-ID: Subject: Re: [pgAdmin4][Patch]: Fixed RM 1603 & RM 1220 To: Ashesh Vashi Cc: Khushboo Vashi , pgadmin-hackers Content-Type: multipart/alternative; boundary=94eb2c089e5cb44007053ee15c10 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 --94eb2c089e5cb44007053ee15c10 Content-Type: text/plain; charset=UTF-8 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. > >> 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 --94eb2c089e5cb44007053ee15c10 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable

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 V= ashi <khushboo.vashi@enterprisedb.com> wrote:
<= div dir=3D"ltr">Hi,

Please find the attached patch to fi= x the below 2 bugs.

RM 1603:=C2=A0[Web Based] Expo= rt database failed if object contains double quotes.
RM=C2=A01220= : 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=C2=A0

This = is not working for me: I tested using Table Export as per Fahar's instr= uctions. As I'm in desktop mode, the first problem was that we get an e= rror 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..=C2=A0

Ye= s, 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 do= esn't always detect completed processes and then keeps re-showing the a= lert.
=C2=A0

Secondly, this patch seem= s to push quoting responsibilty to the front end.
No= - that's not the case, we're using _.escape(..) function on the no= de'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 return the same = data coming sent by the server.
Ahh, OK - I see.
=C2=A0

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

Rig= ht, as we've done in many other places.
=C2=A0
T= his doesn't seem right, because we might want to use the RESTful APIs f= or 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
--94eb2c089e5cb44007053ee15c10--