From 92e8d63a61d60c8a60de6106b8773ffe1c0eaef1 Mon Sep 17 00:00:00 2001 From: Madhuri Date: Tue, 25 Aug 2015 13:16:14 -0700 Subject: [PATCH 1/5] Add validation to check share name length. #828 --- src/rockstor/storageadmin/views/share.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/rockstor/storageadmin/views/share.py b/src/rockstor/storageadmin/views/share.py index eb3683991..5370afb34 100644 --- a/src/rockstor/storageadmin/views/share.py +++ b/src/rockstor/storageadmin/views/share.py @@ -139,6 +139,10 @@ def post(self, request): 'hyphen(-), underscore(_) or a period(.).') handle_exception(Exception(e_msg), request) + if (len(sname) > 255): + e_msg = ('Share name must be less than 255 characters') + handle_exception(Exception(e_msg), request) + if (Share.objects.filter(name=sname).exists()): e_msg = ('Share(%s) already exists. Choose a different name' % sname) handle_exception(Exception(e_msg), request) From a9f38ce17d862afb588ba93ba50a90a389695c55 Mon Sep 17 00:00:00 2001 From: Madhuri Date: Wed, 26 Aug 2015 06:35:52 -0700 Subject: [PATCH 2/5] Improve test coverage for shares. #828 --- .../storageadmin/tests/test_shares.py | 157 ++++++++++++------ 1 file changed, 108 insertions(+), 49 deletions(-) diff --git a/src/rockstor/storageadmin/tests/test_shares.py b/src/rockstor/storageadmin/tests/test_shares.py index 48012a0b0..2d6e94498 100644 --- a/src/rockstor/storageadmin/tests/test_shares.py +++ b/src/rockstor/storageadmin/tests/test_shares.py @@ -55,13 +55,18 @@ def setUpClass(cls): cls.patch_qgroup_id = patch('storageadmin.views.share.qgroup_id') cls.mock_qgroup_id = cls.patch_qgroup_id.start() - cls.mock_qgroup_id.return_value = True + cls.mock_qgroup_id.return_value = '0f123f' # put mocks cls.patch_share_usage = patch('storageadmin.views.share.share_usage') cls.mock_share_usage = cls.patch_share_usage.start() - cls.mock_share_usage.return_value = 500 - + cls.mock_share_usage.return_value = (500, 500) + + # delete mocks + cls.patch_remove_share = patch('storageadmin.views.share.remove_share') + cls.mock_remove_share = cls.patch_remove_share.start() + cls.mock_remove_share.return_value = True + @classmethod def tearDownClass(cls): super(ShareTests, cls).tearDownClass() @@ -70,11 +75,22 @@ def test_get(self): """ Test GET request 1. Get base URL - 2. Get nonexistant share - 3. Get w/ sort parameters + 2. Get existing share + 3. Get nonexistant share + 4. Get w/ sort parameters """ self.get_base(self.BASE_URL) + # get share poolshare1( alreday existing share in fixture fix1) + response = self.client.get('%s/poolshare1' % self.BASE_URL) + self.assertEqual(response.status_code, status.HTTP_200_OK, msg=response) + + # get share that does not exist + e_msg = '' + response = self.client.get('%s/invalid_share' % self.BASE_URL) + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND, msg=response) + + # Get w/ sort parameters response1 = self.client.get('%s?sortby=usage&reverse=yes' % self.BASE_URL) self.assertEqual(response1.status_code, status.HTTP_200_OK, msg=response1.data) @@ -107,8 +123,7 @@ def test_name_regex(self): e_msg = ('Share name must start with a alphanumeric(a-z0-9) character ' 'and can be followed by any of the following characters: ' 'letter(a-z), digits(0-9), hyphen(-), underscore(_) or a period(.).') - invalid_names = ('Share $', '-share', '.share', '', ' ', 'Sh' + - 'a' * 254 + 're',) + invalid_names = ('Share $', '-share', '.share', '', ' ',) for sname in invalid_names: data['sname'] = sname response = self.client.post(self.BASE_URL, data=data) @@ -116,6 +131,15 @@ def test_name_regex(self): status.HTTP_500_INTERNAL_SERVER_ERROR, msg=response.data) self.assertEqual(response.data['detail'], e_msg) + # Share name with more than 255 characters + e_msg = ('Share name must be less than 255 characters') + + data['sname']= 'Sh' + 'a' * 254 + 're' + response = self.client.post(self.BASE_URL, data=data) + self.assertEqual(response.status_code, + status.HTTP_500_INTERNAL_SERVER_ERROR, msg=response.data) + self.assertEqual(response.data['detail'], e_msg) + def test_create(self): """ Test POST request to create shares @@ -219,29 +243,37 @@ def test_resize(self): """ # create new share - data = {'sname': 'rootshare', 'pool': 'rockstor_rockstor', 'size': 100} + data = {'sname': 'share1', 'pool': 'rockstor_rockstor', 'size': 1000} response = self.client.post(self.BASE_URL, data=data) self.assertEqual(response.status_code, status.HTTP_200_OK, msg=response.data) - self.assertEqual(response.data['name'], 'rootshare') - - # resize a share - # data2 = {'sname': 'rootshare', 'pool': 'rockstor_rockstor', 'size': 1000} - data2 = {'size': 1000} - response2 = self.client.put('%s/rootshare' % self.BASE_URL, data=data2) - self.assertEqual(response2.status_code, status.HTTP_200_OK, msg=response2.data) - self.assertEqual(response2.data['size'], 1000) - - # resize a share that doesn't exist - data3 = {'sname': 'invalid', 'size': 1500} - response3 = self.client.put('%s/invalid' % self.BASE_URL, data=data3) + self.assertEqual(response.data['name'], 'share1') + self.assertEqual(response.data['size'], 1000) + + # resize share + data3 = {'size': 2000,} + response3 = self.client.put('%s/share1' % self.BASE_URL, data=data3) + self.assertEqual(response3.status_code, status.HTTP_200_OK, msg=response3.data) + self.assertEqual(response3.data['size'], 2000) + + # resize a 'root' share + data3 = {'size': 1500} + response3 = self.client.put('%s/root' % self.BASE_URL, data=data3) self.assertEqual(response3.status_code, status.HTTP_500_INTERNAL_SERVER_ERROR, msg=response3.data) - e_msg = ('Share(invalid) does not exist.') + e_msg = ('Operation not permitted on this Share(root) because it is a special system Share') self.assertEqual(response3.data['detail'], e_msg) - - # resize to below current share usage value + + # resize a 'home' share + data3 = {'size': 1500} + response3 = self.client.put('%s/home' % self.BASE_URL, data=data3) + self.assertEqual(response3.status_code, + status.HTTP_500_INTERNAL_SERVER_ERROR, msg=response3.data) + e_msg = ('Operation not permitted on this Share(home) because it is a special system Share') + self.assertEqual(response3.data['detail'], e_msg) + + # resize to below current share usage value data3 = {'size': 400} - response3 = self.client.put('%s/rootshare' % self.BASE_URL, data=data3) + response3 = self.client.put('%s/share1' % self.BASE_URL, data=data3) self.assertEqual(response3.status_code, status.HTTP_500_INTERNAL_SERVER_ERROR, msg=response3.data) e_msg = ('Unable to resize because requested new size(400KB) is less ' @@ -251,11 +283,21 @@ def test_resize(self): # resize below 100KB self.mock_share_usage.return_value = 50 data3 = {'size': 99} - response3 = self.client.put('%s/rootshare' % self.BASE_URL, data=data3) + response3 = self.client.put('%s/share1' % self.BASE_URL, data=data3) self.assertEqual(response3.status_code, status.HTTP_500_INTERNAL_SERVER_ERROR, msg=response3.data) e_msg = ('Share size should atleast be 100KB. Given size is 99KB') self.assertEqual(response3.data['detail'], e_msg) + + # resize a share that doesn't exist + data3 = {'sname': 'invalid', 'size': 1500} + response3 = self.client.put('%s/invalid' % self.BASE_URL, data=data3) + self.assertEqual(response3.status_code, + status.HTTP_500_INTERNAL_SERVER_ERROR, msg=response3.data) + e_msg = ('Share(invalid) does not exist') + self.assertEqual(response3.data['detail'], e_msg) + + def test_compression(self): """ @@ -331,9 +373,9 @@ def test_compression(self): @mock.patch('storageadmin.views.share.SambaShare') @mock.patch('storageadmin.views.share.NFSExport') @mock.patch('storageadmin.views.share.Snapshot') - def test_delete(self, mock_snapshot, mock_nfs, mock_samba, mock_sftp, mock_remove_share): + def test_delete_set1(self, mock_snapshot, mock_nfs, mock_samba, mock_sftp, mock_remove_share): """ - Test DELETE request on share + Test DELETE request on share 1. Create valid share 2. Delete share with replication related snapshots 3. Delete share with NFS export @@ -341,8 +383,7 @@ def test_delete(self, mock_snapshot, mock_nfs, mock_samba, mock_sftp, mock_remov 5. Delete share with snapshots 6. Delete share with SFTP export 7. Delete share with remove_share failure (share still mounted) - 8. Delete share - 9. Delete nonexistent share + 8. Delete nonexistent share """ # create share data = {'sname': 'rootshare', 'pool': 'rockstor_rockstor', 'size': 100} @@ -400,28 +441,46 @@ def test_delete(self, mock_snapshot, mock_nfs, mock_samba, mock_sftp, mock_remov status.HTTP_500_INTERNAL_SERVER_ERROR, msg=response5.data) self.assertEqual(response5.data['detail'], e_msg) mock_snapshot.objects.filter(share=share, snap_type='admin').exists.return_value = False - - # Delete share with remove_share failure (share still mounted) - mock_remove_share.return_value = 'foo' - mock_remove_share.side_effect = Exception - e_msg = ('Share(rootshare) is still mounted and cannot be deleted. ' - 'Trying again usually succeeds. But if it does not, you can ' - 'manually unmount it with command: /usr/bin/umount /mnt2/rootshare') - response7 = self.client.delete('%s/rootshare' % self.BASE_URL) - self.assertEqual(response7.status_code, - status.HTTP_500_INTERNAL_SERVER_ERROR, msg=response7.data) - self.assertEqual(response7.data['detail'], e_msg) - mock_remove_share.side_effect = None - - # delete share - response8 = self.client.delete('%s/rootshare' % self.BASE_URL) - self.assertEqual(response8.status_code, status.HTTP_200_OK, msg=response8.data) - assert mock_remove_share.called - self.assertEqual(response8.data, None) - + # delete a share that doesn't exist - e_msg = ('Share(invalid) does not exist.') + e_msg = ('Share(invalid) does not exist') response9 = self.client.delete('%s/invalid' % self.BASE_URL) self.assertEqual(response9.status_code, status.HTTP_500_INTERNAL_SERVER_ERROR, msg=response9.data) self.assertEqual(response9.data['detail'], e_msg) + + @mock.patch('storageadmin.views.share.Service') + def test_delete2(self, mock_service): + # happy path + # create share + data = {'sname': 'rootshare', 'pool': 'pool1', 'size': 100} + response = self.client.post(self.BASE_URL, data=data) + self.assertEqual(response.status_code, status.HTTP_200_OK, msg=response.data) + self.assertEqual(response.data['name'], 'rootshare') + + # Delete share + + mock_service.objects.get.side_effect= None + response7 = self.client.delete('%s/rootshare' % self.BASE_URL) + self.assertEqual(response7.status_code, + status.HTTP_200_OK, msg=response7.data) + + @mock.patch('storageadmin.views.share.Service') + def test_delete3(self, mock_service): + # unhappy path + # create share + data = {'sname': 'rootshare', 'pool': 'pool1', 'size': 100} + response = self.client.post(self.BASE_URL, data=data) + self.assertEqual(response.status_code, status.HTTP_200_OK, msg=response.data) + self.assertEqual(response.data['name'], 'rootshare') + + # Delete share + e_msg = ('Failed to delete the Share(rootshare). Error from the OS: ') + mock_service.objects.get.side_effect= None + self.mock_remove_share.side_effect = Exception + response7 = self.client.delete('%s/rootshare' % self.BASE_URL) + self.assertEqual(response7.status_code, + status.HTTP_500_INTERNAL_SERVER_ERROR, msg=response7.data) + self.assertEqual(response7.data['detail'], e_msg) + + \ No newline at end of file From 43bc8a40308831dbb36bdde83c0da4eb8f1daccc Mon Sep 17 00:00:00 2001 From: Madhuri Date: Wed, 26 Aug 2015 07:37:05 -0700 Subject: [PATCH 3/5] improve code coverage. #828 --- .../storageadmin/tests/test_shares.py | 21 +++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/src/rockstor/storageadmin/tests/test_shares.py b/src/rockstor/storageadmin/tests/test_shares.py index 2d6e94498..0ae9a856f 100644 --- a/src/rockstor/storageadmin/tests/test_shares.py +++ b/src/rockstor/storageadmin/tests/test_shares.py @@ -482,5 +482,22 @@ def test_delete3(self, mock_service): self.assertEqual(response7.status_code, status.HTTP_500_INTERNAL_SERVER_ERROR, msg=response7.data) self.assertEqual(response7.data['detail'], e_msg) - - \ No newline at end of file + self.mock_remove_share.side_effect = None + + # Delete share that is in use by rock-on service + class MockService(object): + def __init__(self, **kwargs): + self.config = {'root_share': 'rootshare',} + e_msg = ('Share(rootshare) cannot be deleted because it is in use ' + 'by Rock-on service. If you really need to delete ' + 'it, (1)turn the service off, (2)change its ' + 'configuration to use a different Share and then ' + '(3)try deleting this Share(%s) again.') + mock_service.objects.get.side_effect = MockService + response7 = self.client.delete('%s/rootshare' % self.BASE_URL) + self.assertEqual(response7.status_code, + status.HTTP_500_INTERNAL_SERVER_ERROR, msg=response7.data) + self.assertEqual(response7.data['detail'], e_msg) + + + \ No newline at end of file From 632e66260af06e10f34f3ae5d3a9c869d4ec1ccc Mon Sep 17 00:00:00 2001 From: Suman Chakravartula Date: Thu, 27 Aug 2015 11:08:20 -0700 Subject: [PATCH 4/5] share name max length check should be 4K not 256 characters. #828 --- .../storageadmin/tests/test_shares.py | 59 +++++++++---------- src/rockstor/storageadmin/views/share.py | 6 +- 2 files changed, 31 insertions(+), 34 deletions(-) diff --git a/src/rockstor/storageadmin/tests/test_shares.py b/src/rockstor/storageadmin/tests/test_shares.py index 0ae9a856f..ece0b9f00 100644 --- a/src/rockstor/storageadmin/tests/test_shares.py +++ b/src/rockstor/storageadmin/tests/test_shares.py @@ -36,7 +36,7 @@ def setUpClass(cls): cls.patch_add_share = patch('storageadmin.views.share.add_share') cls.mock_add_share = cls.patch_add_share.start() cls.mock_add_share.return_value = True - + cls.patch_update_quota = patch('storageadmin.views.share.update_quota') cls.mock_update_quota = cls.patch_update_quota.start() cls.mock_update_quota.return_value = True @@ -61,12 +61,12 @@ def setUpClass(cls): cls.patch_share_usage = patch('storageadmin.views.share.share_usage') cls.mock_share_usage = cls.patch_share_usage.start() cls.mock_share_usage.return_value = (500, 500) - + # delete mocks cls.patch_remove_share = patch('storageadmin.views.share.remove_share') cls.mock_remove_share = cls.patch_remove_share.start() cls.mock_remove_share.return_value = True - + @classmethod def tearDownClass(cls): super(ShareTests, cls).tearDownClass() @@ -75,7 +75,7 @@ def test_get(self): """ Test GET request 1. Get base URL - 2. Get existing share + 2. Get existing share 3. Get nonexistant share 4. Get w/ sort parameters """ @@ -84,7 +84,7 @@ def test_get(self): # get share poolshare1( alreday existing share in fixture fix1) response = self.client.get('%s/poolshare1' % self.BASE_URL) self.assertEqual(response.status_code, status.HTTP_200_OK, msg=response) - + # get share that does not exist e_msg = '' response = self.client.get('%s/invalid_share' % self.BASE_URL) @@ -105,13 +105,13 @@ def test_name_regex(self): 1. Test a few valid regexes (eg: share1, Myshare, 123, etc..) 2. Test a few invalid regexes (eg: -share1, .share etc..) 3. Empty string for share name - 4. max length(255 character) for share name + 4. max length(4096 characters) for share name 5. max length + 1 for share name """ # valid share names data = {'pool': 'rockstor_rockstor', 'size': 1000} valid_names = ('123share', 'SHARE_TEST', 'Zzzz...', '1234', 'myshare', - 'Sha' + 'r' * 250 + 'e',) + 'Sha' + 'r' * 4092 + 'e',) for sname in valid_names: data['sname'] = sname @@ -132,9 +132,9 @@ def test_name_regex(self): self.assertEqual(response.data['detail'], e_msg) # Share name with more than 255 characters - e_msg = ('Share name must be less than 255 characters') + e_msg = ('Share name length cannot exceed 4096 characters') - data['sname']= 'Sh' + 'a' * 254 + 're' + data['sname']= 'Sh' + 'a' * 4093 + 're' response = self.client.post(self.BASE_URL, data=data) self.assertEqual(response.status_code, status.HTTP_500_INTERNAL_SERVER_ERROR, msg=response.data) @@ -248,13 +248,13 @@ def test_resize(self): self.assertEqual(response.status_code, status.HTTP_200_OK, msg=response.data) self.assertEqual(response.data['name'], 'share1') self.assertEqual(response.data['size'], 1000) - + # resize share data3 = {'size': 2000,} response3 = self.client.put('%s/share1' % self.BASE_URL, data=data3) self.assertEqual(response3.status_code, status.HTTP_200_OK, msg=response3.data) self.assertEqual(response3.data['size'], 2000) - + # resize a 'root' share data3 = {'size': 1500} response3 = self.client.put('%s/root' % self.BASE_URL, data=data3) @@ -262,7 +262,7 @@ def test_resize(self): status.HTTP_500_INTERNAL_SERVER_ERROR, msg=response3.data) e_msg = ('Operation not permitted on this Share(root) because it is a special system Share') self.assertEqual(response3.data['detail'], e_msg) - + # resize a 'home' share data3 = {'size': 1500} response3 = self.client.put('%s/home' % self.BASE_URL, data=data3) @@ -270,7 +270,7 @@ def test_resize(self): status.HTTP_500_INTERNAL_SERVER_ERROR, msg=response3.data) e_msg = ('Operation not permitted on this Share(home) because it is a special system Share') self.assertEqual(response3.data['detail'], e_msg) - + # resize to below current share usage value data3 = {'size': 400} response3 = self.client.put('%s/share1' % self.BASE_URL, data=data3) @@ -288,7 +288,7 @@ def test_resize(self): status.HTTP_500_INTERNAL_SERVER_ERROR, msg=response3.data) e_msg = ('Share size should atleast be 100KB. Given size is 99KB') self.assertEqual(response3.data['detail'], e_msg) - + # resize a share that doesn't exist data3 = {'sname': 'invalid', 'size': 1500} response3 = self.client.put('%s/invalid' % self.BASE_URL, data=data3) @@ -297,7 +297,7 @@ def test_resize(self): e_msg = ('Share(invalid) does not exist') self.assertEqual(response3.data['detail'], e_msg) - + def test_compression(self): """ @@ -375,7 +375,7 @@ def test_compression(self): @mock.patch('storageadmin.views.share.Snapshot') def test_delete_set1(self, mock_snapshot, mock_nfs, mock_samba, mock_sftp, mock_remove_share): """ - Test DELETE request on share + Test DELETE request on share 1. Create valid share 2. Delete share with replication related snapshots 3. Delete share with NFS export @@ -441,15 +441,15 @@ def test_delete_set1(self, mock_snapshot, mock_nfs, mock_samba, mock_sftp, mock_ status.HTTP_500_INTERNAL_SERVER_ERROR, msg=response5.data) self.assertEqual(response5.data['detail'], e_msg) mock_snapshot.objects.filter(share=share, snap_type='admin').exists.return_value = False - + # delete a share that doesn't exist e_msg = ('Share(invalid) does not exist') response9 = self.client.delete('%s/invalid' % self.BASE_URL) self.assertEqual(response9.status_code, status.HTTP_500_INTERNAL_SERVER_ERROR, msg=response9.data) self.assertEqual(response9.data['detail'], e_msg) - - @mock.patch('storageadmin.views.share.Service') + + @mock.patch('storageadmin.views.share.Service') def test_delete2(self, mock_service): # happy path # create share @@ -457,15 +457,15 @@ def test_delete2(self, mock_service): response = self.client.post(self.BASE_URL, data=data) self.assertEqual(response.status_code, status.HTTP_200_OK, msg=response.data) self.assertEqual(response.data['name'], 'rootshare') - - # Delete share - + + # Delete share + mock_service.objects.get.side_effect= None response7 = self.client.delete('%s/rootshare' % self.BASE_URL) self.assertEqual(response7.status_code, status.HTTP_200_OK, msg=response7.data) - - @mock.patch('storageadmin.views.share.Service') + + @mock.patch('storageadmin.views.share.Service') def test_delete3(self, mock_service): # unhappy path # create share @@ -473,8 +473,8 @@ def test_delete3(self, mock_service): response = self.client.post(self.BASE_URL, data=data) self.assertEqual(response.status_code, status.HTTP_200_OK, msg=response.data) self.assertEqual(response.data['name'], 'rootshare') - - # Delete share + + # Delete share e_msg = ('Failed to delete the Share(rootshare). Error from the OS: ') mock_service.objects.get.side_effect= None self.mock_remove_share.side_effect = Exception @@ -483,7 +483,7 @@ def test_delete3(self, mock_service): status.HTTP_500_INTERNAL_SERVER_ERROR, msg=response7.data) self.assertEqual(response7.data['detail'], e_msg) self.mock_remove_share.side_effect = None - + # Delete share that is in use by rock-on service class MockService(object): def __init__(self, **kwargs): @@ -492,12 +492,9 @@ def __init__(self, **kwargs): 'by Rock-on service. If you really need to delete ' 'it, (1)turn the service off, (2)change its ' 'configuration to use a different Share and then ' - '(3)try deleting this Share(%s) again.') + '(3)try deleting this Share(%s) again.') mock_service.objects.get.side_effect = MockService response7 = self.client.delete('%s/rootshare' % self.BASE_URL) self.assertEqual(response7.status_code, status.HTTP_500_INTERNAL_SERVER_ERROR, msg=response7.data) self.assertEqual(response7.data['detail'], e_msg) - - - \ No newline at end of file diff --git a/src/rockstor/storageadmin/views/share.py b/src/rockstor/storageadmin/views/share.py index 5370afb34..bd3266950 100644 --- a/src/rockstor/storageadmin/views/share.py +++ b/src/rockstor/storageadmin/views/share.py @@ -139,10 +139,10 @@ def post(self, request): 'hyphen(-), underscore(_) or a period(.).') handle_exception(Exception(e_msg), request) - if (len(sname) > 255): - e_msg = ('Share name must be less than 255 characters') + if (len(sname) > 4096): + e_msg = ('Share name length cannot exceed 4096 characters') handle_exception(Exception(e_msg), request) - + if (Share.objects.filter(name=sname).exists()): e_msg = ('Share(%s) already exists. Choose a different name' % sname) handle_exception(Exception(e_msg), request) From 2d503afe9c07e9e125300ba3dbcde22244e0145c Mon Sep 17 00:00:00 2001 From: Suman Chakravartula Date: Thu, 27 Aug 2015 13:45:46 -0700 Subject: [PATCH 5/5] turns out btrfs subvol names cannot exceed 254 characters. #828 --- src/rockstor/storageadmin/tests/test_shares.py | 8 ++++---- src/rockstor/storageadmin/views/share.py | 5 +++-- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/rockstor/storageadmin/tests/test_shares.py b/src/rockstor/storageadmin/tests/test_shares.py index ece0b9f00..0085469a2 100644 --- a/src/rockstor/storageadmin/tests/test_shares.py +++ b/src/rockstor/storageadmin/tests/test_shares.py @@ -105,13 +105,13 @@ def test_name_regex(self): 1. Test a few valid regexes (eg: share1, Myshare, 123, etc..) 2. Test a few invalid regexes (eg: -share1, .share etc..) 3. Empty string for share name - 4. max length(4096 characters) for share name + 4. max length(254 characters) for share name 5. max length + 1 for share name """ # valid share names data = {'pool': 'rockstor_rockstor', 'size': 1000} valid_names = ('123share', 'SHARE_TEST', 'Zzzz...', '1234', 'myshare', - 'Sha' + 'r' * 4092 + 'e',) + 'Sha' + 'r' * 250 + 'e',) for sname in valid_names: data['sname'] = sname @@ -132,9 +132,9 @@ def test_name_regex(self): self.assertEqual(response.data['detail'], e_msg) # Share name with more than 255 characters - e_msg = ('Share name length cannot exceed 4096 characters') + e_msg = ('Share name length cannot exceed 254 characters') - data['sname']= 'Sh' + 'a' * 4093 + 're' + data['sname']= 'Sh' + 'a' * 251 + 're' response = self.client.post(self.BASE_URL, data=data) self.assertEqual(response.status_code, status.HTTP_500_INTERNAL_SERVER_ERROR, msg=response.data) diff --git a/src/rockstor/storageadmin/views/share.py b/src/rockstor/storageadmin/views/share.py index bd3266950..fb4924c60 100644 --- a/src/rockstor/storageadmin/views/share.py +++ b/src/rockstor/storageadmin/views/share.py @@ -139,8 +139,9 @@ def post(self, request): 'hyphen(-), underscore(_) or a period(.).') handle_exception(Exception(e_msg), request) - if (len(sname) > 4096): - e_msg = ('Share name length cannot exceed 4096 characters') + if (len(sname) > 254): + #btrfs subvolume names cannot exceed 254 characters. + e_msg = ('Share name length cannot exceed 254 characters') handle_exception(Exception(e_msg), request) if (Share.objects.filter(name=sname).exists()):