Repository: getmoto/moto
Issue: S3 - Downloading large files results in 416 Requested Range Not Satisfiable error

Hello,
We've recently updated `boto3` and `moto` from
```
boto3 = "1.9.115"
moto = "1.3.7"
```
to
```
boto3 = "^1.12.16"
moto = "^1.3.14"
```
(using [poetry](https://github.com/python-poetry/poetry) for dependency management)

One of our tests started failing _sometimes_. This is the problematic test:
```python
@mock_s3
class PdfFileDownloaderTestCase(TestCase):
    def setUp(self):
        boto3.client('s3').create_bucket(Bucket=os.environ['AWS_BUCKET_NAME'])
        self.file = open('web/tests/fixtures/asdf.pdf', 'rb')
        self.uploader = PdfFileUploader(self.file)
        self.uploader.upload()

    def test_delete_downloaded_file(self):
        # the following two lines are basically 
        # boto3.client('s3').download_file(bucket_name, file_name, local_path)
        # where bucket_name, file_name and local_path are retrieved from PdfFileUploader
        # e.g. boto3.client('s3').download_file("bucket_name", "asdf.pdf", "/tmp/asdf.pdf")
        downloader = PdfFileDownloader(self.uploader.full_bucket_file_name())
        downloader.download() 

        downloader.delete_downloaded_file()

        self.assertRaises(FileNotFoundError, downloader.file)
```

Error stacktrace:
```
Traceback (most recent call last):
  File "/Users/myuser/projects/project/web/pdf_file_downloader.py", line 16, in download
    self.s3.download_file(self.bucket_name, self.file_name, self.local_path())
  File "/Users/myuser/Library/Caches/pypoetry/virtualenvs/project-X5XCMv3p-py3.6/lib/python3.6/site-packages/boto3/s3/inject.py", line 172, in download_file
    extra_args=ExtraArgs, callback=Callback)
  File "/Users/myuser/Library/Caches/pypoetry/virtualenvs/project-X5XCMv3p-py3.6/lib/python3.6/site-packages/boto3/s3/transfer.py", line 307, in download_file
    future.result()
  File "/Users/myuser/Library/Caches/pypoetry/virtualenvs/project-X5XCMv3p-py3.6/lib/python3.6/site-packages/s3transfer/futures.py", line 106, in result
    return self._coordinator.result()
  File "/Users/myuser/Library/Caches/pypoetry/virtualenvs/project-X5XCMv3p-py3.6/lib/python3.6/site-packages/s3transfer/futures.py", line 265, in result
    raise self._exception
  File "/Users/myuser/Library/Caches/pypoetry/virtualenvs/project-X5XCMv3p-py3.6/lib/python3.6/site-packages/s3transfer/tasks.py", line 126, in __call__
    return self._execute_main(kwargs)
  File "/Users/myuser/Library/Caches/pypoetry/virtualenvs/project-X5XCMv3p-py3.6/lib/python3.6/site-packages/s3transfer/tasks.py", line 150, in _execute_main
    return_value = self._main(**kwargs)
  File "/Users/myuser/Library/Caches/pypoetry/virtualenvs/project-X5XCMv3p-py3.6/lib/python3.6/site-packages/s3transfer/download.py", line 512, in _main
    Bucket=bucket, Key=key, **extra_args)
  File "/Users/myuser/Library/Caches/pypoetry/virtualenvs/project-X5XCMv3p-py3.6/lib/python3.6/site-packages/botocore/client.py", line 316, in _api_call
    return self._make_api_call(operation_name, kwargs)
  File "/Users/myuser/Library/Caches/pypoetry/virtualenvs/project-X5XCMv3p-py3.6/lib/python3.6/site-packages/botocore/client.py", line 626, in _make_api_call
    raise error_class(parsed_response, operation_name)
botocore.exceptions.ClientError: An error occurred (416) when calling the GetObject operation: Requested Range Not Satisfiable
```

Upload and download classes:
```python
import os
import re
import boto3
from pathlib import Path
from botocore.exceptions import ClientError
from django.http import Http404


class PdfFileDownloader:
    def __init__(self, full_bucket_file_name):
        self.bucket_name, self.file_name = self.extract(full_bucket_file_name)
        self.s3 = boto3.client('s3')

    def download(self):
        try:
            import pudb; pu.db
            self.s3.download_file(self.bucket_name, self.file_name, self.local_path())

            return self.local_path()
        except ClientError as exc:
            raise Http404

    def local_path(self):
        return '/tmp/' + self.file_name.replace('/', '')

    def delete_downloaded_file(self):
        if Path(self.local_path()).is_file():
            os.remove(self.local_path())

    def file(self):
        return open(self.local_path(), 'rb')

    def extract(self, full_bucket_file_name):
        match = re.search(r'([\\.a-zA-Z_-]+)\\/(.*)', full_bucket_file_name)

        if match and len(match.groups()) == 2:
            return (match.groups()[0], match.groups()[1])
        else:
            raise RuntimeError(f"Cannot determine bucket and file name for {full_bucket_file_name}")
```

```python
import os
import boto3
import random
import binascii
from datetime import datetime


class PdfFileUploader:
    def __init__(self, file):
        self.file = file
        date = datetime.now().strftime('%Y%m%d%H%M%S')
        random_hex = binascii.b2a_hex(os.urandom(16)).decode('ascii')
        self.bucket_file_name = f"{date}_{random_hex}.pdf"

    def upload(self):
        self.file.seek(0)
        boto3.client('s3').upload_fileobj(self.file, self.bucket_name(), self.bucket_file_name)

        return (self.original_file_name(), self.full_bucket_file_name())

    def original_file_name(self):
        return os.path.basename(self.file.name)

    def bucket_name(self):
        return os.environ['AWS_BUCKET_NAME']

    def full_bucket_file_name(self):
        return f"{self.bucket_name()}/{self.bucket_file_name}"
```


I've tested the same problematic lines of code in the Django shell (without using `moto`) and it worked just fine which is the reason why I suspect `moto` is causing the problem.

Thanks!

Changed files:
moto/s3/models.py

diff --git a/moto/s3/models.py b/moto/s3/models.py
index 5a665e27e..8c2a86f41 100644
--- a/moto/s3/models.py
+++ b/moto/s3/models.py
@@ -12,6 +12,7 @@ import codecs
 import random
 import string
 import tempfile
+import threading
 import sys
 import time
 import uuid
@@ -110,6 +111,7 @@ class FakeKey(BaseModel):
         self._value_buffer = tempfile.SpooledTemporaryFile(max_size=max_buffer_size)
         self._max_buffer_size = max_buffer_size
         self.value = value
+        self.lock = threading.Lock()
 
     @property
     def version_id(self):
@@ -117,8 +119,12 @@ class FakeKey(BaseModel):
 
     @property
     def value(self):
+        self.lock.acquire()
         self._value_buffer.seek(0)
-        return self._value_buffer.read()
+        r = self._value_buffer.read()
+        r = copy.copy(r)
+        self.lock.release()
+        return r
 
     @value.setter
     def value(self, new_value):
@@ -130,6 +136,7 @@ class FakeKey(BaseModel):
         if isinstance(new_value, six.text_type):
             new_value = new_value.encode(DEFAULT_TEXT_ENCODING)
         self._value_buffer.write(new_value)
+        self.contentsize = len(new_value)
 
     def copy(self, new_name=None, new_is_versioned=None):
         r = copy.deepcopy(self)
@@ -157,6 +164,7 @@ class FakeKey(BaseModel):
         self.acl = acl
 
     def append_to_value(self, value):
+        self.contentsize += len(value)
         self._value_buffer.seek(0, os.SEEK_END)
         self._value_buffer.write(value)
 
@@ -229,8 +237,7 @@ class FakeKey(BaseModel):
 
     @property
     def size(self):
-        self._value_buffer.seek(0, os.SEEK_END)
-        return self._value_buffer.tell()
+        return self.contentsize
 
     @property
     def storage_class(self):
@@ -249,6 +256,7 @@ class FakeKey(BaseModel):
         state = self.__dict__.copy()
         state["value"] = self.value
         del state["_value_buffer"]
+        del state["lock"]
         return state
 
     def __setstate__(self, state):
@@ -258,6 +266,7 @@ class FakeKey(BaseModel):
             max_size=self._max_buffer_size
         )
         self.value = state["value"]
+        self.lock = threading.Lock()
 
 
 class FakeMultipart(BaseModel):
@@ -284,7 +293,7 @@ class FakeMultipart(BaseModel):
                 etag = etag.replace('"', "")
             if part is None or part_etag != etag:
                 raise InvalidPart()
-            if last is not None and len(last.value) < UPLOAD_PART_MIN_SIZE:
+            if last is not None and last.contentsize < UPLOAD_PART_MIN_SIZE:
                 raise EntityTooSmall()
             md5s.extend(decode_hex(part_etag)[0])
             total.extend(part.value)