From cf12ec1154567877a63cf0edc8eb62719e7ff138 Mon Sep 17 00:00:00 2001
From: Brian Gough <brian.gough@overleaf.com>
Date: Fri, 7 Dec 2018 17:02:34 +0000
Subject: [PATCH] use the aws sdk to copy files in S3PersistorManager

to work around problems with knox
---
 .../app/coffee/S3PersistorManager.coffee        | 17 +++++++++++------
 .../unit/coffee/S3PersistorManagerTests.coffee  | 10 +++++++---
 2 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/services/filestore/app/coffee/S3PersistorManager.coffee b/services/filestore/app/coffee/S3PersistorManager.coffee
index d6ab47c200..8debdaad4f 100644
--- a/services/filestore/app/coffee/S3PersistorManager.coffee
+++ b/services/filestore/app/coffee/S3PersistorManager.coffee
@@ -11,6 +11,7 @@ path = require("path")
 LocalFileWriter = require("./LocalFileWriter")
 Errors = require("./Errors")
 _ = require("underscore")
+awsS3 = require "aws-sdk/clients/s3"
 
 thirtySeconds = 30 * 1000
 
@@ -25,6 +26,12 @@ buildDefaultOptions = (bucketName, method, key)->
 			uri:"https://#{bucketName}.s3.amazonaws.com/#{key}"
 	}
 
+s3 = new awsS3({
+	credentials:
+		accessKeyId: settings.filestore.s3.key,
+		secretAccessKey: settings.filestore.s3.secret
+})
+
 module.exports =
 
 	sendFile: (bucketName, key, fsPath, callback)->
@@ -90,12 +97,10 @@ module.exports =
 			callback err
 
 	copyFile: (bucketName, sourceKey, destKey, callback)->
-		logger.log bucketName:bucketName, sourceKey:sourceKey, destKey:destKey, "copying file in s3"
-		s3Client = knox.createClient
-			key: settings.filestore.s3.key
-			secret: settings.filestore.s3.secret
-			bucket: bucketName
-		s3Client.copyFile sourceKey, destKey, (err)->
+		logger.log bucketName:bucketName, sourceKey:sourceKey, destKey: destKey, "copying file in s3"
+		source = bucketName + '/' + sourceKey
+		# use the AWS SDK instead of knox due to problems with error handling (https://github.com/Automattic/knox/issues/114)
+		s3.copyObject {Bucket: bucketName, Key: destKey, CopySource: source}, (err) ->
 			if err?
 				logger.err err:err, bucketName:bucketName, sourceKey:sourceKey, destKey:destKey, "something went wrong copying file in aws"
 			callback(err)
diff --git a/services/filestore/test/unit/coffee/S3PersistorManagerTests.coffee b/services/filestore/test/unit/coffee/S3PersistorManagerTests.coffee
index b48fde7820..0860514180 100644
--- a/services/filestore/test/unit/coffee/S3PersistorManagerTests.coffee
+++ b/services/filestore/test/unit/coffee/S3PersistorManagerTests.coffee
@@ -25,11 +25,15 @@ describe "S3PersistorManagerTests", ->
 			get: sinon.stub()
 		@knox =
 			createClient: sinon.stub().returns(@stubbedKnoxClient)
+		@stubbedS3Client =
+			copyObject:sinon.stub()
+		@awsS3 = sinon.stub().returns @stubbedS3Client
 		@LocalFileWriter =
 			writeStream: sinon.stub()
 			deleteFile: sinon.stub()
 		@requires =
 			"knox": @knox
+			"aws-sdk/clients/s3": @awsS3
 			"settings-sharelatex": @settings
 			"./LocalFileWriter":@LocalFileWriter
 			"logger-sharelatex":
@@ -207,11 +211,11 @@ describe "S3PersistorManagerTests", ->
 			@destKey = "my/dest/key"
 			@S3PersistorManager = SandboxedModule.require modulePath, requires: @requires
 
-		it "should use knox to copy file", (done)->
-			@stubbedKnoxClient.copyFile.callsArgWith(2, @error)
+		it "should use AWS SDK to copy file", (done)->
+			@stubbedS3Client.copyObject.callsArgWith(1, @error)
 			@S3PersistorManager.copyFile @bucketName, @sourceKey, @destKey, (err)=>
 				err.should.equal @error
-				@stubbedKnoxClient.copyFile.calledWith(@sourceKey, @destKey).should.equal true
+				@stubbedS3Client.copyObject.calledWith({Bucket: @bucketName, Key: @destKey, CopySource: @bucketName + '/' + @key}).should.equal true
 				done()
 
 	describe "deleteDirectory", ->