浏览代码

[filebrowser] major encoding cleanup to support all cases of % in file and folder names

- Basic principle behinde cleanup is that the path encoding between the frontend and the backend (i.e. js and python) are consistent
- Fixed aditional issues with special characters found.
- Reverted some special case fixes for encoding in order to use a more coherent strategy
Björn Alm 3 年之前
父节点
当前提交
07a882c59c

+ 18 - 22
apps/filebrowser/src/filebrowser/templates/display.mako

@@ -29,7 +29,7 @@
     from urllib import quote as urllib_quote
     from django.utils.translation import ugettext as _
 %>
-<%
+<%  
   path_enc = urllib_quote(path.encode('utf-8'), safe=SAFE_CHARACTERS_URI_COMPONENTS)
   dirname_enc = urlencode(view['dirname'])
   base_url = url('filebrowser:filebrowser.views.view', path=path_enc)
@@ -52,7 +52,7 @@ ${ fb_components.menubar() }
         <!-- ko if: $root.file -->
         <ul class="nav nav-list">
           <!-- ko if: $root.isViewing -->
-            <li><a href="${url('filebrowser:filebrowser.views.view', path=dirname_enc)}"><i class="fa fa-reply"></i> ${_('Back')}</a></li>
+            <li><a  data-bind="click: goToParentDirectory" href=""><i class="fa fa-reply"></i> ${_('Back')}</a></li>
 
             <!-- ko if: $root.file().view.compression() && $root.file().view.compression() === "none" && $root.file().editable -->
               <li><a class="pointer" data-bind="click: $root.editFile"><i class="fa fa-pencil"></i> ${_('Edit file')}</a></li>
@@ -240,8 +240,13 @@ ${ fb_components.menubar() }
   }
 
   function getContent (callback) {
-    var _baseUrl = "${url('filebrowser:filebrowser.views.view', path=path_enc)}";
-
+    // We don't use the python variable path_enc here since that will 
+    // produce a double encoded path after calling the python url function
+    const decodedPath = "${path | n}";
+    const encodedPath = encodeURIComponent(decodedPath);
+    const pathPrefix = "/filebrowser/view=";
+    const contentPath = pathPrefix+encodedPath;
+    
     viewModel.isLoading(true);
 
     var startPage = viewModel.page();
@@ -254,9 +259,7 @@ ${ fb_components.menubar() }
       mode: viewModel.mode()
     };
 
-    contentUrl = fixSpecialCharacterEncoding(_baseUrl);
-
-    $.getJSON(contentUrl, params, function (data) {
+    $.getJSON(contentPath, params, function (data) {
       var _html = "";
 
       viewModel.file(ko.mapping.fromJS(data, { 'ignore': ['view.contents', 'view.xxd'] }));
@@ -307,6 +310,10 @@ ${ fb_components.menubar() }
   function DisplayViewModel (params) {
     var self = this;
 
+    self.goToParentDirectory = function () {
+      huePubSub.publish('open.filebrowserlink', { pathPrefix: "/filebrowser/view=", decodedPath: "${view['dirname'] | n}" });
+    }
+
     self.changePage = function () {
       renderPages();
       getContent(function () {
@@ -372,8 +379,10 @@ ${ fb_components.menubar() }
     self.editFile = function() {
       self.isViewing(false);
       self.isLoading(true);
+      
+      const encodedPath = encodeURIComponent("${path | n}");      
       $.ajax({
-        url: '${ edit_url }' + '?is_embeddable=true',
+        url: '/filebrowser/edit=' + encodedPath + '?is_embeddable=true',
         beforeSend:function (xhr) {
           xhr.setRequestHeader('X-Requested-With', 'Hue');
         },
@@ -394,20 +403,7 @@ ${ fb_components.menubar() }
     }
 
     self.downloadFile = function () {
-      const filePath = "${path}";
-      const correctedFilePath = fixSpecialCharacterEncoding(filePath);
-      // If the hash characters aren't encoded the page library will
-      // split the path on the first occurence and the remaining string will not
-      // be part of the path. Question marks must also be encoded or the string after the first
-      // question mark will be interpreted as the url querystring.
-      // Percentage character must be double encoded due to the page library that decodes the url twice
-      const doubleUrlEncodedPercentage = '%2525';
-      const partiallyEncodedFilePath = correctedFilePath.replaceAll('%', doubleUrlEncodedPercentage).replaceAll('#', encodeURIComponent('#'))
-        .replaceAll('?', encodeURIComponent('?'));
-      const baseUrl = "${url('filebrowser:filebrowser_views_download', path='')}";
-      const fullUrl = baseUrl+partiallyEncodedFilePath;
-
-      huePubSub.publish('open.link', fullUrl);
+      huePubSub.publish('open.filebrowserlink', { pathPrefix: "/filebrowser/download=", decodedPath:  "${path | n}" });      
     };
 
     self.pageChanged = function () {

+ 10 - 5
apps/filebrowser/src/filebrowser/templates/fb_components.mako

@@ -68,7 +68,7 @@ else:
               <a data-bind="text: label, click: show, attr: {'href': '${url('filebrowser:filebrowser.views.view', path=urlencode(''))}' + url}"></a><span class="divider">/</span>
             </li
           ></ul>
-          <input id="hueBreadcrumbText" type="text" style="display:none" data-bind="value: currentDecodedPath" autocomplete="off" class="input-xxlarge" />          
+          <input id="hueBreadcrumbText" type="text" style="display:none" data-bind="value: currentPath" autocomplete="off" class="input-xxlarge" />
         </li>
         % if is_trash_enabled:
         <li class="pull-right">
@@ -80,16 +80,21 @@ else:
       </ul>
     % else:
       <ul class="nav nav-pills hue-breadcrumbs-bar">
-        <li><a data-bind="hueLink: window.HUE_BASE_URL + '/filebrowser/view='+ window.USER_HOME_DIR +'?default_to_home'" class="breadcrumb-link homeLink"><i class="fa fa-home"></i> ${_('Home')}</a></li>
+        <li>
+          <a data-bind="hueLink: window.HUE_BASE_URL + '/filebrowser/view='+ window.USER_HOME_DIR +'?default_to_home'" class="breadcrumb-link homeLink">
+            <i class="fa fa-home"></i> ${_('Home')}
+          </a>
+        </li>
         <li>
           <ul class="hue-breadcrumbs" style="padding-right:40px; padding-top: 12px">
           % for breadcrumb_item in breadcrumbs:
             <% label, f_url = breadcrumb_item['label'], breadcrumb_item['url'] %>
             %if label[-1] == '/':
-            ## Backticks are used as quotes to handle single quote in f_url
-            <li><a data-bind="hueLink: `${'/filebrowser/view=' + f_url}`"><span class="divider">${label}</span></a></li>
+            <li><a href="javascript: void(0)" data-bind="click: ()=> {
+              huePubSub.publish('open.filebrowserlink', { pathPrefix: '/filebrowser/view=', decodedPath: `${f_url | n}` });}"><span class="divider">${label}</span></a></li>
             %else:
-            <li><a data-bind="hueLink: `${'/filebrowser/view=' + f_url}`">${label}</a><span class="divider">/</span></li>
+            <li><a href="javascript: void(0)" data-bind="click: ()=> {
+              huePubSub.publish('open.filebrowserlink', { pathPrefix: '/filebrowser/view=', decodedPath: `${f_url | n}` });}">${label}</a><span class="divider">/</span></li>
             %endif
           % endfor
           </ul>

+ 27 - 119
apps/filebrowser/src/filebrowser/templates/listdir_components.mako

@@ -443,7 +443,7 @@ else:
   <div id="uploadFileModal" class="modal hide fade">
     <div class="modal-header">
       <button type="button" class="close" data-dismiss="modal" aria-label="${ _('Close') }"><span aria-hidden="true">&times;</span></button>
-      <h2 class="modal-title">${_('Upload to')} <span data-bind="text: currentDecodedPath"></span></h2>
+      <h2 class="modal-title">${_('Upload to')} <span data-bind="text: currentPath"></span></h2>
     </div>
     <div class="modal-body form-inline">
       <div id="fileUploader" class="uploader">
@@ -801,16 +801,6 @@ else:
       return (bytes / Math.pow(k, i)).toPrecision(dm) + ' ' + sizes[i];
     }
 
-    var updateHash = function (hash) {
-      hash = encodeURI(decodeURIComponent(hash));
-      %if not is_embeddable:
-      window.location.hash = hash;
-      %else:
-      hueUtils.changeURL('#' + hash);
-      huePubSub.publish('fb.update.hash');
-      %endif
-    }
-
     var Page = function (page) {
       if (page != null) {
         return {
@@ -916,11 +906,10 @@ else:
               if (this.url == null || this.url == "") {
                 // forcing root on empty breadcrumb url
                 this.url = "/";
-              }
-
-              fileBrowserViewModel.targetPageNum(1);
-              fileBrowserViewModel.targetPath("${url('filebrowser:filebrowser.views.view', path='')}" + stripHashes(this.url));
-              updateHash(this.url);
+              }              
+              fileBrowserViewModel.targetPageNum(1);              
+              const pathPrefix = "${url('filebrowser:filebrowser.views.view', path='')}";
+              huePubSub.publish('open.filebrowserlink', { pathPrefix, decodedPath: this.url, fileBrowserModel: fileBrowserViewModel});              
             }
             else {
               window.open($(e.target).attr('href'));
@@ -1055,16 +1044,8 @@ else:
       };
 
       self.currentPath = ko.observable(currentDirPath);
-      self.currentDecodedPath = ko.observable(decodeURI(currentDirPath));      
-
-      self.currentPath.subscribe(function (path) {
-        // currentEditablePath can be edited by the user in #hueBreadcrumbText.
-        // It is using a decoded currentPath in order to correctly display non ASCII characters        
-        const decodedPath = decodeURI(path);
-        if (decodedPath !== self.currentDecodedPath()) {          
-          self.currentDecodedPath(decodedPath);
-        } 
-        
+
+      self.currentPath.subscribe(function (path) {        
         $(document).trigger('currPathLoaded', { path: path });
       });
 
@@ -1171,7 +1152,7 @@ else:
       self.showSummary = function () {
         self.isLoadingSummary(true);
         $("#contentSummaryModal").modal("show");
-        $.getJSON("${url('filebrowser:content_summary', path='')}" + self.selectedFile().path, function (data) {
+        $.getJSON("${url('filebrowser:content_summary', path='')}" + encodeURIComponent(self.selectedFile().path), function (data) {
           if (data.status == 0) {
             self.contentSummary(ko.mapping.fromJS(data.summary));
             self.isLoadingSummary(false);
@@ -1191,8 +1172,8 @@ else:
 
       self.retrieveData = function (clearAssistCache) {
         self.isLoading(true);
-
-        $.getJSON(self.targetPath() + (self.targetPath().indexOf('?') > 0 ? '&' : '?') + "pagesize=" + self.recordsPerPage() + "&pagenum=" + self.targetPageNum() + "&filter=" + self.searchQuery() + "&sortby=" + self.sortBy() + "&descending=" + self.sortDescending() + "&format=json", function (data) {
+        const encodedSearchFilter = encodeURIComponent(self.searchQuery());
+        $.getJSON(self.targetPath() + (self.targetPath().indexOf('?') > 0 ? '&' : '?') + "pagesize=" + self.recordsPerPage() + "&pagenum=" + self.targetPageNum() + "&filter=" + encodedSearchFilter + "&sortby=" + self.sortBy() + "&descending=" + self.sortDescending() + "&format=json", function (data) {
           if (data.error){
             $(document).trigger("error", data.error);
             self.isLoading(false);
@@ -1349,7 +1330,6 @@ else:
             e.preventDefault();
             fileBrowserViewModel.targetPageNum(1);
             fileBrowserViewModel.targetPath("${url('filebrowser:filebrowser.views.view', path='')}?" + folderPath);
-            updateHash('');
             fileBrowserViewModel.retrieveData();
           }
           else {
@@ -1367,60 +1347,24 @@ else:
       }
 
       self.viewFile = function (file, e) {
-        const urlEncodedPercentage = '%25';
-        
         e.stopImmediatePropagation();
+        const decodedPath = file.path;
+        const pathPrefix = "${url('filebrowser:filebrowser.views.view', path='')}";
+
         if (file.type == "dir") {
           // Reset page number so that we don't hit a page that doesn't exist
-          self.targetPageNum(1);
+          self.targetPageNum(1);        
           self.enableFilterAfterSearch = false;
           self.searchQuery("");
-          self.targetPath(file.url);
-
-          const path = file.path;
-          const percentageEncodedPath = path.includes('%') && !path.includes(urlEncodedPercentage) ? 
-            path.replaceAll('%', urlEncodedPercentage) : path;
-        
-          updateHash(stripHashes(percentageEncodedPath));
+          huePubSub.publish('open.filebrowserlink', { pathPrefix, decodedPath, fileBrowserModel: self });
         } else {
-          
-          // Fix to encode potential hash characters in a file name when viewing a file
-          let url = file.name.indexOf('#') >= 0 && file.url.indexOf('#') >= 0 
-            ? file.url.replaceAll('#', encodeURIComponent('#')) : file.url;
-
-          %if is_embeddable:
-
-          // Fix. The '%' character needs to be encoded twice due to a bug in the page library
-          // that decodes the url twice
-          
-          if (file.path.includes('%') && !file.path.includes(urlEncodedPercentage)) {            
-            url = url.replaceAll(urlEncodedPercentage, encodeURIComponent(urlEncodedPercentage));
-          }
-
-          huePubSub.publish('open.link', url);
-          %else:
-          window.location.href = url;
-          %endif
-        }
-      };
-
-      self.editFile = function () {
-        huePubSub.publish('open.link', self.selectedFile().url.replace("${url('filebrowser:filebrowser.views.view', path='')}", "${url('filebrowser:filebrowser_views_edit', path='')}"));
+          huePubSub.publish('open.filebrowserlink', { pathPrefix, decodedPath });
+        }        
       };
 
       self.downloadFile = function () {
-        const baseUrl = "${url('filebrowser:filebrowser_views_download', path='')}";
-      // If the hash characters aren't encoded the page library will
-      // split the path on the first occurence and the remaining string will not
-      // be part of the path. Question marks must also be encoded or the string after the first
-      // question mark will be interpreted as the url querystring.
-      // Percentage character must be double encoded due to the page library that decodes the url twice
-        const doubleUrlEncodedPercentage = '%2525';
-        const partiallyEncodedFilePath = self.selectedFile().path.replaceAll('%', doubleUrlEncodedPercentage).replaceAll('#', encodeURIComponent('#'))
-          .replaceAll('?', encodeURIComponent('?'));;
-        const fullUrl = baseUrl+partiallyEncodedFilePath;
-        
-        huePubSub.publish('open.link', fullUrl);
+        huePubSub.publish('ignore.next.unload');
+        huePubSub.publish('open.filebrowserlink', { pathPrefix: '/filebrowser/download=', decodedPath: self.selectedFile().path });  
       };
 
       self.renameFile = function () {
@@ -1488,7 +1432,7 @@ else:
 
         if (!isMoveOnSelf){
           hiddenFields($("#moveForm"), "src_path", paths);
-          $("#moveForm").attr("action", "/filebrowser/move?next=${url('filebrowser:filebrowser.views.view', path='')}" + self.currentPath());
+          $("#moveForm").attr("action", "/filebrowser/move?next=${url('filebrowser:filebrowser.views.view', path='')}" + encodeURIComponent(self.currentPath()));
           $('#moveForm').ajaxForm({
             dataType:  'json',
             success: function() {
@@ -1698,7 +1642,7 @@ else:
 
         $("#deleteForm").attr("action", "/filebrowser/rmtree" + "?" +
           (self.skipTrash() ? "skip_trash=true&" : "") +
-          "next=${url('filebrowser:filebrowser.views.view', path='')}" + self.currentPath());
+          "next=${url('filebrowser:filebrowser.views.view', path='')}" + encodeURIComponent(self.currentPath()));
 
         $("#deleteModal").modal({
           keyboard:true,
@@ -2493,39 +2437,7 @@ else:
 
       $("*[rel='tooltip']").tooltip({ placement:"bottom" });
 
-      var hashchange = function () {
-        if (window.location.pathname.indexOf('/filebrowser') > -1) {
-          var targetPath = "";
-          var hash = decodeURI(window.location.hash.substring(1));
-          if (hash != null && hash != "" && hash.indexOf('/') > -1) {
-            targetPath = "${url('filebrowser:filebrowser.views.view', path='')}";
-            if (hash.indexOf("!!") != 0) {
-              targetPath += stripHashes(encodeURIComponent(hash));
-            }
-            else {
-              targetPath = fileBrowserViewModel.targetPath() + encodeURI(hash);
-            }
-            fileBrowserViewModel.targetPageNum(1)
-          }
-          if (window.location.href.indexOf("#") == -1) {
-            fileBrowserViewModel.targetPageNum(1);
-            targetPath = "${current_request_path | n,unicode }";
-          }
-          if (targetPath != "") {
-            fileBrowserViewModel.targetPath(targetPath);
-            fileBrowserViewModel.retrieveData();
-          }
-        }
-      }
-
-      huePubSub.subscribe('fb.update.hash', hashchange, 'filebrowser');
-
-      if (window.location.hash != null && window.location.hash.length > 1) {
-        hashchange();
-      }
-      else {
-        fileBrowserViewModel.retrieveData();
-      }
+      fileBrowserViewModel.retrieveData();
 
 
       $("#editBreadcrumb").click(function (e) {
@@ -2546,17 +2458,14 @@ else:
             $.jHueNotify.warn("${ _('Listing of buckets is not allowed. Redirecting to the home directory.') }");
             target_path = window.USER_HOME_DIR;
           } 
-          fileBrowserViewModel.targetPath("${url('filebrowser:filebrowser.views.view', path='')}" + target_path); 
+          fileBrowserViewModel.targetPath("${url('filebrowser:filebrowser.views.view', path='')}" + encodeURIComponent(target_path)); 
           fileBrowserViewModel.getStats(function (data) {
-            if (data.type != null && data.type == "file") {
-              %if is_embeddable:
-              huePubSub.publish('open.link', data.url);
-              %else:
-              huePubSub.publish('open.link', data.url);
-              %endif
+            const pathPrefix = "${url('filebrowser:filebrowser.views.view', path='')}";
+            if (data.type != null && data.type == "file") {              
+              huePubSub.publish('open.filebrowserlink', { pathPrefix, decodedPath: target_path});              
               return false;
             } else {
-              updateHash(target_path);
+              huePubSub.publish('open.filebrowserlink', { pathPrefix, decodedPath: target_path, fileBrowserModel: fileBrowserViewModel});
             }
             $("#jHueHdfsAutocomplete").hide();
           });
@@ -2583,7 +2492,6 @@ else:
         }
       });
 
-      $(window).bind("hashchange.fblist", hashchange);
 
       $(".actionbar").data("originalWidth", $(".actionbar").width());
 

+ 23 - 22
apps/filebrowser/src/filebrowser/views.py

@@ -142,12 +142,17 @@ def index(request):
 
   return view(request, path)
 
+def _decode_slashes(path):
+  # This is a fix for some installations where the path is still having the slash (/) encoded
+  # as %2F while the rest of the path is actually decoded. 
+  encoded_slash = '%2F'
+  if path.startswith(encoded_slash) or path.startswith('abfs:' + encoded_slash) or path.startswith('s3a:' + encoded_slash):
+    path = path.replace(encoded_slash, '/')
+
+  return path
 
 def _normalize_path(path):
-  # Prevent decoding of already decoded path, every path contains a '/' which would be encoded to %2F, hence
-  # if / is present it means that it's already been decoded.
-  if '/' not in path:
-    path = unquote_url(path)
+  path = _decode_slashes(path)
 
   # Check if protocol missing / and add it back (e.g. Kubernetes ingress can strip double slash)
   if path.startswith('abfs:/') and not path.startswith('abfs://'):
@@ -367,7 +372,7 @@ def save_file(request):
   """
   form = EditorForm(request.POST)
   is_valid = form.is_valid()
-  path = form.cleaned_data.get('path')
+  path = form['path'].value()
 
   path = _normalize_path(path)
 
@@ -406,7 +411,7 @@ def parse_breadcrumbs(path):
     if url and not url.endswith('/'):
       url += '/'
     url += part
-    breadcrumbs.append({'url': urllib_quote(url.encode('utf-8'), safe=SAFE_CHARACTERS_URI_COMPONENTS), 'label': part})
+    breadcrumbs.append({'url': url, 'label': part})
   return breadcrumbs
 
 
@@ -591,7 +596,7 @@ def listdir_paged(request, path):
       # The following should probably be deprecated
       'cwd_set': True,
       'file_filter': 'any',
-      'current_dir_path': urllib_quote(path.encode('utf-8'), safe=SAFE_CHARACTERS_URI),
+      'current_dir_path': path,
       '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 [],
@@ -1218,7 +1223,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_unquote(src_dir), urllib_unquote(dest_path))
+      dest_path = request.fs.join(src_dir, 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)
@@ -1233,14 +1238,13 @@ def set_replication(request):
 
   return generic_op(SetReplicationFactorForm, request, smart_set_replication, ["src_path", "replication_factor"], None)
 
-
 def mkdir(request):
   def smart_mkdir(path, name):
     # Make sure only one directory is specified at a time.
     # 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_unquote(path), urllib_unquote(name)))
+    request.fs.mkdir(request.fs.join(path, name))
 
   return generic_op(MkDirForm, request, smart_mkdir, ["path", "name"], "path")
 
@@ -1252,8 +1256,8 @@ def touch(request):
       raise PopupException(_("Could not name file \"%s\": Slashes are not allowed in filenames." % name))
     request.fs.create(
         request.fs.join(
-            urllib_unquote(path.encode('utf-8') if not isinstance(path, str) else path),
-            urllib_unquote(name.encode('utf-8') if not isinstance(name, str) else name)
+            (path.encode('utf-8') if not isinstance(path, str) else path),
+            (name.encode('utf-8') if not isinstance(name, str) else name)
         )
     )
 
@@ -1265,7 +1269,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_unquote(arg['path']), 'skip_trash' in request.GET)
+      request.fs.do_as_user(request.user, request.fs.rmtree, 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,
@@ -1281,8 +1285,8 @@ def move(request):
       if arg['src_path'] == arg['dest_path']:
         raise PopupException(_('Source path and destination path cannot be same'))
       request.fs.rename(
-          urllib_unquote(arg['src_path'].encode('utf-8') if not isinstance(arg['src_path'], str) else arg['src_path']),
-          urllib_unquote(arg['dest_path'].encode('utf-8') if not isinstance(arg['dest_path'], str) else arg['dest_path'])
+          arg['src_path'].encode('utf-8') if not isinstance(arg['src_path'], str) else arg['src_path'],
+          arg['dest_path'].encode('utf-8') if not isinstance(arg['dest_path'], str) else arg['dest_path']
       )
   return generic_op(RenameFormSet, request, bulk_move, ["src_path", "dest_path"], None,
                     data_extractor=formset_data_extractor(recurring, params),
@@ -1298,7 +1302,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(unquote_url(arg['src_path']), unquote_url(arg['dest_path']), recursive=True, owner=request.user)
+      request.fs.copy(arg['src_path'], 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,
@@ -1314,7 +1318,7 @@ def chmod(request):
   def bulk_chmod(*args, **kwargs):
     op = partial(request.fs.chmod, recursive=request.POST.get('recursive', False))
     for arg in args:
-      op(urllib_unquote(arg['path']), arg['mode'])
+      op(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",
@@ -1404,8 +1408,8 @@ def _upload_file(request):
 
   if form.is_valid():
     uploaded_file = request.FILES['hdfs_file']
-    dest = scheme_absolute_path(unquote_url(request.GET['dest']), unquote_url(request.GET['dest']))
-    filepath = request.fs.join(dest, unquote_url(uploaded_file.name))
+    dest = request.GET['dest']
+    filepath = request.fs.join(dest, uploaded_file.name)
 
     if request.fs.isdir(dest) and posixpath.sep in uploaded_file.name:
       raise PopupException(_('Sorry, no "%(sep)s" in the filename %(name)s.' % {'sep': posixpath.sep, 'name': uploaded_file.name}))
@@ -1469,9 +1473,6 @@ def compress_files_using_batch_job(request):
 
     if upload_path and file_names and archive_name:
       try:
-        upload_path = urllib_unquote(upload_path)
-        archive_name = urllib_unquote(archive_name)
-        file_names = [urllib_unquote(name) for name in file_names]
         response = compress_files_in_hdfs(request, file_names, upload_path, archive_name)
       except Exception as e:
         response['message'] = _('Exception occurred while compressing files: %s' % e)

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

@@ -1645,23 +1645,13 @@ class TestFileChooserRedirect(object):
 
 class TestNormalizePath(object):
 
-  def test_should_decode_encoded_path(self):
-    encoded_path = '%2Fsome%2Fpath%20with%20space%20in%20name'
-    expected_path = '/some/path with space in name'
+  def test_should_decode_encoded_slash_only(self):
+    encoded_path = '%2Fsome%2Fpath%20with%20space%20in name'
+    expected_path = '/some/path%20with%20space%20in name'
 
     normalized = _normalize_path(encoded_path)
     assert_equal(expected_path, normalized)
 
-  def test_decoding_should_only_happen_once(self):
-    encoded_path = '%2Fsome%2Ffolder%2Fwith%2520percent%20twenty%20in%20the%20name'
-    expected_decoded_path = '/some/folder/with%20percent twenty in the name'
-
-    normalized_once = _normalize_path(encoded_path)
-    assert_equal(expected_decoded_path, normalized_once)
-
-    normalized_twice = _normalize_path(normalized_once)
-    assert_equal(expected_decoded_path, normalized_twice)
-
   def test_abfs_correction(self):
     path = 'abfs:/some/path'
     expected_corrected_path = 'abfs://some/path'

+ 1 - 30
desktop/core/src/desktop/js/ext/fileuploader.custom.js

@@ -29,35 +29,6 @@ import $ from 'jquery';
 // Helper functions
 //
 
-const splitProtocolAndPathName = (fullUrl) => {
-    // The path name can contain one or more questionmarks (?) that are NOT specifying 
-    // the search/query start for these urls so we use a custom regex instead of URL() to parse.
-    const splitRegex = /^(\w*:)?(.*)$/i;
-    const [match, protocol, pathName] = splitRegex.exec(fullUrl);
-    return [protocol ?? '', pathName ]
-}
-
-const isUnicodeAlphaNumeric = /^[\p{L}\p{N}]*$/u;
-const encodeUploadPath = (char) => {            
-    const needsEncoding = !isUnicodeAlphaNumeric.test(char) && char !== '/' && char !== '?';
-    return needsEncoding ? encodeURIComponent(char) : char;
-}
-
-/**
- * Fixes encoding issues with the destination folder url.
- * Some special non-alphanumeric characters like plus (+) and ampersand (&) needs encoding
- * while the slash (/) which represents a directory and question mark (?) should not be encoded. 
- * Alphanumeric characters that are non ASCII (e.g. åäö) should not be encoded since that
- * will fail if the target is s3 or abfs.     
- * @param {string} destination The "destination url" used for the filesystem. Might not be a valid url.
- */
-const fixUploadDestination = (destination) => {
-    const decodedDestination = decodeURIComponent(destination);
-    const [protocol, path] = splitProtocolAndPathName(decodedDestination);
-    const updatedPath = [...path].map(encodeUploadPath).join('');
-    return `${protocol}${updatedPath}`;        
-}
-
 
 let qq = {};
 
@@ -1289,7 +1260,7 @@ qq.extend(qq.UploadHandlerXhr.prototype, {
         formData.append(params.fileFieldLabel, file, file.name.normalize('NFC'));
         formData.append('dest', params.dest);
 
-        const destination = fixUploadDestination(params.dest);
+        const destination = encodeURIComponent(params.dest);
         var action = this._options.action + "?dest=" + destination;
         xhr.open("POST", action, true);
         xhr.send(formData);

+ 11 - 7
desktop/core/src/desktop/js/jquery/plugins/jquery.filechooser.js

@@ -288,13 +288,17 @@ Plugin.prototype.navigateTo = function (path) {
   $(_parent.element)
     .find('.filechooser-tree')
     .html('<i style="font-size: 24px; color: #DDD" class="fa fa-spinner fa-spin"></i>');
-  let pageSize = '?pagesize=1000';
-  const index = path.indexOf('?');
-  if (index > -1) {
-    pageSize = path.substring(index) + pageSize.replace(/\?/, '&');
-    path = path.substring(0, index);
-  }
-  $.getJSON('/filebrowser/view=' + encodeURIComponent(path) + pageSize, data => {
+
+  // The default paths '/?default_to_home', '/?default_s3_home' etc already contains a query string
+  // so we append pageSizeParam as an additional param, not a new query string.
+  const isDefaultRedirectFlag = /^\/\?default_.*_(home|trash)$/.test(path);
+  const pageSizeParam = 'pagesize=1000';
+  const encodedPath = encodeURIComponent(path);
+  const pathAndQuery = isDefaultRedirectFlag
+    ? path + '&' + pageSizeParam
+    : encodedPath + '?' + pageSizeParam;
+
+  $.getJSON('/filebrowser/view=' + pathAndQuery, data => {
     $(_parent.element).find('.filechooser-tree').empty();
 
     path = data.current_dir_path || path; // use real path.

+ 39 - 43
desktop/core/src/desktop/js/onePageViewModel.js

@@ -317,9 +317,6 @@ class OnePageViewModel {
       huePubSub.publish('hue.datatable.search.hide');
       huePubSub.publish('hue.scrollleft.hide');
       huePubSub.publish('context.panel.visible', false);
-      if (app === 'filebrowser') {
-        $(window).unbind('hashchange.fblist');
-      }
       if (app.startsWith('oozie')) {
         huePubSub.clearAppSubscribers('oozie');
       }
@@ -618,13 +615,6 @@ class OnePageViewModel {
         }
       },
       { url: '/filebrowser/view=*', app: 'filebrowser' },
-      {
-        url: '/filebrowser/download=*',
-        app: function (ctx) {
-          const filePathParam = getModifiedCtxParamForFilePath(ctx, '/filebrowser/download=*');
-          location.href = window.HUE_BASE_URL + '/filebrowser/download=' + filePathParam;
-        }
-      },
       {
         url: '/filebrowser/*',
         app: function () {
@@ -832,42 +822,18 @@ class OnePageViewModel {
       });
     });
 
-    // The page library encodes the plus character (+) as a space in the returning
-    // params object. That causes the path used by the file viewer and downloader to break and this
-    // is a defensive fix to handle that while still having access to the other ctx params
-    // needed to "revert" the incorrect encoding.
-    const fixPlusCharTurnedIntoSpace = (ctx, pathWithoutParams) => {
-      const pathHasPlus = ctx.path.indexOf('+') >= 0;
-      const paramsHaveSpaces = ctx.params[0] && ctx.params[0].indexOf(' ') >= 0;
-
-      if (pathHasPlus && paramsHaveSpaces) {
-        // The original params with the plus character (+) can still be found in ctx.path
-        // and we use those to create our own encoded version that handles the plus character correctly.
-        const paramsOnly = ctx.path.replace(pathWithoutParams, '');
-        return decodeURIComponent(paramsOnly);
-      }
-
-      return ctx?.params[0];
-    };
-
-    // The page library decodes any hash characters (#) in the params object, so unless it is explicitly
-    // encoded again the file download links will break.
-    const fixHashCharInParam = (ctx, filePathParam) => {
-      const pathHasUTF8EncodedHash = ctx.path.indexOf('%23') >= 0;
-      const paramHasHash = ctx.params[0] && ctx.params[0].indexOf('#') >= 0;
-      const encodeHashChar = pathHasUTF8EncodedHash && paramHasHash;
-
-      return encodeHashChar
-        ? filePathParam.replaceAll('#', encodeURIComponent('#'))
-        : filePathParam;
-    };
-
     const getModifiedCtxParamForFilePath = (ctx, mappingUrl) => {
+      // FIX. The page library decodes the ctx.params differently than it decodes
+      // the ctx.path, e.g. '+' is turned into ' ' which we don't want. Therefore we use
+      // the path to extract the params manually and get the correct characters.
       const pathWithoutParams = mappingUrl.slice(0, -1);
-      let filePathParam = fixPlusCharTurnedIntoSpace(ctx, pathWithoutParams);
-      filePathParam = fixHashCharInParam(ctx, filePathParam);
+      const paramsOnly = ctx.path.replace(pathWithoutParams, '');
+      const filePathParam = decodeURIComponent(paramsOnly);
 
-      return filePathParam.replaceAll('?', encodeURIComponent('?'));
+      // Unfortunate hack needed since % has to be double encoded since the page library
+      // decodes it twice. This is temporarily double encoding only between the
+      // huePubSub.publish('open.link', fullUrl); and the onePgeViewModel.js.
+      return filePathParam.replaceAll('%25', '%');
     };
 
     pageMapping.forEach(mapping => {
@@ -922,6 +888,36 @@ class OnePageViewModel {
         console.warn('Received an open.link without href.');
       }
     });
+
+    huePubSub.subscribe('open.filebrowserlink', ({ pathPrefix, decodedPath, fileBrowserModel }) => {
+      if (pathPrefix.includes('download=')) {
+        // The download view on the backend requires the slashes not to
+        // be encoded in order for the file to be correctly named.
+        const encodedPath = encodeURIComponent(decodedPath).replaceAll('%2F', '/');
+        window.location = pathPrefix + encodedPath;
+        return;
+      }
+
+      const appPrefix = '/hue';
+      const urlEncodedPercentage = '%25';
+      // Fix. The '%' character needs to be encoded twice due to a bug in the page library
+      // that decodes the url twice. Even when we don't directly call pag() we still need this
+      // fix since the user can reload the page which will trigger a call to page().
+      const pageFixedEncodedPath = encodeURIComponent(
+        decodedPath.replaceAll('%', urlEncodedPercentage)
+      );
+      const href = window.HUE_BASE_URL + appPrefix + pathPrefix + pageFixedEncodedPath;
+
+      // We don't want reload the entire filebrowser when navigating between folders
+      // and already on the listdir_components page.
+      if (fileBrowserModel) {
+        fileBrowserModel.targetPath(pathPrefix + encodeURIComponent(decodedPath));
+        window.history.pushState(null, '', href);
+        fileBrowserModel.retrieveData();
+      } else {
+        page(href);
+      }
+    });
   }
 }
 

+ 11 - 2
desktop/core/src/desktop/lib/fs/__init__.py

@@ -32,10 +32,19 @@ else:
 
 def splitpath(path):
   split = lib_urlparse(path)
+  path_parsed_as_query = ''
+  
+  # Make sure the splitpath can handle a path that contains "?" since
+  # that is the case for the file browser paths.
+  if '?' in path:
+    path_parsed_as_query = '?'
+    if split.query:
+      path_parsed_as_query = '?' + split.query
+
   if split.scheme and split.netloc:
-    parts = [split.scheme + '://', split.netloc] + split.path.split('/')
+    parts = [split.scheme + '://', split.netloc] + (split.path + path_parsed_as_query).split('/')
   elif split.scheme and split.path:
-    parts = [split.scheme + ':/'] + split.path.split('/')
+    parts = [split.scheme + ':/'] + (split.path + path_parsed_as_query).split('/')
   elif split.scheme:
     parts = [split.scheme + ("://" if path.find("://") >= 0 else ":/")]
   else:

+ 11 - 1
desktop/core/src/desktop/lib/fs/fs_test.py

@@ -1,3 +1,4 @@
+# -*- coding: utf-8 -*-
 # Licensed to Cloudera, Inc. under one
 # or more contributor license agreements.  See the NOTICE file
 # distributed with this work for additional information
@@ -29,8 +30,17 @@ def test_splitpath():
   eq_(s('s3a://bucket/key'), ['s3a://', 'bucket', 'key'])
   eq_(s('s3a://bucket/key/'), ['s3a://', 'bucket', 'key'])
   eq_(s('s3a://bucket/bar/foo'), ['s3a://', 'bucket', 'bar', 'foo'])
+  eq_(s('s3a://bucket/bar/foo?I-have-a-questionmark-in-the-folder-name/me?to'), \
+    ['s3a://', 'bucket', 'bar', 'foo?I-have-a-questionmark-in-the-folder-name', 'me?to'])
+  eq_(s(u"s3a://bucket/all%20% ~@$&()*!+=;.?'Tжейкоб-åäö-你好"), \
+    ['s3a://', 'bucket', u"all%20% ~@$&()*!+=;.?'Tжейкоб-åäö-你好"])
+  
 
   eq_(s('/'), ['/'])
   eq_(s('/dir'), ['/', 'dir'])
   eq_(s('/dir/file'), ['/', 'dir', 'file'])
-  eq_(s('/dir/file/'), ['/', 'dir', 'file'])
+  eq_(s('/dir/file/'), ['/', 'dir', 'file'])
+  eq_(s('/dir/file/foo?I-have-a-questionmark-in-the-folder-name/me?to'), \
+    ['/', 'dir', 'file', 'foo?I-have-a-questionmark-in-the-folder-name', 'me?to'])
+  eq_(s(u"/dir/all%20% ~@$&()*!+=;.?'Tжейкоб-åäö-你好"), \
+    ['/', 'dir', u"all%20% ~@$&()*!+=;.?'Tжейкоб-åäö-你好"])

+ 5 - 3
desktop/core/src/desktop/lib/tasks/compress_files/compress_utils.py

@@ -39,7 +39,8 @@ def compress_files_in_hdfs(request, file_names, upload_path, archive_name):
 
   _upload_compress_files_script_to_hdfs(request.fs)
 
-  files = [{"value": upload_path + '/' + urllib_quote(file_name.encode('utf-8'), SAFE_CHARACTERS_URI)} for file_name in file_names]
+  files = [{"value": urllib_quote(upload_path.encode('utf-8'), SAFE_CHARACTERS_URI) + '/' + \
+    urllib_quote(file_name.encode('utf-8'), SAFE_CHARACTERS_URI)} for file_name in file_names]
   files.append({'value': '/user/' + DEFAULT_USER.get() + '/common/compress_files_in_hdfs.sh'})
   start_time = json.loads(request.POST.get('start_time', '-1'))
 
@@ -66,6 +67,7 @@ def _upload_compress_files_script_to_hdfs(fs):
     fs.do_as_user(DEFAULT_USER.get(), fs.chmod, '/user/' + DEFAULT_USER.get() + '/common/', 0o755)
 
   if not fs.do_as_user(DEFAULT_USER.get(), fs.exists, '/user/' + DEFAULT_USER.get() + '/common/compress_files_in_hdfs.sh'):
-    fs.do_as_user(DEFAULT_USER.get(), fs.copyFromLocal, get_desktop_root() + '/core/src/desktop/lib/tasks/compress_files/compress_in_hdfs.sh',
-                          '/user/' + DEFAULT_USER.get() + '/common/compress_files_in_hdfs.sh')
+    fs.do_as_user(DEFAULT_USER.get(), fs.copyFromLocal, get_desktop_root() + \
+      '/core/src/desktop/lib/tasks/compress_files/compress_in_hdfs.sh',\
+        '/user/' + DEFAULT_USER.get() + '/common/compress_files_in_hdfs.sh')
     fs.do_as_user(DEFAULT_USER.get(), fs.chmod, '/user/' + DEFAULT_USER.get() + '/common/', 0o755)

+ 6 - 5
desktop/libs/indexer/src/indexer/api3.py

@@ -124,7 +124,7 @@ def _convert_format(format_dict, inverse=False):
 def guess_format(request):
   file_format = json.loads(request.POST.get('fileFormat', '{}'))
   file_type = file_format['file_type']
-  path = urllib_unquote(file_format["path"])
+  path = file_format["path"]
   
   if sys.version_info[0] < 3 and (file_type == 'excel' or path[-3:] == 'xls' or path[-4:] == 'xlsx'):
     return JsonResponse({'status': -1, 'message': 'Python2 based Hue does not support Excel file importer'})
@@ -295,7 +295,8 @@ def guess_field_types(request):
 
   elif file_format['inputFormat'] == 'file':
     indexer = MorphlineIndexer(request.user, request.fs)
-    path = urllib_unquote(file_format["path"])
+    path = file_format["path"]
+
     if path[-3:] == 'xls' or path[-4:] == 'xlsx':
       path = excel_to_csv_file_name_change(path)
     stream = request.fs.open(path)
@@ -458,7 +459,7 @@ def importer_submit(request):
   file_encoding = None
   if source['inputFormat'] == 'file':
     if source['path']:
-      path = urllib_unquote(source['path'])
+      path = source['path']
       if path[-3:] == 'xls' or path[-4:] == 'xlsx':
         path = excel_to_csv_file_name_change(path)
       source['path'] = request.fs.netnormpath(path)
@@ -612,7 +613,7 @@ def _small_indexing(user, fs, client, source, destination, index_name):
   errors = []
 
   if source['inputFormat'] not in ('manual', 'table', 'query_handle'):
-    path = urllib_unquote(source["path"])
+    path = source["path"]
     stats = fs.stats(path)
     if stats.size > MAX_UPLOAD_SIZE:
       raise PopupException(_('File size is too large to handle!'))
@@ -624,7 +625,7 @@ def _small_indexing(user, fs, client, source, destination, index_name):
 
   if source['inputFormat'] == 'file':
     kwargs['separator'] = source['format']['fieldSeparator']
-    path = urllib_unquote(source["path"])
+    path = source["path"]
     data = fs.read(path, 0, MAX_UPLOAD_SIZE)
 
   if client.is_solr_six_or_more():

+ 1 - 1
desktop/libs/indexer/src/indexer/indexers/sql.py

@@ -82,7 +82,7 @@ class SQLIndexer(object):
     kudu_partition_columns = destination['kuduPartitionColumns']
     comment = destination['description']
 
-    source_path = urllib_unquote(source['path'])
+    source_path = source['path']
     load_data = destination['importData']
     isIceberg = destination['isIceberg']