[FIX] Attachment download button behavior (#15172)

RocketChat · Aug 13, 2019 · 6f30e70 · 6f30e70
1 parent 045826b
commit 6f30e70
Show file tree

Hide file tree

Showing 7 changed files with 3,023 additions and 3,008 deletions.
diff --git a/app/file-upload/server/config/AmazonS3.js b/app/file-upload/server/config/AmazonS3.js
@@ -8,23 +8,21 @@ import { FileUploadClass, FileUpload } from '../lib/FileUpload';
 import '../../ufs/AmazonS3/server.js';
 
 const get = function(file, req, res) {
-	const fileUrl = this.store.getRedirectURL(file);
+	const forceDownload = typeof req.query.download !== 'undefined';
+	const fileUrl = this.store.getRedirectURL(file, forceDownload);
 
-	if (fileUrl) {
-		const storeType = file.store.split(':').pop();
-		if (settings.get(`FileUpload_S3_Proxy_${ storeType }`)) {
-			const request = /^https:/.test(fileUrl) ? https : http;
-			request.get(fileUrl, (fileRes) => fileRes.pipe(res));
-		} else {
-			res.removeHeader('Content-Length');
-			res.removeHeader('Cache-Control');
-			res.setHeader('Location', fileUrl);
-			res.writeHead(302);
-			res.end();
-		}
-	} else {
-		res.end();
+	if (!fileUrl) {
+		return res.end();
+	}
+
+	const storeType = file.store.split(':').pop();
+	if (settings.get(`FileUpload_S3_Proxy_${ storeType }`)) {
+		const request = /^https:/.test(fileUrl) ? https : http;
+
+		return FileUpload.proxyFile(file.name, fileUrl, forceDownload, request, req, res);
 	}
+
+	return FileUpload.redirectToFile(fileUrl, req, res);
 };
 
 const copy = function(file, out) {

diff --git a/app/file-upload/server/config/GoogleStorage.js b/app/file-upload/server/config/GoogleStorage.js
@@ -8,31 +8,30 @@ import { settings } from '../../../settings';
 import '../../ufs/GoogleStorage/server.js';
 
 const get = function(file, req, res) {
-	this.store.getRedirectURL(file, (err, fileUrl) => {
+	const forceDownload = typeof req.query.download !== 'undefined';
+
+	this.store.getRedirectURL(file, forceDownload, (err, fileUrl) => {
 		if (err) {
-			console.error(err);
+			return console.error(err);
 		}
 
-		if (fileUrl) {
-			const storeType = file.store.split(':').pop();
-			if (settings.get(`FileUpload_GoogleStorage_Proxy_${ storeType }`)) {
-				const request = /^https:/.test(fileUrl) ? https : http;
-				request.get(fileUrl, (fileRes) => fileRes.pipe(res));
-			} else {
-				res.removeHeader('Content-Length');
-				res.removeHeader('Cache-Control');
-				res.setHeader('Location', fileUrl);
-				res.writeHead(302);
-				res.end();
-			}
-		} else {
-			res.end();
+		if (!fileUrl) {
+			return res.end();
 		}
+
+		const storeType = file.store.split(':').pop();
+		if (settings.get(`FileUpload_GoogleStorage_Proxy_${ storeType }`)) {
+			const request = /^https:/.test(fileUrl) ? https : http;
+
+			return FileUpload.proxyFile(file.name, fileUrl, forceDownload, request, req, res);
+		}
+
+		return FileUpload.redirectToFile(fileUrl, req, res);
 	});
 };
 
 const copy = function(file, out) {
-	this.store.getRedirectURL(file, (err, fileUrl) => {
+	this.store.getRedirectURL(file, false, (err, fileUrl) => {
 		if (err) {
 			console.error(err);
 		}

diff --git a/app/file-upload/server/lib/FileUpload.js b/app/file-upload/server/lib/FileUpload.js
@@ -375,6 +375,20 @@ export const FileUpload = {
 
 		return false;
 	},
+
+	redirectToFile(fileUrl, req, res) {
+		res.removeHeader('Content-Length');
+		res.removeHeader('Cache-Control');
+		res.setHeader('Location', fileUrl);
+		res.writeHead(302);
+		res.end();
+	},
+
+	proxyFile(fileName, fileUrl, forceDownload, request, req, res) {
+		res.setHeader('Content-Disposition', `${ forceDownload ? 'attachment' : 'inline' }; filename="${ encodeURI(fileName) }"`);
+
+		request.get(fileUrl, (fileRes) => fileRes.pipe(res));
+	},
 };
 
 export class FileUploadClass {

diff --git a/app/file-upload/ufs/AmazonS3/server.js b/app/file-upload/ufs/AmazonS3/server.js
@@ -47,10 +47,11 @@ export class AmazonS3Store extends UploadFS.Store {
 			}
 		};
 
-		this.getRedirectURL = function(file) {
+		this.getRedirectURL = function(file, forceDownload = false) {
 			const params = {
 				Key: this.getPath(file),
 				Expires: classOptions.URLExpiryTimeSpan,
+				ResponseContentDisposition: `${ forceDownload ? 'attachment' : 'inline' }; filename="${ encodeURI(file.name) }"`,
 			};
 
 			return s3.getSignedUrl('getObject', params);
@@ -140,7 +141,6 @@ export class AmazonS3Store extends UploadFS.Store {
 				Key: this.getPath(file),
 				Body: writeStream,
 				ContentType: file.type,
-				ContentDisposition: `inline; filename="${ encodeURI(file.name) }"`,
 
 			}, (error) => {
 				if (error) {

diff --git a/app/file-upload/ufs/GoogleStorage/server.js b/app/file-upload/ufs/GoogleStorage/server.js
@@ -30,10 +30,10 @@ export class GoogleStorageStore extends UploadFS.Store {
 			}
 		};
 
-		this.getRedirectURL = function(file, callback) {
+		this.getRedirectURL = function(file, forceDownload = false, callback) {
 			const params = {
 				action: 'read',
-				responseDisposition: 'inline',
+				responseDisposition: forceDownload ? 'attachment' : 'inline',
 				expires: Date.now() + this.options.URLExpiryTimeSpan * 1000,
 			};
 

diff --git a/app/message-attachments/client/messageAttachment.html b/app/message-attachments/client/messageAttachment.html
@@ -57,7 +57,7 @@
 					{{#if title_link}}
 						<a href="{{getURL title_link}}" target="_blank" rel="noopener noreferrer">{{title}}</a>
 						{{#if title_link_download}}
-							<a class="attachment-download-icon rc-tooltip" aria-label="{{_ 'Download'}}" href="{{getURL title_link}}" target="_blank" download="" rel="noopener noreferrer">{{> icon icon="download"}}</a>
+							<a class="attachment-download-icon rc-tooltip" aria-label="{{_ 'Download'}}" href="{{getURL title_link}}?download" target="_blank" download="{{title}}" rel="noopener noreferrer">{{> icon icon="download"}}</a>
 						{{/if}}
 					{{else}}
 						{{title}}