From 40ac59090bfba92d95ec55e03f983b3c6bd39514 Mon Sep 17 00:00:00 2001
From: Brian Gough <brian.gough@overleaf.com>
Date: Wed, 7 Nov 2018 16:22:09 +0000
Subject: [PATCH] make backends consistent by deleting temporary files

---
 .../app/coffee/FSPersistorManager.coffee      | 20 +++++++++++++++----
 .../app/coffee/S3PersistorManager.coffee      | 10 ++++++----
 2 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/services/filestore/app/coffee/FSPersistorManager.coffee b/services/filestore/app/coffee/FSPersistorManager.coffee
index 0868216a15..733202e4cd 100644
--- a/services/filestore/app/coffee/FSPersistorManager.coffee
+++ b/services/filestore/app/coffee/FSPersistorManager.coffee
@@ -13,10 +13,19 @@ module.exports =
   sendFile: ( location, target, source, callback = (err)->) ->
     filteredTarget = filterName target
     logger.log location:location, target:filteredTarget, source:source, "sending file"
-    fs.rename source, "#{location}/#{filteredTarget}", (err) ->
-      if err!=null
+    done = _.once (err) ->
+      if err?
         logger.err err:err, location:location, target:filteredTarget, source:source, "Error on put of file"
-      callback err
+      callback(err)
+    # actually copy the file (instead of moving it) to maintain consistent behaviour
+    # between the different implementations
+    sourceStream = fs.createReadStream source
+    sourceStream.on 'error', done
+    targetStream = fs.createWriteStream "#{location}/#{filteredTarget}"
+    targetStream.on 'error', done
+    targetStream.on 'finish', () ->
+      done()
+    sourceStream.pipe targetStream
 
   sendStream: ( location, target, sourceStream, callback = (err)->) ->
     logger.log location:location, target:target, "sending file stream"
@@ -26,7 +35,10 @@ module.exports =
       if err?
         logger.err  location:location, target:target, fsPath:fsPath, err:err, "something went wrong writing stream to disk"
         return callback err
-      @sendFile location, target, fsPath, callback
+      @sendFile location, target, fsPath, (err) -> 
+        # delete the temporary file created above and return the original error
+        LocalFileWriter.deleteFile fsPath, () ->
+          callback(err)
 
   # opts may be {start: Number, end: Number}
   getFileStream: (location, name, opts, _callback = (err, res)->) ->
diff --git a/services/filestore/app/coffee/S3PersistorManager.coffee b/services/filestore/app/coffee/S3PersistorManager.coffee
index 2bd6eb0e9b..3055810131 100644
--- a/services/filestore/app/coffee/S3PersistorManager.coffee
+++ b/services/filestore/app/coffee/S3PersistorManager.coffee
@@ -42,9 +42,8 @@ module.exports =
 			if res.statusCode != 200
 				logger.err bucketName:bucketName, key:key, fsPath:fsPath, "non 200 response from s3 putting file"
 				return callback("non 200 response from s3 on put file")
-			LocalFileWriter.deleteFile fsPath, (err)->
-				logger.log res:res,  bucketName:bucketName, key:key, fsPath:fsPath,"file uploaded to s3"
-				callback(err)
+			logger.log res:res,  bucketName:bucketName, key:key, fsPath:fsPath,"file uploaded to s3"
+			callback(err)
 		putEventEmiter.on "error", (err)->
 			logger.err err:err,  bucketName:bucketName, key:key, fsPath:fsPath, "error emmited on put of file"
 			callback err
@@ -57,7 +56,10 @@ module.exports =
 			if err?
 				logger.err  bucketName:bucketName, key:key, fsPath:fsPath, err:err, "something went wrong writing stream to disk"
 				return callback(err)
-			@sendFile bucketName, key, fsPath, callback
+			@sendFile bucketName, key, fsPath, (err) ->
+				# delete the temporary file created above and return the original error
+				LocalFileWriter.deleteFile fsPath, () ->
+					callback(err)
 
 	# opts may be {start: Number, end: Number}
 	getFileStream: (bucketName, key, opts, callback = (err, res)->)->