Browse Source

[fb] Prevent decoding of already decoded path in the File Browser

Currently decoding happens at least twice in views.py when requesting to view a folder, this causes an issue when files are stored with actual encoded values in the name.

An example is if a folder exists in hdfs with the name 'some%20folder', the UI would encode it and request 'some%2520folder' (% becomes %25), which if decoded once would be correct, 'some%20folder', but if it's done twice the resulting name would be 'some folder' with a space instead of %20 which is incorrect.

We can safely assume that each path contains a '/' char, so we can use that to check if a path has already been decoded. An encoded path would never contain a '/' as it would be %2F.
Johan Åhlén 3 years ago
parent
commit
57eabed7fc

+ 4 - 3
apps/filebrowser/src/filebrowser/views.py

@@ -144,9 +144,10 @@ def index(request):
 
 
 def _normalize_path(path):
-  decoded_path = unquote_url(path)
-  if path != decoded_path:
-    path = decoded_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)
 
   # 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://'):

+ 52 - 1
apps/filebrowser/src/filebrowser/views_test.py

@@ -63,7 +63,7 @@ from useradmin.models import User, Group
 from filebrowser.conf import ENABLE_EXTRACT_UPLOADED_ARCHIVE, MAX_SNAPPY_DECOMPRESSION_SIZE,\
   REMOTE_STORAGE_HOME
 from filebrowser.lib.rwx import expand_mode
-from filebrowser.views import snappy_installed
+from filebrowser.views import snappy_installed, _normalize_path
 
 if sys.version_info[0] > 2:
   from urllib.parse import unquote as urllib_unquote, urlparse
@@ -1642,3 +1642,54 @@ class TestFileChooserRedirect(object):
           response = self.client.get('/filebrowser/view=')
 
           _normalize_path.assert_called_with('/')
+
+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'
+
+    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'
+
+    normalized_once = _normalize_path(path)
+    assert_equal(expected_corrected_path, normalized_once)
+
+    normalized_twice = _normalize_path(normalized_once)
+    assert_equal(expected_corrected_path, normalized_twice)
+
+  def test_abfs_correction_already_correct(self):
+    path = 'abfs://some/path'
+
+    normalized = _normalize_path(path)
+    assert_equal(path, normalized)
+
+  def test_s3a_correction(self):
+    path = 's3a:%2Fsome%2Fpath'
+    expected_corrected_path = 's3a://some/path'
+
+    normalized_once = _normalize_path(path)
+    assert_equal(expected_corrected_path, normalized_once)
+
+    normalized_twice = _normalize_path(normalized_once)
+    assert_equal(expected_corrected_path, normalized_twice)
+
+  def test_s3a_correction_already_correct(self):
+    path = 's3a://some/path'
+
+    normalized = _normalize_path(path)
+    assert_equal(path, normalized)