Repository: Duke-GCB/DukeDSClient

Issue:
Error 403 downloading project from s3 backend
Error printed out by ddsclient:
```
requests.exceptions.HTTPError: 403 Client Error: Forbidden for 
url: https://eso.duhs.duke.edu//ca....
```
Payload returned by S3:
```
<Error><Code>SignatureDoesNotMatch</Code><Message>The request signature we calculated
does not match the signature you provided. Check your Secret Access Key and signing method. 
For more information, see REST Authentication and SOAP Authentication for details.
</Message><RequestId>...</RequestId></Error
```

Error seems to be due to an extra slash in the url.

Removing the slash from the following line fixes this issue:
https://github.com/Duke-GCB/DukeDSClient/blob/95e2d44613240a68d78da8c887acfbcab18f5136/ddsc/core/download.py#L424



Discussion:

Patch:
diff --git a/ddsc/core/download.py b/ddsc/core/download.py
index 545d438..b8ec446 100644
--- a/ddsc/core/download.py
+++ b/ddsc/core/download.py
@@ -12,6 +12,8 @@ from ddsc.core.remotestore import RemoteStore, ProjectFile, RemoteFileUrl
 FETCH_EXTERNAL_PUT_RETRY_TIMES = 5
 FETCH_EXTERNAL_RETRY_SECONDS = 20
 RESOURCE_NOT_CONSISTENT_RETRY_SECONDS = 2
+SWIFT_EXPIRED_STATUS_CODE = 401
+S3_EXPIRED_STATUS_CODE = 403
 
 
 class ProjectDownload(object):
@@ -421,7 +423,10 @@ class RetryChunkDownloader(object):
         headers = self.get_range_headers()
         if file_download.http_headers:
             headers.update(file_download.http_headers)
-        url = '{}/{}'.format(file_download.host, file_download.url)
+        separator = ""
+        if not file_download.url.startswith("/"):
+            separator = "/"
+        url = '{}{}{}'.format(file_download.host, separator, file_download.url)
         return url, headers
 
     def get_range_headers(self):
@@ -436,7 +441,8 @@ class RetryChunkDownloader(object):
         :param headers: dict: headers used to download this file chunk
         """
         response = requests.get(url, headers=headers, stream=True)
-        if response.status_code == 401:
+        if response.status_code == SWIFT_EXPIRED_STATUS_CODE \
+                or response.status_code == S3_EXPIRED_STATUS_CODE:
             raise DownloadInconsistentError(response.text)
         response.raise_for_status()
         self.actual_bytes_read = 0


Test patch:
diff --git a/ddsc/core/tests/test_download.py b/ddsc/core/tests/test_download.py
index 7535a22..00a620c 100644
--- a/ddsc/core/tests/test_download.py
+++ b/ddsc/core/tests/test_download.py
@@ -438,9 +438,23 @@ class TestRetryChunkDownloader(TestCase):
         with self.assertRaises(DownloadInconsistentError):
             downloader.retry_download_loop()
 
-    def test_get_url_and_headers_for_range(self):
+    def test_get_url_and_headers_for_range_with_no_slashes(self):
         mock_context = Mock()
         mock_file_download = Mock(host='somehost', url='someurl')
+        mock_file_download.http_headers = {}
+        downloader = RetryChunkDownloader(project_file=None, local_path=None, seek_amt=None,
+                                          bytes_to_read=None, download_context=mock_context)
+
+        downloader.get_range_headers = Mock()
+        downloader.get_range_headers.return_value = {'Range': 'bytes=100-200'}
+
+        url, headers = downloader.get_url_and_headers_for_range(mock_file_download)
+
+        self.assertEqual(url, 'somehost/someurl')
+
+    def test_get_url_and_headers_for_range(self):
+        mock_context = Mock()
+        mock_file_download = Mock(host='somehost', url='/someurl')
         mock_file_download.http_headers = {'secret': '222'}
         downloader = RetryChunkDownloader(project_file=None, local_path=None, seek_amt=None,
                                           bytes_to_read=None, download_context=mock_context)
@@ -463,16 +477,28 @@ class TestRetryChunkDownloader(TestCase):
         self.assertEqual(downloader.get_range_headers(), {'Range': 'bytes=200-299'})
 
     @patch('ddsc.core.download.requests')
-    def test_download_chunk_inconsistent(self, mock_requests):
+    def test_download_chunk_inconsistent_swift(self, mock_requests):
+        mock_context = Mock()
+        downloader = RetryChunkDownloader(project_file=None, local_path=None, seek_amt=None,
+                                          bytes_to_read=None, download_context=mock_context)
+
+        mock_requests.get.return_value = Mock(status_code=401, text='Bad Swift Download')
+
+        with self.assertRaises(DownloadInconsistentError) as raised_exception:
+            downloader.download_chunk('someurl', {})
+            self.assertEqual(str(raised_exception), 'Bad Swift Download')
+
+    @patch('ddsc.core.download.requests')
+    def test_download_chunk_inconsistent_s3(self, mock_requests):
         mock_context = Mock()
         downloader = RetryChunkDownloader(project_file=None, local_path=None, seek_amt=None,
                                           bytes_to_read=None, download_context=mock_context)
 
-        mock_requests.get.return_value = Mock(status_code=401, text='Bad Download')
+        mock_requests.get.return_value = Mock(status_code=403, text='Bad S3 Download')
 
         with self.assertRaises(DownloadInconsistentError) as raised_exception:
             downloader.download_chunk('someurl', {})
-            self.assertEqual(str(raised_exception), 'Bad Download')
+            self.assertEqual(str(raised_exception), 'Bad S3 Download')
 
     @patch('ddsc.core.download.requests')
     def test_download_chunk_works(self, mock_requests):