Add checks to ensure UID have correct format

It may easily happen with DICOM JSON that one passes a list of UIDs
rather than an individual UID.
This commit adds checks that ensure that provided UIDs have the correct
type and the expected format.

src/dicomweb_client/api.py

@@ -675,7 +675,9 @@ def retrieve_study(self, study_instance_uid):
 
         '''
         if study_instance_uid is None:
-            raise ValueError('Study UID is required for retrieval of study.')
+            raise ValueError(
+                'Study Instance UID is required for retrieval of study.'
+            )
         url = self._get_studies_url(study_instance_uid)
         return self._http_get_multipart_application_dicom(url)
 
@@ -695,12 +697,36 @@ def retrieve_study_metadata(self, study_instance_uid):
         '''
         if study_instance_uid is None:
             raise ValueError(
-                'Study UID is required for retrieval of study metadata.'
+                'Study Instance UID is required for retrieval of '
+                'study metadata.'
             )
         url = self._get_studies_url(study_instance_uid)
         url += '/metadata'
         return self._http_get_application_json(url)
 
+    def _check_uid_format(self, uid):
+        '''Checks whether a DICOM UID has the correct format.
+
+        Parameters
+        ----------
+        uid: str
+            DICOM UID
+
+        Raises
+        ------
+        TypeError
+            when `uid` is not a string
+        ValueError
+            when `uid` doesn't match the regular expression pattern
+            ``"^[.0-9]+$"``
+
+        '''
+        if not isinstance(uid, six.string_types):
+            raise TypeError('DICOM UID must be a string.')
+        pattern = re.compile('^[.0-9]+$')
+        if not pattern.search(uid):
+            raise ValueError('DICOM UID has invalid format.')
+
     def search_for_series(self, study_instance_uid=None, fuzzymatching=None,
                           limit=None, offset=None, fields=None,
                           search_filters={}):
@@ -730,6 +756,8 @@ def search_for_series(self, study_instance_uid=None, fuzzymatching=None,
             (see `returned attributes <http://dicom.nema.org/medical/dicom/current/output/chtml/part18/sect_6.7.html#table_6.7.1-2a>`_)
 
         ''' # noqa
+        if study_instance_uid is not None:
+            self._check_uid_format(study_instance_uid)
         url = self._get_series_url(study_instance_uid)
         params = self._parse_query_parameters(
             fuzzymatching, limit, offset, fields, **search_filters
@@ -758,9 +786,15 @@ def retrieve_series(self, study_instance_uid, series_instance_uid):
 
         '''
         if study_instance_uid is None:
-            raise ValueError('Study UID is required for retrieval of series.')
+            raise ValueError(
+                'Study Instance UID is required for retrieval of series.'
+            )
+        self._check_uid_format(study_instance_uid)
         if series_instance_uid is None:
-            raise ValueError('Series UID is required for retrieval of series.')
+            raise ValueError(
+                'Series Instance UID is required for retrieval of series.'
+            )
+        self._check_uid_format(series_instance_uid)
         url = self._get_series_url(study_instance_uid, series_instance_uid)
         return self._http_get_multipart_application_dicom(url)
 
@@ -782,12 +816,16 @@ def retrieve_series_metadata(self, study_instance_uid, series_instance_uid):
         '''
         if study_instance_uid is None:
             raise ValueError(
-                'Study UID is required for retrieval of series metadata.'
+                'Study Instance UID is required for retrieval of '
+                'series metadata.'
             )
+        self._check_uid_format(study_instance_uid)
         if series_instance_uid is None:
             raise ValueError(
-                'Series UID is required for retrieval of series metadata.'
+                'Series Instance UID is required for retrieval of '
+                'series metadata.'
             )
+        self._check_uid_format(series_instance_uid)
         url = self._get_series_url(study_instance_uid, series_instance_uid)
         url += '/metadata'
         return self._http_get_application_json(url)
@@ -824,6 +862,8 @@ def search_for_instances(self, study_instance_uid=None,
             (see `returned attributes <http://dicom.nema.org/medical/dicom/current/output/chtml/part18/sect_6.7.html#table_6.7.1-2b>`_)
 
         ''' # noqa
+        if study_instance_uid is not None:
+            self._check_uid_format(study_instance_uid)
         url = self._get_instances_url(study_instance_uid, series_instance_uid)
         params = self._parse_query_parameters(
             fuzzymatching, limit, offset, fields, **search_filters
@@ -857,16 +897,19 @@ def retrieve_instance(self, study_instance_uid, series_instance_uid,
 
         if study_instance_uid is None:
             raise ValueError(
-                'Study UID is required for retrieval of instance.'
+                'Study Instance UID is required for retrieval of instance.'
             )
+        self._check_uid_format(study_instance_uid)
         if series_instance_uid is None:
             raise ValueError(
-                'Series UID is required for retrieval of instance.'
+                'Series Instance UID is required for retrieval of instance.'
             )
+        self._check_uid_format(series_instance_uid)
         if sop_instance_uid is None:
             raise ValueError(
-                'Instance UID is required for retrieval of instance.'
+                'SOP Instance UID is required for retrieval of instance.'
             )
+        self._check_uid_format(sop_instance_uid)
         url = self._get_instances_url(
             study_instance_uid, series_instance_uid, sop_instance_uid
         )
@@ -913,15 +956,18 @@ def retrieve_instance_metadata(self, study_instance_uid,
         '''
         if study_instance_uid is None:
             raise ValueError(
-                'Study UID is required for retrieval of instance metadata.'
+                'Study Instance UID is required for retrieval of '
+                'instance metadata.'
             )
         if series_instance_uid is None:
             raise ValueError(
-                'Series UID is required for retrieval of instance metadata.'
+                'Series Instance UID is required for retrieval of '
+                'instance metadata.'
             )
         if sop_instance_uid is None:
             raise ValueError(
-                'Instance UID is required for retrieval of instance metadata.'
+                'SOP Instance UID is required for retrieval of '
+                'instance metadata.'
             )
         url = self._get_instances_url(
             study_instance_uid, series_instance_uid, sop_instance_uid
@@ -961,15 +1007,15 @@ def retrieve_instance_frames(self, study_instance_uid, series_instance_uid,
         '''
         if study_instance_uid is None:
             raise ValueError(
-                'Study UID is required for retrieval of instance frames.'
+                'Study Instance UID is required for retrieval of frames.'
             )
         if series_instance_uid is None:
             raise ValueError(
-                'Series UID is required for retrieval of instance frames.'
+                'Series Instance UID is required for retrieval of frames.'
             )
         if sop_instance_uid is None:
             raise ValueError(
-                'Instance UID is required for retrieval of instance frames.'
+                'SOP Instance UID is required for retrieval of frames.'
             )
         url = self._get_instances_url(
             study_instance_uid, series_instance_uid, sop_instance_uid

src/dicomweb_client/tests/test_api.py

@@ -2,6 +2,7 @@
 import json
 from io import BytesIO
 
+import pytest
 import pydicom
 
 
@@ -50,6 +51,18 @@ def test_search_for_series(httpserver, client, cache_dir):
     assert request.accept_mimetypes[0][0] == 'application/dicom+json'
 
 
+def test_search_for_series_wrong_uid_type(httpserver, client, cache_dir):
+    study_instance_uid = ['1.2.3.4']
+    with pytest.raises(TypeError):
+        client.search_for_series(study_instance_uid=study_instance_uid)
+
+
+def test_search_for_series_wrong_uid_value(httpserver, client, cache_dir):
+    study_instance_uid = '1_2_3_4'
+    with pytest.raises(ValueError):
+        client.search_for_series(study_instance_uid=study_instance_uid)
+
+
 def test_search_for_series_limit_offset(httpserver, client, cache_dir):
     cache_filename = os.path.join(cache_dir, 'search_for_series.json')
     with open(cache_filename, 'r') as f: