Subject: [pgAdmin4][RM#3129] handle encoding issue in File manager

From: Murtuza Zabuawala <[email-masked]>
Date: Thu, 1 Mar 2018 13:30:29 +0530

Hi,

PFA patch to fix the issue where user was not able to open the file with
non utf-8 encoding.

--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
diff --git a/web/pgadmin/tools/sqleditor/__init__.py b/web/pgadmin/tools/sqleditor/__init__.py
index 8c17c97..0f3c909 100644
--- a/web/pgadmin/tools/sqleditor/__init__.py
+++ b/web/pgadmin/tools/sqleditor/__init__.py
@@ -38,6 +38,8 @@ from pgadmin.utils.exception import ConnectionLost
 from pgadmin.utils.sqlautocomplete.autocomplete import SQLAutoComplete
 from pgadmin.tools.sqleditor.utils.query_tool_preferences import \
     RegisterQueryToolPreferences
+from pgadmin.tools.sqleditor.utils.query_tool_fs_utils import \
+    read_file_generator
 
 MODULE_NAME = 'sqleditor'
 
@@ -1360,16 +1362,7 @@ def load_file():
             errormsg=gettext("File type not supported")
         )
 
-    def gen():
-        with codecs.open(file_path, 'r', encoding=enc) as fileObj:
-            while True:
-                # 4MB chunk (4 * 1024 * 1024 Bytes)
-                data = fileObj.read(4194304)
-                if not data:
-                    break
-                yield data
-
-    return Response(gen(), mimetype='text/plain')
+    return Response(read_file_generator(file_path, enc), mimetype='text/plain')

 @blueprint.route('/save_file/', methods=["PUT", "POST"], endpoint='save_file')
diff --git a/web/pgadmin/tools/sqleditor/utils/query_tool_fs_utils.py b/web/pgadmin/tools/sqleditor/utils/query_tool_fs_utils.py
new file mode 100644
index 0000000..7ed96e0
--- /dev/null
+++ b/web/pgadmin/tools/sqleditor/utils/query_tool_fs_utils.py
@@ -0,0 +1,53 @@
+##########################################################################
+#
+# pgAdmin 4 - PostgreSQL Tools
+#
+# Copyright (C) 2013 - 2018, The pgAdmin Development Team
+# This software is released under the PostgreSQL Licence
+#
+##########################################################################
+
+import codecs
+
+def read_file_generator(file, enc):
+    """
+    This will read the content of the file selected by user
+
+    Returns:
+        Content of file
+    """
+    try:
+        with codecs.open(file, 'r', encoding=enc) as fileObj:
+            while True:
+                # 4MB chunk (4 * 1024 * 1024 Bytes)
+                data = fileObj.read(4194304)
+                if not data:
+                    break
+                yield data
+    except UnicodeDecodeError:
+        # This is the closest equivalent Python 3 offers to the permissive
+        # Python 2 text handling model. The latin-1 encoding in Python
+        # implements ISO_8859-1:1987 which maps all possible byte values
+        # to the first 256 Unicode code points, and thus ensures decoding
+        # errors will never occur regardless of the configured error and
+        # handles most of the Windows encodings
+        # handler.
+        # Ref: https://goo.gl/vDhggS
+        with codecs.open(file, 'r', encoding='latin-1') as fileObj:
+            while True:
+                # 4MB chunk (4 * 1024 * 1024 Bytes)
+                data = fileObj.read(4194304)
+                if not data:
+                    break
+                yield data
+    except Exception:
+        # As a last resort we will use the provided encoding and then
+        # ignore the decoding errors
+        with codecs.open(file, 'r', encoding=enc, errors='ignore') as fileObj:
+            while True:
+                # 4MB chunk (4 * 1024 * 1024 Bytes)
+                data = fileObj.read(4194304)
+                if not data:
+                    break
+                yield data
+
diff --git a/web/pgadmin/tools/sqleditor/utils/tests/test_file_other_encoding.sql b/web/pgadmin/tools/sqleditor/utils/tests/test_file_other_encoding.sql
new file mode 100644
index 0000000..6e3bf3f
--- /dev/null
+++ b/web/pgadmin/tools/sqleditor/utils/tests/test_file_other_encoding.sql
@@ -0,0 +1,2 @@
+/*Copyright � 2017/*
+SELECT 1;
\ No newline at end of file
diff --git a/web/pgadmin/tools/sqleditor/utils/tests/test_file_utf8_encoding.sql b/web/pgadmin/tools/sqleditor/utils/tests/test_file_utf8_encoding.sql
new file mode 100644
index 0000000..7358741
--- /dev/null
+++ b/web/pgadmin/tools/sqleditor/utils/tests/test_file_utf8_encoding.sql
@@ -0,0 +1,2 @@
+/*Copyright © 2017/*
+SELECT 1;
\ No newline at end of file
diff --git a/web/pgadmin/tools/sqleditor/utils/tests/test_query_tool_fs_utils.py b/web/pgadmin/tools/sqleditor/utils/tests/test_query_tool_fs_utils.py
new file mode 100644
index 0000000..d4e8dc4
--- /dev/null
+++ b/web/pgadmin/tools/sqleditor/utils/tests/test_query_tool_fs_utils.py
@@ -0,0 +1,39 @@
+##########################################################################
+#
+# pgAdmin 4 - PostgreSQL Tools
+#
+# Copyright (C) 2013 - 2018, The pgAdmin Development Team
+# This software is released under the PostgreSQL Licence
+#
+##########################################################################
+import os
+from pgadmin.utils.route import BaseTestGenerator
+from pgadmin.tools.sqleditor.utils.query_tool_fs_utils import \
+    read_file_generator
+
+
+class TestReadFileGeneratorForEncoding(BaseTestGenerator):
+    """
+    Check that the start_running_query method works as intended
+    """
+
+    scenarios = [
+        ('When user is trying to load the file with utf-8 encoding', dict(
+            file='test_file_utf8_encoding.sql',
+            encoding='utf-8'
+        )),
+        ('When user is trying to load the file with other encoding and'
+         ' trying to use utf-8 encoding to read it', dict(
+            file='test_file_other_encoding.sql',
+            encoding='utf-8'
+        )),
+    ]
+
+    def setUp(self):
+        self.dir_path = os.path.dirname(os.path.realpath(__file__))
+        self.complate_path = os.path.join(self.dir_path, self.file)
+
+    def runTest(self):
+        result = read_file_generator(self.complate_path, self.encoding)
+        # Check if file is read properly by the generator
+        self.assertIn('SELECT 1', next(result))

---

From: Joao De Almeida Pereira <[email-masked]>
Date: Thu, 01 Mar 2018 15:26:06 +0000

Hello Murtuza,

The code looks pretty good, love the fact that you extracted it so that our
file size stop growing and get more manageable. All tests pass on our CI.

The only issue I found was linting:

pycodestyle --config=.pycodestyle pgadmin/tools/sqleditor/
pgadmin/tools/sqleditor/utils/query_tool_fs_utils.py:12: [E302] expected 2
blank lines, found 1
pgadmin/tools/sqleditor/utils/query_tool_fs_utils.py:53: [W391] blank line
at end of file
pgadmin/tools/sqleditor/utils/tests/test_query_tool_fs_utils.py:27: [E121]
continuation line under-indented for hanging indent
pgadmin/tools/sqleditor/utils/tests/test_query_tool_fs_utils.py:29: [E122]
continuation line missing indentation or outdented
1       E121 continuation line under-indented for hanging indent
1       E122 continuation line missing indentation or outdented
1       E302 expected 2 blank lines, found 1
1       W391 blank line at end of file
4

When this is fixed I think we are good to go

Thanks
Joao

---

From: Murtuza Zabuawala <[email-masked]>
Date: Thu, 1 Mar 2018 21:26:02 +0530

Thanks Joao for reviewing.

Attaching updated patch fixing PEP8 issues.

On Thu, Mar 1, 2018 at 8:56 PM, Joao De Almeida Pereira <
[email-masked]> wrote:

> Hello Murtuza,
>
> The code looks pretty good, love the fact that you extracted it so that
> our file size stop growing and get more manageable. All tests pass on our
> CI.
>
> pgadmin/tools/sqleditor/utils/query_tool_fs_utils.py:12: [E302] expected
> 2 blank lines, found 1
> pgadmin/tools/sqleditor/utils/query_tool_fs_utils.py:53: [W391] blank
> line at end of file
> pgadmin/tools/sqleditor/utils/tests/test_query_tool_fs_utils.py:27:
> [E121] continuation line under-indented for hanging indent
> pgadmin/tools/sqleditor/utils/tests/test_query_tool_fs_utils.py:29:
> [E122] continuation line missing indentation or outdented
> On Thu, Mar 1, 2018 at 3:01 AM Murtuza Zabuawala <murtuza.zabuawala@
> enterprisedb.com> wrote:
>
>> Hi,
>>

 
-    def gen():

index 0000000..ad1df0f
index 0000000..7cab067
@@ -0,0 +1,45 @@
+        (
+            'When user is trying to load the file with utf-8 encoding',
+            dict(
+            )
+        ),
+        (
+            'When user is trying to load the file with other encoding and'
+            ' trying to use utf-8 encoding to read it',
+            dict(
+            )
+        ),

---

From: Joao De Almeida Pereira <[email-masked]>
Date: Thu, 01 Mar 2018 22:11:28 +0000

Hello Joao,
The pipeline is green and I believe the change is good to be merged.

Thanks
Joao

---

From: Dave Page <[email-masked]>
Date: Fri, 2 Mar 2018 13:37:12 +0000

Thanks guys, patch applied.

On Thu, Mar 1, 2018 at 10:11 PM, Joao De Almeida Pereira <
[email-masked]> wrote:

> On Thu, Mar 1, 2018 at 10:56 AM Murtuza Zabuawala <murtuza.zabuawala@
> enterprisedb.com> wrote:
>
>>> pgadmin/tools/sqleditor/utils/query_tool_fs_utils.py:12: [E302]
>>> expected 2 blank lines, found 1
>>>> PFA patch to fix the issue where user was not able to open the file
>>>> with non utf-8 encoding.
>>>>
>>>>
>>>> --
>>>> Regards,
>>>> Murtuza Zabuawala

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

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