From ed055c172b8540160753b5973acae7ffd15922ac Mon Sep 17 00:00:00 2001 From: Rakshith Bhyravabhotla Date: Wed, 6 Nov 2019 15:54:35 -0800 Subject: [PATCH 1/5] Improve Connection String message --- .../azure/storage/blob/_shared/base_client.py | 7 ++++--- .../azure/storage/fileshare/_shared/base_client.py | 7 ++++--- .../azure/storage/queue/_shared/base_client.py | 7 ++++--- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/sdk/storage/azure-storage-blob/azure/storage/blob/_shared/base_client.py b/sdk/storage/azure-storage-blob/azure/storage/blob/_shared/base_client.py index 9ad754b92b14..dee5ff10469e 100644 --- a/sdk/storage/azure-storage-blob/azure/storage/blob/_shared/base_client.py +++ b/sdk/storage/azure-storage-blob/azure/storage/blob/_shared/base_client.py @@ -317,9 +317,10 @@ def format_shared_key_credential(account, credential): def parse_connection_str(conn_str, credential, service): conn_str = conn_str.rstrip(";") - conn_settings = dict( # pylint: disable=consider-using-dict-comprehension - [s.split("=", 1) for s in conn_str.split(";")] - ) + conn_settings = [s.split("=", 1) for s in conn_str.split(";")] + if any([len(i) != 2 for i in conn_settings]): + raise ValueError("Connection string is either missing or malformed.") + conn_settings = dict(conn_settings) endpoints = _SERVICE_PARAMS[service] primary = None secondary = None diff --git a/sdk/storage/azure-storage-file-share/azure/storage/fileshare/_shared/base_client.py b/sdk/storage/azure-storage-file-share/azure/storage/fileshare/_shared/base_client.py index 9ad754b92b14..dee5ff10469e 100644 --- a/sdk/storage/azure-storage-file-share/azure/storage/fileshare/_shared/base_client.py +++ b/sdk/storage/azure-storage-file-share/azure/storage/fileshare/_shared/base_client.py @@ -317,9 +317,10 @@ def format_shared_key_credential(account, credential): def parse_connection_str(conn_str, credential, service): conn_str = conn_str.rstrip(";") - conn_settings = dict( # pylint: disable=consider-using-dict-comprehension - [s.split("=", 1) for s in conn_str.split(";")] - ) + conn_settings = [s.split("=", 1) for s in conn_str.split(";")] + if any([len(i) != 2 for i in conn_settings]): + raise ValueError("Connection string is either missing or malformed.") + conn_settings = dict(conn_settings) endpoints = _SERVICE_PARAMS[service] primary = None secondary = None diff --git a/sdk/storage/azure-storage-queue/azure/storage/queue/_shared/base_client.py b/sdk/storage/azure-storage-queue/azure/storage/queue/_shared/base_client.py index 9ad754b92b14..dee5ff10469e 100644 --- a/sdk/storage/azure-storage-queue/azure/storage/queue/_shared/base_client.py +++ b/sdk/storage/azure-storage-queue/azure/storage/queue/_shared/base_client.py @@ -317,9 +317,10 @@ def format_shared_key_credential(account, credential): def parse_connection_str(conn_str, credential, service): conn_str = conn_str.rstrip(";") - conn_settings = dict( # pylint: disable=consider-using-dict-comprehension - [s.split("=", 1) for s in conn_str.split(";")] - ) + conn_settings = [s.split("=", 1) for s in conn_str.split(";")] + if any([len(i) != 2 for i in conn_settings]): + raise ValueError("Connection string is either missing or malformed.") + conn_settings = dict(conn_settings) endpoints = _SERVICE_PARAMS[service] primary = None secondary = None From 0b2360bb73e0d13f556817290d17d32627f86cb8 Mon Sep 17 00:00:00 2001 From: Rakshith Bhyravabhotla Date: Wed, 6 Nov 2019 16:17:24 -0800 Subject: [PATCH 2/5] change --- .../azure/storage/blob/_shared/base_client.py | 4 ++-- .../azure/storage/fileshare/_shared/base_client.py | 4 ++-- .../azure/storage/queue/_shared/base_client.py | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/sdk/storage/azure-storage-blob/azure/storage/blob/_shared/base_client.py b/sdk/storage/azure-storage-blob/azure/storage/blob/_shared/base_client.py index dee5ff10469e..e7452997cc32 100644 --- a/sdk/storage/azure-storage-blob/azure/storage/blob/_shared/base_client.py +++ b/sdk/storage/azure-storage-blob/azure/storage/blob/_shared/base_client.py @@ -318,8 +318,8 @@ def format_shared_key_credential(account, credential): def parse_connection_str(conn_str, credential, service): conn_str = conn_str.rstrip(";") conn_settings = [s.split("=", 1) for s in conn_str.split(";")] - if any([len(i) != 2 for i in conn_settings]): - raise ValueError("Connection string is either missing or malformed.") + if any([len(tup) != 2 for tup in conn_settings]): + raise ValueError("Connection string is either blank or malformed.") conn_settings = dict(conn_settings) endpoints = _SERVICE_PARAMS[service] primary = None diff --git a/sdk/storage/azure-storage-file-share/azure/storage/fileshare/_shared/base_client.py b/sdk/storage/azure-storage-file-share/azure/storage/fileshare/_shared/base_client.py index dee5ff10469e..e7452997cc32 100644 --- a/sdk/storage/azure-storage-file-share/azure/storage/fileshare/_shared/base_client.py +++ b/sdk/storage/azure-storage-file-share/azure/storage/fileshare/_shared/base_client.py @@ -318,8 +318,8 @@ def format_shared_key_credential(account, credential): def parse_connection_str(conn_str, credential, service): conn_str = conn_str.rstrip(";") conn_settings = [s.split("=", 1) for s in conn_str.split(";")] - if any([len(i) != 2 for i in conn_settings]): - raise ValueError("Connection string is either missing or malformed.") + if any([len(tup) != 2 for tup in conn_settings]): + raise ValueError("Connection string is either blank or malformed.") conn_settings = dict(conn_settings) endpoints = _SERVICE_PARAMS[service] primary = None diff --git a/sdk/storage/azure-storage-queue/azure/storage/queue/_shared/base_client.py b/sdk/storage/azure-storage-queue/azure/storage/queue/_shared/base_client.py index dee5ff10469e..e7452997cc32 100644 --- a/sdk/storage/azure-storage-queue/azure/storage/queue/_shared/base_client.py +++ b/sdk/storage/azure-storage-queue/azure/storage/queue/_shared/base_client.py @@ -318,8 +318,8 @@ def format_shared_key_credential(account, credential): def parse_connection_str(conn_str, credential, service): conn_str = conn_str.rstrip(";") conn_settings = [s.split("=", 1) for s in conn_str.split(";")] - if any([len(i) != 2 for i in conn_settings]): - raise ValueError("Connection string is either missing or malformed.") + if any([len(tup) != 2 for tup in conn_settings]): + raise ValueError("Connection string is either blank or malformed.") conn_settings = dict(conn_settings) endpoints = _SERVICE_PARAMS[service] primary = None From 15a9925151d0b1e267a861c381c0a7f3f4ab58f6 Mon Sep 17 00:00:00 2001 From: Rakshith Bhyravabhotla Date: Wed, 6 Nov 2019 16:39:22 -0800 Subject: [PATCH 3/5] add tests --- .../azure/storage/blob/_shared/base_client.py | 2 +- .../azure-storage-blob/tests/test_blob_client.py | 12 ++++++++++++ .../azure/storage/fileshare/_shared/base_client.py | 2 +- .../tests/test_file_client.py | 10 ++++++++++ .../azure/storage/queue/_shared/base_client.py | 2 +- .../azure-storage-queue/tests/test_queue_client.py | 13 +++++++++++++ 6 files changed, 38 insertions(+), 3 deletions(-) diff --git a/sdk/storage/azure-storage-blob/azure/storage/blob/_shared/base_client.py b/sdk/storage/azure-storage-blob/azure/storage/blob/_shared/base_client.py index e7452997cc32..7ae31d69679c 100644 --- a/sdk/storage/azure-storage-blob/azure/storage/blob/_shared/base_client.py +++ b/sdk/storage/azure-storage-blob/azure/storage/blob/_shared/base_client.py @@ -318,7 +318,7 @@ def format_shared_key_credential(account, credential): def parse_connection_str(conn_str, credential, service): conn_str = conn_str.rstrip(";") conn_settings = [s.split("=", 1) for s in conn_str.split(";")] - if any([len(tup) != 2 for tup in conn_settings]): + if any(len(tup) != 2 for tup in conn_settings): raise ValueError("Connection string is either blank or malformed.") conn_settings = dict(conn_settings) endpoints = _SERVICE_PARAMS[service] diff --git a/sdk/storage/azure-storage-blob/tests/test_blob_client.py b/sdk/storage/azure-storage-blob/tests/test_blob_client.py index 9c1393206320..60a11e8baa39 100644 --- a/sdk/storage/azure-storage-blob/tests/test_blob_client.py +++ b/sdk/storage/azure-storage-blob/tests/test_blob_client.py @@ -559,5 +559,17 @@ def callback(response): custom_headers = {'User-Agent': 'customer_user_agent'} service.get_service_properties(raw_response_hook=callback, headers=custom_headers) + @GlobalStorageAccountPreparer() + def test_error_with_blank_conn_str(self, resource_group, location, storage_account, storage_account_key): + # Arrange + + for service_type in SERVICES.items(): + # Act + with self.assertRaises(ValueError) as e: + service = service_type[0].from_connection_string("", blob_name="test", container_name="container") + + self.assertEqual( + str(e.exception), "Connection string is either blank or malformed.") + # ------------------------------------------------------------------------------ diff --git a/sdk/storage/azure-storage-file-share/azure/storage/fileshare/_shared/base_client.py b/sdk/storage/azure-storage-file-share/azure/storage/fileshare/_shared/base_client.py index e7452997cc32..7ae31d69679c 100644 --- a/sdk/storage/azure-storage-file-share/azure/storage/fileshare/_shared/base_client.py +++ b/sdk/storage/azure-storage-file-share/azure/storage/fileshare/_shared/base_client.py @@ -318,7 +318,7 @@ def format_shared_key_credential(account, credential): def parse_connection_str(conn_str, credential, service): conn_str = conn_str.rstrip(";") conn_settings = [s.split("=", 1) for s in conn_str.split(";")] - if any([len(tup) != 2 for tup in conn_settings]): + if any(len(tup) != 2 for tup in conn_settings): raise ValueError("Connection string is either blank or malformed.") conn_settings = dict(conn_settings) endpoints = _SERVICE_PARAMS[service] diff --git a/sdk/storage/azure-storage-file-share/tests/test_file_client.py b/sdk/storage/azure-storage-file-share/tests/test_file_client.py index 2a0cf4e43db7..cef200af0ac7 100644 --- a/sdk/storage/azure-storage-file-share/tests/test_file_client.py +++ b/sdk/storage/azure-storage-file-share/tests/test_file_client.py @@ -401,6 +401,16 @@ def callback(response): custom_headers = {'User-Agent': 'customer_user_agent'} service.get_service_properties(raw_response_hook=callback, headers=custom_headers) + def test_error_with_blank_conn_str(self): + # Arrange + + for service_type in SERVICES.items(): + # Act + with self.assertRaises(ValueError) as e: + service = service_type[0].from_connection_string("", share_name="test", directory_path="foo/bar", file_path="temp/dat") + + self.assertEqual( + str(e.exception), "Connection string is either blank or malformed.") # ------------------------------------------------------------------------------ if __name__ == '__main__': diff --git a/sdk/storage/azure-storage-queue/azure/storage/queue/_shared/base_client.py b/sdk/storage/azure-storage-queue/azure/storage/queue/_shared/base_client.py index e7452997cc32..7ae31d69679c 100644 --- a/sdk/storage/azure-storage-queue/azure/storage/queue/_shared/base_client.py +++ b/sdk/storage/azure-storage-queue/azure/storage/queue/_shared/base_client.py @@ -318,7 +318,7 @@ def format_shared_key_credential(account, credential): def parse_connection_str(conn_str, credential, service): conn_str = conn_str.rstrip(";") conn_settings = [s.split("=", 1) for s in conn_str.split(";")] - if any([len(tup) != 2 for tup in conn_settings]): + if any(len(tup) != 2 for tup in conn_settings): raise ValueError("Connection string is either blank or malformed.") conn_settings = dict(conn_settings) endpoints = _SERVICE_PARAMS[service] diff --git a/sdk/storage/azure-storage-queue/tests/test_queue_client.py b/sdk/storage/azure-storage-queue/tests/test_queue_client.py index 47c36d7e2339..159ea90f5f56 100644 --- a/sdk/storage/azure-storage-queue/tests/test_queue_client.py +++ b/sdk/storage/azure-storage-queue/tests/test_queue_client.py @@ -458,6 +458,19 @@ def test_create_queue_client_with_complete_queue_url(self, resource_group, locat # Assert self.assertEqual(service.scheme, 'https') self.assertEqual(service.queue_name, 'bar') + + @GlobalStorageAccountPreparer() + def test_error_with_blank_conn_str(self, resource_group, location, storage_account, storage_account_key): + # Arrange + + for service_type in SERVICES.items(): + # Act + with self.assertRaises(ValueError) as e: + service = service_type[0].from_connection_string("", queue_name="test") + + self.assertEqual( + str(e.exception), "Connection string is either blank or malformed.") + # ------------------------------------------------------------------------------ if __name__ == '__main__': unittest.main() From 3ea7c04de4c664fa41c2de29109ff21d485e1b0a Mon Sep 17 00:00:00 2001 From: Rakshith Bhyravabhotla Date: Thu, 7 Nov 2019 11:25:22 -0800 Subject: [PATCH 4/5] changes --- .../tests/test_blob_client.py | 18 +++++++++--------- .../tests/test_file_client.py | 17 +++++++++-------- .../tests/test_queue_client.py | 18 +++++++++--------- 3 files changed, 27 insertions(+), 26 deletions(-) diff --git a/sdk/storage/azure-storage-blob/tests/test_blob_client.py b/sdk/storage/azure-storage-blob/tests/test_blob_client.py index 60a11e8baa39..180f01be27e9 100644 --- a/sdk/storage/azure-storage-blob/tests/test_blob_client.py +++ b/sdk/storage/azure-storage-blob/tests/test_blob_client.py @@ -559,17 +559,17 @@ def callback(response): custom_headers = {'User-Agent': 'customer_user_agent'} service.get_service_properties(raw_response_hook=callback, headers=custom_headers) - @GlobalStorageAccountPreparer() - def test_error_with_blank_conn_str(self, resource_group, location, storage_account, storage_account_key): + def test_error_with_malformed_conn_str(self): # Arrange - for service_type in SERVICES.items(): - # Act - with self.assertRaises(ValueError) as e: - service = service_type[0].from_connection_string("", blob_name="test", container_name="container") - - self.assertEqual( - str(e.exception), "Connection string is either blank or malformed.") + for conn_str in ["", "foobar", "foobar=baz=foo", "foo;bar;baz", "foo=;bar=;", "=", ";", "=;=="]: + for service_type in SERVICES.items(): + # Act + with self.assertRaises(ValueError) as e: + service = service_type[0].from_connection_string(conn_str, blob_name="test", container_name="foo/bar") + + self.assertEqual( + str(e.exception), "Connection string is either blank or malformed.") # ------------------------------------------------------------------------------ diff --git a/sdk/storage/azure-storage-file-share/tests/test_file_client.py b/sdk/storage/azure-storage-file-share/tests/test_file_client.py index cef200af0ac7..4e91fd6470ec 100644 --- a/sdk/storage/azure-storage-file-share/tests/test_file_client.py +++ b/sdk/storage/azure-storage-file-share/tests/test_file_client.py @@ -401,16 +401,17 @@ def callback(response): custom_headers = {'User-Agent': 'customer_user_agent'} service.get_service_properties(raw_response_hook=callback, headers=custom_headers) - def test_error_with_blank_conn_str(self): + def test_error_with_malformed_conn_str(self): # Arrange - for service_type in SERVICES.items(): - # Act - with self.assertRaises(ValueError) as e: - service = service_type[0].from_connection_string("", share_name="test", directory_path="foo/bar", file_path="temp/dat") - - self.assertEqual( - str(e.exception), "Connection string is either blank or malformed.") + for conn_str in ["", "foobar", "foobar=baz=foo", "foo;bar;baz", "foo=;bar=;", "=", ";", "=;=="]: + for service_type in SERVICES.items(): + # Act + with self.assertRaises(ValueError) as e: + service = service_type[0].from_connection_string(conn_str, share_name="test", directory_path="foo/bar", file_path="temp/dat") + + self.assertEqual( + str(e.exception), "Connection string is either blank or malformed.") # ------------------------------------------------------------------------------ if __name__ == '__main__': diff --git a/sdk/storage/azure-storage-queue/tests/test_queue_client.py b/sdk/storage/azure-storage-queue/tests/test_queue_client.py index 159ea90f5f56..4f0f07d03ca7 100644 --- a/sdk/storage/azure-storage-queue/tests/test_queue_client.py +++ b/sdk/storage/azure-storage-queue/tests/test_queue_client.py @@ -459,17 +459,17 @@ def test_create_queue_client_with_complete_queue_url(self, resource_group, locat self.assertEqual(service.scheme, 'https') self.assertEqual(service.queue_name, 'bar') - @GlobalStorageAccountPreparer() - def test_error_with_blank_conn_str(self, resource_group, location, storage_account, storage_account_key): + def test_error_with_malformed_conn_str(self): # Arrange - for service_type in SERVICES.items(): - # Act - with self.assertRaises(ValueError) as e: - service = service_type[0].from_connection_string("", queue_name="test") - - self.assertEqual( - str(e.exception), "Connection string is either blank or malformed.") + for conn_str in ["", "foobar", "foobar=baz=foo", "foo;bar;baz", "foo=;bar=;", "=", ";", "=;=="]: + for service_type in SERVICES.items(): + # Act + with self.assertRaises(ValueError) as e: + service = service_type[0].from_connection_string(conn_str, queue_name="test") + + self.assertEqual( + str(e.exception), "Connection string is either blank or malformed.") # ------------------------------------------------------------------------------ if __name__ == '__main__': From 1e84063ae230d2b88eb91a39a28c7512cd6a2bde Mon Sep 17 00:00:00 2001 From: Rakshith Bhyravabhotla Date: Fri, 8 Nov 2019 10:56:35 -0800 Subject: [PATCH 5/5] changes --- .../azure-storage-blob/tests/test_blob_client.py | 11 +++++++---- .../tests/test_file_client.py | 8 ++++++-- .../azure-storage-queue/tests/test_queue_client.py | 8 ++++++-- 3 files changed, 19 insertions(+), 8 deletions(-) diff --git a/sdk/storage/azure-storage-blob/tests/test_blob_client.py b/sdk/storage/azure-storage-blob/tests/test_blob_client.py index 180f01be27e9..18a4dcd38bf1 100644 --- a/sdk/storage/azure-storage-blob/tests/test_blob_client.py +++ b/sdk/storage/azure-storage-blob/tests/test_blob_client.py @@ -561,15 +561,18 @@ def callback(response): def test_error_with_malformed_conn_str(self): # Arrange - - for conn_str in ["", "foobar", "foobar=baz=foo", "foo;bar;baz", "foo=;bar=;", "=", ";", "=;=="]: + for conn_str in ["", "foobar", "foo;bar;baz", ";", "foobar=baz=foo" , "foo=;bar=;", "=", "=;=="]: for service_type in SERVICES.items(): # Act with self.assertRaises(ValueError) as e: service = service_type[0].from_connection_string(conn_str, blob_name="test", container_name="foo/bar") - self.assertEqual( - str(e.exception), "Connection string is either blank or malformed.") + if conn_str in("", "foobar", "foo;bar;baz", ";"): + self.assertEqual( + str(e.exception), "Connection string is either blank or malformed.") + elif conn_str in ("foobar=baz=foo" , "foo=;bar=;", "=", "=;=="): + self.assertEqual( + str(e.exception), "Connection string missing required connection details.") # ------------------------------------------------------------------------------ diff --git a/sdk/storage/azure-storage-file-share/tests/test_file_client.py b/sdk/storage/azure-storage-file-share/tests/test_file_client.py index 4e91fd6470ec..b2139249a434 100644 --- a/sdk/storage/azure-storage-file-share/tests/test_file_client.py +++ b/sdk/storage/azure-storage-file-share/tests/test_file_client.py @@ -410,8 +410,12 @@ def test_error_with_malformed_conn_str(self): with self.assertRaises(ValueError) as e: service = service_type[0].from_connection_string(conn_str, share_name="test", directory_path="foo/bar", file_path="temp/dat") - self.assertEqual( - str(e.exception), "Connection string is either blank or malformed.") + if conn_str in("", "foobar", "foo;bar;baz", ";"): + self.assertEqual( + str(e.exception), "Connection string is either blank or malformed.") + elif conn_str in ("foobar=baz=foo" , "foo=;bar=;", "=", "=;=="): + self.assertEqual( + str(e.exception), "Connection string missing required connection details.") # ------------------------------------------------------------------------------ if __name__ == '__main__': diff --git a/sdk/storage/azure-storage-queue/tests/test_queue_client.py b/sdk/storage/azure-storage-queue/tests/test_queue_client.py index 4f0f07d03ca7..e4b21e8120c4 100644 --- a/sdk/storage/azure-storage-queue/tests/test_queue_client.py +++ b/sdk/storage/azure-storage-queue/tests/test_queue_client.py @@ -468,8 +468,12 @@ def test_error_with_malformed_conn_str(self): with self.assertRaises(ValueError) as e: service = service_type[0].from_connection_string(conn_str, queue_name="test") - self.assertEqual( - str(e.exception), "Connection string is either blank or malformed.") + if conn_str in("", "foobar", "foo;bar;baz", ";"): + self.assertEqual( + str(e.exception), "Connection string is either blank or malformed.") + elif conn_str in ("foobar=baz=foo" , "foo=;bar=;", "=", "=;=="): + self.assertEqual( + str(e.exception), "Connection string missing required connection details.") # ------------------------------------------------------------------------------ if __name__ == '__main__':