backup: Allow SSH directory paths with : in them

- From SSH manual, a path may contain : if it is presented as absolute path.
  Update regular expression for splitting accordingly.

- Allow Null paths similar to SSH.

- Perform a full regular expression match when splitting path.

- Simplify regular expression.

- Update tests.

Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: Joseph Nuthalapati <njoseph@thoughtworks.com>

plinth/modules/backups/__init__.py

@@ -165,4 +165,4 @@ def split_path(path):
     Network interface information is kept in the hostname if provided.
     e.g. fe80::2078:6c26:498a:1fa5%wlp1s0
     """
-    return re.findall(r'(.*)[@].*?(.*)[:](.*)', path)[0]
+    return re.findall(r'^(.*)@([^/]*):(.*)$', path)[0]

plinth/modules/backups/forms.py

@@ -120,12 +120,9 @@ def repository_validator(path):
             raise ValidationError(_(f'Invalid hostname: {hostname}'))
 
     # Validate directory path
-    if not re.match(r'[^\0]+', dir_path):
+    if not re.match(r'[^\0]*', dir_path):
         raise ValidationError(_(f'Invalid directory path: {dir_path}'))
 
-    # Just for tests. A validator doesn't have to return anything.
-    return True
-
 
 class AddRepositoryForm(forms.Form):
     repository = forms.CharField(

plinth/modules/backups/tests/test_validators.py

@@ -21,12 +21,15 @@ Tests for form field validators in backups.
 import pytest
 from django.core.exceptions import ValidationError
 
+from .. import split_path
 from ..forms import repository_validator
 
 
-def validate_repository(valid_list, invalid_list, path_string):
+def _validate_repository(valid_list, invalid_list, path_string):
-    assert all(
+    """Assert that repository strings in given list (in)validate properly."""
-        repository_validator(path_string.format(item)) for item in valid_list)
+    for item in valid_list:
+        repository_validator(path_string.format(item))
+
     for item in invalid_list:
         path = path_string.format(item)
         with pytest.raises(ValidationError):
@@ -34,34 +37,47 @@ def validate_repository(valid_list, invalid_list, path_string):
 
 
 def test_repository_paths_validation():
+    """Test that repository strings are validated properly."""
     valid_paths = ['sshuser@10.0.2.2:~/backups']
     invalid_paths = [
         'mary had a little lamb', 'someone@example.com', 'www.example.com',
         'sshuser@hostname'
     ]
     path_string = '{}'
-    validate_repository(valid_paths, invalid_paths, path_string)
+    _validate_repository(valid_paths, invalid_paths, path_string)
 
 
 def test_repository_username_validation():
+    """Test that usernames in repository string are validated properly."""
     valid_usernames = ['sshuser', 'cypher_punk-2077', '_user', '_-_']
     invalid_usernames = ['1two', 'somebody else']
     path_string = '{}@example.org:~/backups'
-    validate_repository(valid_usernames, invalid_usernames, path_string)
+    _validate_repository(valid_usernames, invalid_usernames, path_string)
 
 
 def test_repository_hostname_validation():
+    """Test that hostnames in repository string are validated properly."""
     valid_hostnames = [
         '192.168.0.1', 'fe80::2078:6c26:498a:1fa5%wlps0', 'freedombox.org',
         '1.percent.org', 'freedombox', '::1'
     ]
     invalid_hostnames = ['192.fe80::2089:1fa5']
     path_string = 'user@{}:~/backups'
-    validate_repository(valid_hostnames, invalid_hostnames, path_string)
+    _validate_repository(valid_hostnames, invalid_hostnames, path_string)
 
 
 def test_repository_dir_path_validation():
-    valid_dir_paths = ['~/backups', '/home/user/backup-folder_1/']
+    """Test that paths in repository string are validated properly."""
-    invalid_dir_paths = ['']
+    valid_dir_paths = [
+        '~/backups', '/home/user/backup-folder_1/', '', '/foo:bar/'
+    ]
+    invalid_dir_paths = []
     path_string = 'user@localhost:{}'
-    validate_repository(valid_dir_paths, invalid_dir_paths, path_string)
+    _validate_repository(valid_dir_paths, invalid_dir_paths, path_string)
+
+
+def test_respository_with_colon_path():
+    """Test that a colon is possible in directory path."""
+    _, hostname, path = split_path('user@fe80::2078:6c26:498a:1fa5:/foo:bar')
+    assert hostname == 'fe80::2078:6c26:498a:1fa5'
+    assert path == '/foo:bar'