From 9c8674baa3ea09e697776057f1ce26d56493da13 Mon Sep 17 00:00:00 2001 From: Sunil Mohan Adapa Date: Mon, 24 Jun 2019 16:35:55 -0700 Subject: [PATCH] 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 Reviewed-by: Joseph Nuthalapati --- plinth/modules/backups/__init__.py | 2 +- plinth/modules/backups/forms.py | 5 +-- .../modules/backups/tests/test_validators.py | 34 ++++++++++++++----- 3 files changed, 27 insertions(+), 14 deletions(-) diff --git a/plinth/modules/backups/__init__.py b/plinth/modules/backups/__init__.py index 34a0d6e65..3c8d48695 100644 --- a/plinth/modules/backups/__init__.py +++ b/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] diff --git a/plinth/modules/backups/forms.py b/plinth/modules/backups/forms.py index 120a62c34..2a5bba9e5 100644 --- a/plinth/modules/backups/forms.py +++ b/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( diff --git a/plinth/modules/backups/tests/test_validators.py b/plinth/modules/backups/tests/test_validators.py index d540832fc..e23bb8bf0 100644 --- a/plinth/modules/backups/tests/test_validators.py +++ b/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): - assert all( - repository_validator(path_string.format(item)) for item in valid_list) +def _validate_repository(valid_list, invalid_list, path_string): + """Assert that repository strings in given list (in)validate properly.""" + 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/'] - invalid_dir_paths = [''] + """Test that paths in repository string are validated properly.""" + 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'