From 5045a4f1681b8c4a152210a9245e59af30744422 Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Fri, 25 Mar 2016 18:35:55 +0300 Subject: [PATCH 1/2] Fix handling of meta=None in S3FilesStore.persist_file --- scrapy/pipelines/files.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scrapy/pipelines/files.py b/scrapy/pipelines/files.py index 1fd2571e2..c5dbd307b 100644 --- a/scrapy/pipelines/files.py +++ b/scrapy/pipelines/files.py @@ -147,7 +147,7 @@ class S3FilesStore(object): Bucket=self.bucket, Key=key_name, Body=buf, - Metadata={k: str(v) for k, v in six.iteritems(meta)}, + Metadata={k: str(v) for k, v in six.iteritems(meta or {})}, ACL=self.POLICY, **extra) else: From fc8cd45a48fb21d33e7f6edaec2aa8275c6ea942 Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Sun, 27 Mar 2016 21:29:21 +0200 Subject: [PATCH 2/2] Fix a race condition in the FilesPipeline Checksum calculation could happen simultaniously with persisting the file in the store (which is done in a thread): they operated on the same buf object. Concretely this lead to a bug with S3FilesStore when using botocore: the signature did not match because the position in the buf was already at the end. The fix is to move checksum calculation before passing buf to the store. --- scrapy/pipelines/files.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scrapy/pipelines/files.py b/scrapy/pipelines/files.py index c5dbd307b..b1b8404bb 100644 --- a/scrapy/pipelines/files.py +++ b/scrapy/pipelines/files.py @@ -364,8 +364,9 @@ class FilesPipeline(MediaPipeline): def file_downloaded(self, response, request, info): path = self.file_path(request, response=response, info=info) buf = BytesIO(response.body) - self.store.persist_file(path, buf, info) checksum = md5sum(buf) + buf.seek(0) + self.store.persist_file(path, buf, info) return checksum def item_completed(self, results, item, info):