Avoid leaving partially downloaded redirected files

3 files changed, 25 insertions, 4 deletions

diff --git a/TODO b/TODO
index d71dd49..adc0593 100644
--- a/TODO
+++ b/TODO
@@ -10,7 +10,6 @@
 - move highlevel code from ad, eg, auth related
 - restrict sync content, especially for actiivity versions
 - after switching to global seqno, tweek blobs chaching model
-- do not keep partially created .pull files while processing pull sync requests on master
 
 1.0
 ===
diff --git a/sugar_network/node/router.py b/sugar_network/node/router.py
index f26509b..d4065f9 100644
--- a/sugar_network/node/router.py
+++ b/sugar_network/node/router.py
@@ -253,8 +253,12 @@ def _download_redirect(url):
     if not exists(cached_path):
         _logger.debug('Download %r redirect into %r', url, cached_path)
         with util.new_file(cached_path) as f:
-            response = http.request('GET', url, allow_redirects=True)
-            for chunk in response.iter_content(chunk_size=BUFFER_SIZE):
-                f.write(chunk)
+            try:
+                response = http.request('GET', url, allow_redirects=True)
+                for chunk in response.iter_content(chunk_size=BUFFER_SIZE):
+                    f.write(chunk)
+            except Exception:
+                os.unlink(f.name)
+                raise
 
     return file(cached_path, 'rb')
diff --git a/tests/units/router.py b/tests/units/router.py
index dcad14d..96dfe6c 100755
--- a/tests/units/router.py
+++ b/tests/units/router.py
@@ -256,6 +256,24 @@ class RouterTest(tests.Test):
         assert context == rest.get('/document2/%s/blob' % guid)
         assert 0 == os.stat(cached_path).st_mtime
 
+    def test_HandleRedirects_AvoidPArtiallyDownloadedFiles(self):
+        URL = 'http://foo.bar'
+
+        class Document2(Document):
+
+            @ad.active_property(ad.BlobProperty)
+            def blob(self, value):
+                raise ad.Redirect(URL)
+
+        self.fork(self.restful_server, [User, Document2])
+        rest = tests.Request('http://localhost:8800')
+
+        guid = rest.post('/document2', {'term': 'probe'})
+        rest.put('/document2/%s/blob' % guid, 'blob')
+
+        cached_path = 'tmp/' + hashlib.sha1(URL).hexdigest() + '.redirect'
+        self.assertRaises(RuntimeError, rest.get, '/document2/%s/blob' % guid)
+        assert not exists(cached_path)
 
     def test_Request_MultipleQueryArguments(self):
         request = _Request({