Browse Source

HUE-8737 [fb] Fix urllib.parse and io issue in py2

Ying Chen 6 years ago
parent
commit
1cd53c868a

+ 7 - 2
apps/filebrowser/src/filebrowser/forms.py

@@ -20,7 +20,8 @@ standard_library.install_aliases()
 from builtins import zip
 from builtins import range
 import logging
-import urllib.request, urllib.parse, urllib.error
+import sys
+import urllib.request, urllib.error
 
 from django import forms
 from django.contrib.auth.models import User, Group
@@ -34,6 +35,10 @@ from filebrowser.lib import rwx
 
 from django.utils.translation import ugettext_lazy as _
 
+if sys.version_info[0] > 2:
+  from urllib.parse import unquote as urllib_unquote
+else:
+  from urllib import unquote as urllib_unquote
 
 logger = logging.getLogger(__name__)
 
@@ -79,7 +84,7 @@ class EditorForm(forms.Form):
   encoding = CharField(label=_('Encoding'), required=False)
 
   def clean_path(self):
-    return urllib.parse.unquote(self.cleaned_data.get('path', ''))
+    return urllib_unquote(self.cleaned_data.get('path', ''))
 
   def clean_contents(self):
     return self.cleaned_data.get('contents', '').replace('\r\n', '\n')

+ 9 - 4
apps/filebrowser/src/filebrowser/lib/xxd_test.py

@@ -23,15 +23,20 @@ from builtins import range
 import unittest
 import logging
 import random
-import io
+import sys
 import subprocess
 
-from . import xxd
+from filebrowser.lib import xxd
 
 from nose.plugins.skip import SkipTest
 
 from subprocess import Popen, PIPE
 
+if sys.version_info[0] > 2:
+  from io import StringIO as string_io
+else:
+  from cStringIO import StringIO as string_io
+
 LOG = logging.getLogger(__name__)
 
 LENGTH = 1024*10 # 10KB
@@ -93,8 +98,8 @@ class XxdTest(unittest.TestCase):
     (stdin, stderr) = p.communicate(random_text)
     self.assertFalse(stderr)
 
-    output = io.StringIO()
-    xxd.main(io.StringIO(random_text), output)
+    output = string_io()
+    xxd.main(string_io(random_text), output)
     self._verify_content(stdin, output.getvalue())
 
 if __name__ == "__main__":

+ 46 - 37
apps/filebrowser/src/filebrowser/views.py

@@ -28,12 +28,12 @@ import parquet
 import posixpath
 import re
 import stat as stat_module
-import urllib.request, urllib.parse, urllib.error
+import sys
+import urllib.request, urllib.error
 from urllib.parse import urlparse
 
 from bz2 import decompress
 from datetime import datetime
-from io import StringIO
 from gzip import GzipFile
 
 from django.contrib.auth.models import User, Group
@@ -81,6 +81,15 @@ from filebrowser.forms import RenameForm, UploadFileForm, UploadArchiveForm, MkD
 
 from desktop.auth.backend import is_admin
 
+if sys.version_info[0] > 2:
+  from io import string_io as string_io
+  from urllib.parse import quote as urllib_quote
+  from urllib.parse import unquote as urllib_unquote
+else:
+  from cStringIO import StringIO as string_io
+  from urllib import quote as urllib_quote
+  from urllib import unquote as urllib_unquote
+
 DEFAULT_CHUNK_SIZE_BYTES = 1024 * 4 # 4KB
 MAX_CHUNK_SIZE_BYTES = 1024 * 1024 # 1MB
 
@@ -130,7 +139,7 @@ def download(request, path):
     This is inspired by django.views.static.serve.
     ?disposition={attachment, inline}
     """
-    decoded_path = urllib.parse.unquote(path)
+    decoded_path = urllib_unquote(path)
     if path != decoded_path:
       path = decoded_path
     if not SHOW_DOWNLOAD_BUTTON.get():
@@ -179,23 +188,23 @@ def download(request, path):
 
 def view(request, path):
     """Dispatches viewing of a path to either index() or fileview(), depending on type."""
-    decoded_path = urllib.parse.unquote(path)
+    decoded_path = urllib_unquote(path)
     if path != decoded_path:
       path = decoded_path
     # default_to_home is set in bootstrap.js
     if 'default_to_home' in request.GET:
         home_dir_path = request.user.get_home_directory()
         if request.fs.isdir(home_dir_path):
-            return format_preserving_redirect(request, '/filebrowser/view=' + urllib.parse.quote(home_dir_path.encode('utf-8'), safe=SAFE_CHARACTERS_URI_COMPONENTS))
+            return format_preserving_redirect(request, '/filebrowser/view=' + urllib_quote(home_dir_path.encode('utf-8'), safe=SAFE_CHARACTERS_URI_COMPONENTS))
 
     # default_to_home is set in bootstrap.js
     if 'default_to_trash' in request.GET:
         home_trash_path = _home_trash_path(request.fs, request.user, path)
         if request.fs.isdir(home_trash_path):
-            return format_preserving_redirect(request, '/filebrowser/view=' + urllib.parse.quote(home_trash_path.encode('utf-8'), safe=SAFE_CHARACTERS_URI_COMPONENTS))
+            return format_preserving_redirect(request, '/filebrowser/view=' + urllib_quote(home_trash_path.encode('utf-8'), safe=SAFE_CHARACTERS_URI_COMPONENTS))
         trash_path = request.fs.trash_path(path)
         if request.fs.isdir(trash_path):
-            return format_preserving_redirect(request, '/filebrowser/view=' + urllib.parse.quote(trash_path.encode('utf-8'), safe=SAFE_CHARACTERS_URI_COMPONENTS))
+            return format_preserving_redirect(request, '/filebrowser/view=' + urllib_quote(trash_path.encode('utf-8'), safe=SAFE_CHARACTERS_URI_COMPONENTS))
 
     try:
         stats = request.fs.stats(path)
@@ -236,7 +245,7 @@ def _home_trash_path(fs, user, path):
 
 
 def home_relative_view(request, path):
-  decoded_path = urllib.parse.unquote(path)
+  decoded_path = urllib_unquote(path)
   if path != decoded_path:
     path = decoded_path
   home_dir_path = request.user.get_home_directory()
@@ -248,7 +257,7 @@ def home_relative_view(request, path):
 
 def edit(request, path, form=None):
     """Shows an edit form for the given path. Path does not necessarily have to exist."""
-    decoded_path = urllib.parse.unquote(path)
+    decoded_path = urllib_unquote(path)
     if path != decoded_path:
       path = decoded_path
     try:
@@ -306,7 +315,7 @@ def save_file(request):
     form = EditorForm(request.POST)
     is_valid = form.is_valid()
     path = form.cleaned_data.get('path')
-    decoded_path = urllib.parse.unquote(path)
+    decoded_path = urllib_unquote(path)
     if path != decoded_path:
       path = decoded_path
 
@@ -345,7 +354,7 @@ def parse_breadcrumbs(path):
       if url and not url.endswith('/'):
         url += '/'
       url += part
-      breadcrumbs.append({'url': urllib.parse.quote(url.encode('utf-8'), safe=SAFE_CHARACTERS_URI_COMPONENTS), 'label': part})
+      breadcrumbs.append({'url': urllib_quote(url.encode('utf-8'), safe=SAFE_CHARACTERS_URI_COMPONENTS), 'label': part})
     return breadcrumbs
 
 
@@ -355,7 +364,7 @@ def listdir(request, path):
 
     Intended to be called via view().
     """
-    decoded_path = urllib.parse.unquote(path)
+    decoded_path = urllib_unquote(path)
     if path != decoded_path:
       path = decoded_path
     if not request.fs.isdir(path):
@@ -373,8 +382,8 @@ def listdir(request, path):
         'path': path,
         'file_filter': file_filter,
         'breadcrumbs': breadcrumbs,
-        'current_dir_path': urllib.parse.quote(path.encode('utf-8'), safe=SAFE_CHARACTERS_URI),
-        'current_request_path': '/filebrowser/view=' + urllib.parse.quote(path.encode('utf-8'), safe=SAFE_CHARACTERS_URI_COMPONENTS),
+        'current_dir_path': urllib_quote(path.encode('utf-8'), safe=SAFE_CHARACTERS_URI),
+        'current_request_path': '/filebrowser/view=' + urllib_quote(path.encode('utf-8'), safe=SAFE_CHARACTERS_URI_COMPONENTS),
         'home_directory': home_dir_path if home_dir_path and request.fs.isdir(home_dir_path) else None,
         'cwd_set': True,
         'is_superuser': request.user.username == request.fs.superuser,
@@ -437,7 +446,7 @@ def listdir_paged(request, path):
       filter=?          - Specify a substring filter to search for in
                           the filename field.
     """
-    decoded_path = urllib.parse.unquote(path)
+    decoded_path = urllib_unquote(path)
     if path != decoded_path:
       path = decoded_path
     if not request.fs.isdir(path):
@@ -520,7 +529,7 @@ def listdir_paged(request, path):
     data = {
         'path': path,
         'breadcrumbs': breadcrumbs,
-        'current_request_path': '/filebrowser/view=' + urllib.parse.quote(path.encode('utf-8'), safe=SAFE_CHARACTERS_URI_COMPONENTS),
+        'current_request_path': '/filebrowser/view=' + urllib_quote(path.encode('utf-8'), safe=SAFE_CHARACTERS_URI_COMPONENTS),
         'is_trash_enabled': is_trash_enabled,
         'files': page.object_list if page else [],
         'page': _massage_page(page, paginator) if page else {},
@@ -530,7 +539,7 @@ def listdir_paged(request, path):
         # The following should probably be deprecated
         'cwd_set': True,
         'file_filter': 'any',
-        'current_dir_path': urllib.parse.quote(path.encode('utf-8'), safe=SAFE_CHARACTERS_URI),
+        'current_dir_path': urllib_quote(path.encode('utf-8'), safe=SAFE_CHARACTERS_URI),
         'is_fs_superuser': is_fs_superuser,
         'groups': is_fs_superuser and [str(x) for x in Group.objects.values_list('name', flat=True)] or [],
         'users': is_fs_superuser and [str(x) for x in User.objects.values_list('username', flat=True)] or [],
@@ -571,7 +580,7 @@ def _massage_stats(request, stats):
         'type': filetype(stats['mode']),
         'rwx': rwx(stats['mode'], stats['aclBit']),
         'mode': stringformat(stats['mode'], "o"),
-        'url': '/filebrowser/view=' + urllib.parse.quote(normalized.encode('utf-8'), safe=SAFE_CHARACTERS_URI_COMPONENTS),
+        'url': '/filebrowser/view=' + urllib_quote(normalized.encode('utf-8'), safe=SAFE_CHARACTERS_URI_COMPONENTS),
         'is_sentry_managed': request.fs.is_sentry_managed(path)
     }
 
@@ -583,7 +592,7 @@ def stat(request, path):
     Intended for use via AJAX (and hence doesn't provide
     an HTML view).
     """
-    decoded_path = urllib.parse.unquote(path)
+    decoded_path = urllib_unquote(path)
     if path != decoded_path:
       path = decoded_path
     if not request.fs.exists(path):
@@ -593,7 +602,7 @@ def stat(request, path):
 
 
 def content_summary(request, path):
-    decoded_path = urllib.parse.unquote(path)
+    decoded_path = urllib_unquote(path)
     if path != decoded_path:
       path = decoded_path
     if not request.fs.exists(path):
@@ -625,7 +634,7 @@ def display(request, path):
     sequence files, decompress gzipped text files, etc.).
     There exists a python-magic package to interface with libmagic.
     """
-    decoded_path = urllib.parse.unquote(path)
+    decoded_path = urllib_unquote(path)
     if path != decoded_path:
       path = decoded_path
     if not request.fs.isfile(path):
@@ -747,7 +756,7 @@ def read_contents(codec_type, path, fs, offset, length):
     """
     contents = ''
     fhandle = None
-    decoded_path = urllib.parse.unquote(path)
+    decoded_path = urllib_unquote(path)
     if path != decoded_path:
       path = decoded_path
     try:
@@ -813,7 +822,7 @@ def _read_snappy(fhandle, path, offset, length, stats):
     if stats.size > MAX_SNAPPY_DECOMPRESSION_SIZE.get():
         raise PopupException(_('Failed to decompress snappy compressed file. File size is greater than allowed max snappy decompression size of %d.') % MAX_SNAPPY_DECOMPRESSION_SIZE.get())
 
-    return _read_simple(StringIO(_decompress_snappy(fhandle.read())), path, offset, length, stats)
+    return _read_simple(string_io(_decompress_snappy(fhandle.read())), path, offset, length, stats)
 
 
 def _read_avro(fhandle, path, offset, length, stats):
@@ -846,9 +855,9 @@ def _read_avro(fhandle, path, offset, length, stats):
 def _read_parquet(fhandle, path, offset, length, stats):
     try:
         size = 1 * 128 * 1024 * 1024  # Buffer file stream to 128 MB chunks
-        data = StringIO(fhandle.read(size))
+        data = string_io(fhandle.read(size))
 
-        dumped_data = StringIO()
+        dumped_data = string_io()
         parquet._dump(data, ParquetOptions(limit=1000), out=dumped_data)
         dumped_data.seek(offset)
         return dumped_data.read()
@@ -862,7 +871,7 @@ def _read_gzip(fhandle, path, offset, length, stats):
     if offset and offset != 0:
         raise PopupException(_("Offsets are not supported with Gzip compression."))
     try:
-        contents = GzipFile('', 'r', 0, StringIO(fhandle.read())).read(length)
+        contents = GzipFile('', 'r', 0, string_io(fhandle.read())).read(length)
     except Exception as e:
         logging.exception('Could not decompress file at "%s": %s' % (path, e))
         raise PopupException(_("Failed to decompress file."))
@@ -1136,7 +1145,7 @@ def rename(request):
           raise PopupException(_("Could not rename folder \"%s\" to \"%s\": Hashes are not allowed in filenames." % (src_path, dest_path)))
         if "/" not in dest_path:
             src_dir = os.path.dirname(src_path)
-            dest_path = request.fs.join(urllib.parse.unquote(src_dir), urllib.parse.unquote(dest_path))
+            dest_path = request.fs.join(urllib_unquote(src_dir), urllib_unquote(dest_path))
         if request.fs.exists(dest_path):
           raise PopupException(_('The destination path "%s" already exists.') % dest_path)
         request.fs.rename(src_path, dest_path)
@@ -1145,7 +1154,7 @@ def rename(request):
 
 def set_replication(request):
     def smart_set_replication(src_path, replication_factor):
-        result = request.fs.set_replication(urllib.parse.unquote(src_path), replication_factor)
+        result = request.fs.set_replication(urllib_unquote(src_path), replication_factor)
         if not result:
             raise PopupException(_("Setting of replication factor failed"))
 
@@ -1158,7 +1167,7 @@ def mkdir(request):
         # No absolute directory specification allowed.
         if posixpath.sep in name or "#" in name:
             raise PopupException(_("Could not name folder \"%s\": Slashes or hashes are not allowed in filenames." % name))
-        request.fs.mkdir(request.fs.join(urllib.parse.unquote(path), urllib.parse.unquote(name)))
+        request.fs.mkdir(request.fs.join(urllib_unquote(path), urllib_unquote(name)))
 
     return generic_op(MkDirForm, request, smart_mkdir, ["path", "name"], "path")
 
@@ -1168,7 +1177,7 @@ def touch(request):
         # No absolute path specification allowed.
         if posixpath.sep in name:
             raise PopupException(_("Could not name file \"%s\": Slashes are not allowed in filenames." % name))
-        request.fs.create(request.fs.join(urllib.parse.unquote(path), urllib.parse.unquote(name)))
+        request.fs.create(request.fs.join(urllib_unquote(path), urllib_unquote(name)))
 
     return generic_op(TouchForm, request, smart_touch, ["path", "name"], "path")
 
@@ -1178,7 +1187,7 @@ def rmtree(request):
     params = ["path"]
     def bulk_rmtree(*args, **kwargs):
         for arg in args:
-            request.fs.do_as_user(request.user, request.fs.rmtree, urllib.parse.unquote(arg['path']), 'skip_trash' in request.GET)
+            request.fs.do_as_user(request.user, request.fs.rmtree, urllib_unquote(arg['path']), 'skip_trash' in request.GET)
     return generic_op(RmTreeFormSet, request, bulk_rmtree, ["path"], None,
                       data_extractor=formset_data_extractor(recurring, params),
                       arg_extractor=formset_arg_extractor,
@@ -1193,7 +1202,7 @@ def move(request):
         for arg in args:
             if arg['src_path'] == arg['dest_path']:
                 raise PopupException(_('Source path and destination path cannot be same'))
-            request.fs.rename(urllib.parse.unquote(arg['src_path']), urllib.parse.unquote(arg['dest_path']))
+            request.fs.rename(urllib_unquote(arg['src_path']), urllib_unquote(arg['dest_path']))
     return generic_op(RenameFormSet, request, bulk_move, ["src_path", "dest_path"], None,
                       data_extractor=formset_data_extractor(recurring, params),
                       arg_extractor=formset_arg_extractor,
@@ -1208,7 +1217,7 @@ def copy(request):
         for arg in args:
             if arg['src_path'] == arg['dest_path']:
                 raise PopupException(_('Source path and destination path cannot be same'))
-            request.fs.copy(urllib.parse.unquote(arg['src_path']), urllib.parse.unquote(arg['dest_path']), recursive=True, owner=request.user)
+            request.fs.copy(urllib_unquote(arg['src_path']), urllib_unquote(arg['dest_path']), recursive=True, owner=request.user)
     return generic_op(CopyFormSet, request, bulk_copy, ["src_path", "dest_path"], None,
                       data_extractor=formset_data_extractor(recurring, params),
                       arg_extractor=formset_arg_extractor,
@@ -1222,7 +1231,7 @@ def chmod(request):
     def bulk_chmod(*args, **kwargs):
         op = curry(request.fs.chmod, recursive=request.POST.get('recursive', False))
         for arg in args:
-            op(urllib.parse.unquote(arg['path']), arg['mode'])
+            op(urllib_unquote(arg['path']), arg['mode'])
     # mode here is abused: on input, it's a string, but when retrieved,
     # it's an int.
     return generic_op(ChmodFormSet, request, bulk_chmod, ['path', 'mode'], "path",
@@ -1247,7 +1256,7 @@ def chown(request):
     def bulk_chown(*args, **kwargs):
         op = curry(request.fs.chown, recursive=request.POST.get('recursive', False))
         for arg in args:
-            varg = [urllib.parse.unquote(arg[param]) for param in param_names]
+            varg = [urllib_unquote(arg[param]) for param in param_names]
             op(*varg)
 
     return generic_op(ChownFormSet, request, bulk_chown, param_names, "path",
@@ -1262,7 +1271,7 @@ def trash_restore(request):
     params = ["path"]
     def bulk_restore(*args, **kwargs):
         for arg in args:
-            request.fs.do_as_user(request.user, request.fs.restore, urllib.parse.unquote(arg['path']))
+            request.fs.do_as_user(request.user, request.fs.restore, urllib_unquote(arg['path']))
     return generic_op(RestoreFormSet, request, bulk_restore, ["path"], None,
                       data_extractor=formset_data_extractor(recurring, params),
                       arg_extractor=formset_arg_extractor,
@@ -1311,7 +1320,7 @@ def _upload_file(request):
 
     if form.is_valid():
         uploaded_file = request.FILES['hdfs_file']
-        dest = scheme_absolute_path(urllib.parse.unquote(request.GET['dest']), urllib.parse.unquote(form.cleaned_data['dest']))
+        dest = scheme_absolute_path(urllib_unquote(request.GET['dest']), urllib_unquote(form.cleaned_data['dest']))
         filepath = request.fs.join(dest, uploaded_file.name)
 
         if request.fs.isdir(dest) and posixpath.sep in uploaded_file.name:

+ 8 - 3
apps/filebrowser/src/filebrowser/views_test.py

@@ -26,8 +26,9 @@ import json
 import logging
 import os
 import re
+import sys
 import tempfile
-import urllib.request, urllib.parse, urllib.error
+import urllib.request, urllib.error
 import urllib.parse
 from avro import schema, datafile, io
 
@@ -49,6 +50,10 @@ from filebrowser.conf import ENABLE_EXTRACT_UPLOADED_ARCHIVE, MAX_SNAPPY_DECOMPR
 from filebrowser.lib.rwx import expand_mode
 from filebrowser.views import snappy_installed
 
+if sys.version_info[0] > 2:
+  from urllib.parse import unquote as urllib_unquote
+else:
+  from urllib import unquote as urllib_unquote
 
 LOG = logging.getLogger(__name__)
 
@@ -440,7 +445,7 @@ class TestFileBrowserWithHadoop(object):
     assert_equal(stat_dir, response.context[0]['path'])
 
     response = self.c.get('/filebrowser/view=/test-filebrowser/?default_to_home')
-    assert_true(re.search('%s$' % home, urllib.parse.unquote(response['Location'])))
+    assert_true(re.search('%s$' % home, urllib_unquote(response['Location'])))
 
     # Test path relative to home directory
     self.cluster.fs.do_as_user('test', self.cluster.fs.mkdir, '%s/test_dir' % home)
@@ -776,7 +781,7 @@ class TestFileBrowserWithHadoop(object):
     response = self.c.get('/filebrowser/view=/')
     assert_equal(response.context[0]['path'], '/')
     response = self.c.get('/filebrowser/view=/?default_to_home=1')
-    assert_equal("/filebrowser/view=/user/test", urllib.parse.unquote(response["location"]))
+    assert_equal("/filebrowser/view=/user/test", urllib_unquote(response["location"]))
 
 
   def test_view_access(self):