Subject: PATCH: Encoding issue with qtLiteral function (pgAdmin4)

From: Murtuza Zabuawala <[email-masked]>
Date: Fri, 22 Jul 2016 19:41:01 +0530

Hi,

PFA patch to fix the issue when we use qtLiteral function and user enters
special characters(like emojis) which it fails to handle.
RM#1307

--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgadmin-hackers mailing list ([email-masked])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

---

From: Dave Page <[email-masked]>
Date: Fri, 22 Jul 2016 16:50:42 +0100

Hi

> special characters(like emojis) which it fails to handle.

It still seems to be broken to me. For example, if I try to set a
comment on a table to 😬, I get the following error:

2016-07-22 16:47:49,526: INFO werkzeug: 127.0.0.1 - - [22/Jul/2016
16:47:49] "GET /browser/table/msql/1/1/17251/2200/17267?id=17267&description=%F0%9F%98%AC+&columns=%7B%22changed%22%3A%5B%7B%22attlen%22%3Anull%7D%2C%7B%22attlen%22%3Anull%7D%2C%7B%22attlen%22%3Anull%7D%2C%7B%22attlen%22%3Anull%7D%2C%7B%22attlen%22%3Anull%7D%5D%7D&_=1469202363116
HTTP/1.1" 500 -
Traceback (most recent call last):
  File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
line 2000, in __call__
    return self.wsgi_app(environ, start_response)
  File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
line 1991, in wsgi_app
    response = self.make_response(self.handle_exception(e))
  File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
line 1567, in handle_exception
    reraise(exc_type, exc_value, tb)
  File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
line 1988, in wsgi_app
    response = self.full_dispatch_request()
  File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
line 1641, in full_dispatch_request
    rv = self.handle_user_exception(e)
  File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
line 1544, in handle_user_exception
    reraise(exc_type, exc_value, tb)
  File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
line 1639, in full_dispatch_request
    rv = self.dispatch_request()
  File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
line 1625, in dispatch_request
    return self.view_functions[rule.endpoint](**req.view_args)
  File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/views.py",
line 84, in view
    return self.dispatch_request(*args, **kwargs)
  File "/Users/dpage/git/pgadmin4/web/pgadmin/browser/utils.py", line
235, in dispatch_request
    return method(*args, **kwargs)
  File "/Users/dpage/git/pgadmin4/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/__init__.py",
line 276, in wrap
    self.conn = self.manager.connection(did=kwargs['did'])
  File "/Users/dpage/git/pgadmin4/web/pgadmin/utils/driver/psycopg2/__init__.py",
line 1278, in connection
    raise Exception(msg_active_conn)
Exception: Server has no active connection. Please connect to the server.

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

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

---

From: Murtuza Zabuawala <[email-masked]>
Date: Mon, 25 Jul 2016 11:49:57 +0530

Hi Dave,

I'm not able to re-produce the issue on my side (PFA screenshot), and from
exception log you mentioned it looks like connection to database issue
rather than encoding.
Would you please try & test one more time?

Regards,
Murtuza

On Fri, Jul 22, 2016 at 9:20 PM, Dave Page <[email-masked]> wrote:

> 16:47:49] "GET
> /browser/table/msql/1/1/17251/2200/17267?id=17267&description=%F0%9F%98%AC+&columns=%7B%22changed%22%3A%5B%7B%22attlen%22%3Anull%7D%2C%7B%22attlen%22%3Anull%7D%2C%7B%22attlen%22%3Anull%7D%2C%7B%22attlen%22%3Anull%7D%2C%7B%22attlen%22%3Anull%7D%5D%7D&_=1469202363116
> HTTP/1.1" 500 -
>   File
> "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
> line 2000, in __call__
>   File
> "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
> line 1991, in wsgi_app
>   File
> "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
>   File
> "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
> line 1988, in wsgi_app
>   File
> "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
>   File
> "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
>   File
> "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
>   File
> "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
>   File
> "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/views.py",
> line 84, in view
>   File
> "/Users/dpage/git/pgadmin4/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/__init__.py",
> line 276, in wrap
>   File
> "/Users/dpage/git/pgadmin4/web/pgadmin/utils/driver/psycopg2/__init__.py",
> line 1278, in connection

---

From: Dave Page <[email-masked]>
Date: Mon, 25 Jul 2016 12:31:41 +0100

Hi

Whenever I connect to my primary test server I get the following in the
console, and see the attached error:

2016-07-25 12:22:56,684: INFO pgadmin: Connection Request for server#1
2016-07-25 12:22:56,688: ERROR pgadmin: 'ascii' codec can't decode byte
0x8f in position 0: ordinal not in range(128)
Traceback (most recent call last):
  File
"/Users/dpage/git/pgadmin4/web/pgadmin/browser/server_groups/servers/__init__.py",
line 785, in connect
    server_types=ServerType.types()
  File
"/Users/dpage/git/pgadmin4/web/pgadmin/utils/driver/psycopg2/__init__.py",
line 250, in connect
    password = password.decode()
UnicodeDecodeError: 'ascii' codec can't decode byte 0x8f in position 0:
ordinal not in range(128)

I can no longer connect to that server even without the patch, following
the testing I did on your patch last week.

To make everythign work again, I had to drop the SQLite database. If I then
try to add an emojii to the comment for the server (with your patch), I get:

2016-07-25 12:29:36,930: INFO werkzeug: 127.0.0.1 - - [25/Jul/2016
12:29:36] "PUT /browser/server/obj/1/1 HTTP/1.1" 500 -
Traceback (most recent call last):
  File
"/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
line 2000, in __call__
    return self.wsgi_app(environ, start_response)
  File
"/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
line 1991, in wsgi_app
    response = self.make_response(self.handle_exception(e))
  File
"/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
line 1567, in handle_exception
    reraise(exc_type, exc_value, tb)
  File
"/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
line 1988, in wsgi_app
    response = self.full_dispatch_request()
  File
"/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
line 1641, in full_dispatch_request
    rv = self.handle_user_exception(e)
  File
"/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
line 1544, in handle_user_exception
    reraise(exc_type, exc_value, tb)
  File
"/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
line 1639, in full_dispatch_request
    rv = self.dispatch_request()
  File
"/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/app.py",
line 1625, in dispatch_request
    return self.view_functions[rule.endpoint](**req.view_args)
  File
"/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packages/flask/views.py",
line 84, in view
    return self.dispatch_request(*args, **kwargs)
  File "/Users/dpage/git/pgadmin4/web/pgadmin/browser/utils.py", line 235,
in dispatch_request
    return method(*args, **kwargs)
  File
"/Users/dpage/git/pgadmin4/web/pgadmin/browser/server_groups/servers/__init__.py",
line 403, in update
    data = request.form if request.form else
json.loads(request.data.decode())
UnicodeDecodeError: 'ascii' codec can't decode byte 0xf0 in position 119:
ordinal not in range(128)

---

From: Murtuza Zabuawala <[email-masked]>
Date: Tue, 26 Jul 2016 19:04:11 +0530

Hi Dave,

PFA updated patch to fix encoding issue, I have tested in Python2/3 and
looks like working in both environment.

Almost all nodes are affected with this change, I tried to test & cover all
basic operations on nodes but still it requires some testing.

This patch will cover below tickets,
RM#1307
RM#1479

Please review.

---

From: Dave Page <[email-masked]>
Date: Tue, 26 Jul 2016 15:06:28 +0100

Hi

> This patch will cover below tickets,

I've committed this as-is, as it fixes things nicely as far as I can
see... with one exception: I couldn't create Functions with utf-8
names. Can you fix that please?

Thanks.

---

From: Dave Page <[email-masked]>
Date: Tue, 26 Jul 2016 17:02:58 +0100

> names. Can you fix that please?

I spoke too soon - Types don't play nicely either - try creating the
following in the UI.

CREATE TYPE public."🍺" AS ENUM
    ('a', 'b');

ALTER TYPE public."🍺"
    OWNER TO postgres;

Isn't there a way we can force everything to UTF-8 without having to
modify individual nodes? That's what we did in pgAdmin 3, then never
worried about the problem again.

---

From: Murtuza Zabuawala <[email-masked]>
Date: Wed, 27 Jul 2016 16:48:59 +0530

Hi Dave,

1) Unicode conversion was working properly for function node, It was
failing due to wrong sql templates.
- I've fixed them.
2) Type is also working fine with encoding for me(PFA screenshots), Can you
please provide error you got?

This patch also makes Provider optional in security label control &
templates.

-------
Regarding enforcing to utf-8, In my opinion we can enforce everything to
utf-8 if we use any of one python version but to make it compatible with
both python2 and python3 version by forcing utf-8 encoding it will be
difficult to handle its side effects.

For example,

*Python2,*

>>> txt_sample = 'Hello'
>>> isinstance(txt_sample, str)
True
>>> isinstance(txt_sample, bytes)
True
>>> isinstance(txt_sample, unicode)
False
>>> txt_sample.encode('utf-8')
'Hello'
>>> txt_sample.decode('utf-8')
u'Hello'

Forcing to unicode in Python2,

>>> txt_sample = u'Hello'
>>> isinstance(txt_sample, str)
False
>>> isinstance(txt_sample, bytes)
False
>>> isinstance(txt_sample, unicode)
True

*Python3,*

>>> txt_sample = 'Hello'
>>> isinstance(txt_sample, str)
True
>>> isinstance(txt_sample, bytes)
False
>>> txt_sample.encode('utf-8')
b'Hello'
>>> txt_sample.decode('utf-8')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: 'str' object has no attribute 'decode'

And also psycopg2 driver returns text as string in Python2 even if that
text contains unicode characters, In python3 that just works fine because
string is Unicode by default.

Regards,
Murtuza

---

From: Dave Page <[email-masked]>
Date: Wed, 27 Jul 2016 12:34:41 +0100

Hi

On Wed, Jul 27, 2016 at 12:18 PM, Murtuza Zabuawala
<[email-masked]> wrote:
> Hi Dave,
>
> 1) Unicode conversion was working properly for function node, It was failing
> due to wrong sql templates.
> - I've fixed them.

I just did a test, and it's still failing - see the attached
screenshot. Aside from the encoding issue, it looks like it's adding
extra quotes around the parameter data type - double-double quoting
the schema and type, and then quoting the whole lot again in the
CREATE part. The ALTER part looks fine.

> please provide error you got?

Hmm, I can't reproduce that now. Weird.

> difficult to handle its side effects.

Urgh. Ok, thanks.

> Python2,
> Python3,
> And also psycopg2 driver returns text as string in Python2 even if that text
> contains unicode characters, In python3 that just works fine because string
> is Unicode by default.
>
>
> Regards,
> Murtuza
>
> --
> Regards,
> Murtuza Zabuawala

---

From: Murtuza Zabuawala <[email-masked]>
Date: Wed, 27 Jul 2016 18:07:41 +0530

Okay, Let me check on that.

Meanwhile I'm attaching patch for making Provider optional in security
labels control.

Regards,
Murtuza

On Wed, Jul 27, 2016 at 5:04 PM, Dave Page <[email-masked]> wrote:

> text

---

From: Dave Page <[email-masked]>
Date: Wed, 27 Jul 2016 15:01:25 +0100

Thanks - applied!

---

From: Murtuza Zabuawala <[email-masked]>
Date: Thu, 28 Jul 2016 15:26:33 +0530

Hi Dave,

PFA updated patch to fix issue in functions which was due to improper
handing of nested quoting of types in templates.
I also fixed it in Table/Column node.

Please review.

Regards,
Murtuza

---

From: Dave Page <[email-masked]>
Date: Mon, 1 Aug 2016 14:21:29 +0100

Hi

> I also fixed it in Table/Column node.

This doesn't seem right to me:

                # If schema is not pg_catalog & then add schema
                if row['nspname'] != 'pg_catalog' \
                        and row['nspname'] == 'public':
                    rtn_type = row['nspname'] + "." + row['typname']
                else:
                    rtn_type = row['typname']

Shouldn't the "and row['nspname'] == 'public'" be removed?

---

From: Murtuza Zabuawala <[email-masked]>
Date: Mon, 1 Aug 2016 19:03:55 +0530

Hi Dave,

We have to added it because postgres format_type() function do not provide
us type with 'public' schema prefix to it, due to which wrong SQL's are
getting generated.
Rest other types are pre fixed with their respective schema as required.

Thanks,
Murtuza

---

From: Dave Page <[email-masked]>
Date: Mon, 1 Aug 2016 14:46:08 +0100

> Rest other types are pre fixed with their respective schema as required.

OK, so:

                # If schema is not pg_catalog & then add schema
                if row['nspname'] == 'public':
                    rtn_type = row['nspname'] + "." + row['typname']
                else:
                    rtn_type = row['typname']

?

!= pg_catalog is redundant with == public.

---

From: Dave Page <[email-masked]>
Date: Mon, 1 Aug 2016 15:24:19 +0100

On Mon, Aug 1, 2016 at 2:53 PM, Murtuza Zabuawala
<[email-masked]> wrote:
> Yes... :-)

OK, committed with the following:

                # If schema is public, prefix it. Otherwise, we should
already have it.
                if row['nspname'] == 'public':
                    rtn_type = "public." + row['typname']
                else:
                    rtn_type = row['typname']

> On Mon, Aug 1, 2016 at 7:16 PM, Dave Page <[email-masked]> wrote:
>>