[FIX] Download my data with file uploads (#19862)

.mocharc.js

@@ -11,6 +11,7 @@ module.exports = {
 	spec: [
 		'app/**/*.tests.js',
 		'app/**/*.tests.ts',
+		'server/**/*.tests.ts',
 		'client/**/*.spec.ts',
 	],
 };

app/user-data-download/server/cronProcessDownloads.js

@@ -1,6 +1,5 @@
 import fs from 'fs';
 import util from 'util';
-import path from 'path';
 
 import _ from 'underscore';
 import { Meteor } from 'meteor/meteor';
@@ -14,6 +13,7 @@ import { Subscriptions, Rooms, Users, Uploads, Messages, UserDataFiles, ExportOp
 import { FileUpload } from '../../file-upload/server';
 import * as Mailer from '../../mailer';
 import { readSecondaryPreferred } from '../../../server/database/readSecondaryPreferred';
+import { joinPath } from '../../../server/lib/fileUtils';
 
 const fsStat = util.promisify(fs.stat);
 const fsOpen = util.promisify(fs.open);
@@ -182,7 +182,7 @@ export const copyFile = function(attachmentData, assetsPath) {
 	if (!file) {
 		return;
 	}
-	FileUpload.copy(file, path.join(assetsPath, `${ attachmentData._id }-${ attachmentData.name }`));
+	FileUpload.copy(file, joinPath(assetsPath, `${ attachmentData._id }-${ attachmentData.name }`));
 };
 
 const exportMessageObject = (type, messageObject, messageFile) => {
@@ -330,7 +330,7 @@ const generateChannelsFile = function(type, exportPath, exportOperation) {
 		return;
 	}
 
-	const fileName = path.join(exportPath, 'channels.json');
+	const fileName = joinPath(exportPath, 'channels.json');
 	startFile(fileName,
 		exportOperation.roomList.map((roomData) =>
 			JSON.stringify({
@@ -351,7 +351,7 @@ export const exportRoomMessagesToFile = async function(exportPath, assetsPath, e
 
 	const limit = settings.get('UserData_MessageLimitPerRequest') > 0 ? settings.get('UserData_MessageLimitPerRequest') : 1000;
 	for (const exportOpRoomData of roomList) {
-		const filePath = path.join(exportPath, exportOpRoomData.targetFile);
+		const filePath = joinPath(exportPath, exportOpRoomData.targetFile);
 		if (exportOpRoomData.status === 'pending') {
 			exportOpRoomData.status = 'exporting';
 			startFile(filePath, exportType === 'html' ? '<meta http-equiv="content-type" content="text/html; charset=utf-8">' : '');
@@ -397,7 +397,7 @@ const generateUserFile = function(exportOperation, userData) {
 		services: Object.keys(services),
 	};
 
-	const fileName = path.join(exportOperation.exportPath, exportOperation.fullExport ? 'user.json' : 'user.html');
+	const fileName = joinPath(exportOperation.exportPath, exportOperation.fullExport ? 'user.json' : 'user.html');
 	startFile(fileName, '');
 
 	if (exportOperation.fullExport) {
@@ -440,7 +440,7 @@ const generateUserAvatarFile = function(exportOperation, userData) {
 		return;
 	}
 
-	const filePath = path.join(exportOperation.exportPath, 'avatar');
+	const filePath = joinPath(exportOperation.exportPath, 'avatar');
 	if (FileUpload.copy(file, filePath)) {
 		exportOperation.generatedAvatar = true;
 	}
@@ -504,7 +504,7 @@ const continueExportOperation = async function(exportOperation) {
 				copyFile(attachmentData, exportOperation.assetsPath);
 			});
 
-			const targetFile = path.join(zipFolder, `${ exportOperation.userId }.zip`);
+			const targetFile = joinPath(zipFolder, `${ exportOperation.userId }.zip`);
 			if (await fsExists(targetFile)) {
 				await fsUnlink(targetFile);
 			}
@@ -515,7 +515,7 @@ const continueExportOperation = async function(exportOperation) {
 		if (exportOperation.status === 'compressing') {
 			createDir(zipFolder);
 
-			exportOperation.generatedFile = path.join(zipFolder, `${ exportOperation.userId }.zip`);
+			exportOperation.generatedFile = joinPath(zipFolder, `${ exportOperation.userId }.zip`);
 			if (!await fsExists(exportOperation.generatedFile)) {
 				await makeZipFile(exportOperation.exportPath, exportOperation.generatedFile);
 			}

package-lock.json

@@ -17680,6 +17680,21 @@
 			"resolved": "https://registry.npmjs.org/file-uri-to-path/-/file-uri-to-path-1.0.0.tgz",
 			"integrity": "sha512-0Zt+s3L7Vf1biwWZ29aARiVYLx7iMGnEUl9x33fbB/j3jR81u/O2LbqK+Bm1CDSNDKVtJ/YjwY7TUd5SkeLQLw=="
 		},
+		"filename-reserved-regex": {
+			"version": "2.0.0",
+			"resolved": "https://registry.npmjs.org/filename-reserved-regex/-/filename-reserved-regex-2.0.0.tgz",
+			"integrity": "sha1-q/c9+rc10EVECr/qLZHzieu/oik="
+		},
+		"filenamify": {
+			"version": "4.2.0",
+			"resolved": "https://registry.npmjs.org/filenamify/-/filenamify-4.2.0.tgz",
+			"integrity": "sha512-pkgE+4p7N1n7QieOopmn3TqJaefjdWXwEkj2XLZJLKfOgcQKkn11ahvGNgTD8mLggexLiDFQxeTs14xVU22XPA==",
+			"requires": {
+				"filename-reserved-regex": "^2.0.0",
+				"strip-outer": "^1.0.1",
+				"trim-repeated": "^1.0.0"
+			}
+		},
 		"filesize": {
 			"version": "3.6.1",
 			"resolved": "https://registry.npmjs.org/filesize/-/filesize-3.6.1.tgz",
@@ -31856,6 +31871,14 @@
 			"resolved": "https://registry.npmjs.org/strip-json-comments/-/strip-json-comments-2.0.1.tgz",
 			"integrity": "sha1-PFMZQukIwml8DsNEhYwobHygpgo="
 		},
+		"strip-outer": {
+			"version": "1.0.1",
+			"resolved": "https://registry.npmjs.org/strip-outer/-/strip-outer-1.0.1.tgz",
+			"integrity": "sha512-k55yxKHwaXnpYGsOzg4Vl8+tDrWylxDEpknGjhTiZB8dFRU5rTo9CAzeycivxV3s+zlTKwrs6WxMxR95n26kwg==",
+			"requires": {
+				"escape-string-regexp": "^1.0.2"
+			}
+		},
 		"stubs": {
 			"version": "3.0.0",
 			"resolved": "https://registry.npmjs.org/stubs/-/stubs-3.0.0.tgz",
@@ -33215,6 +33238,14 @@
 			"integrity": "sha512-C4+gOpvmxaSMKuEf9Qc134F1ZuOHVXKRbtEflf4NTtuuJDEIJ9p5PXsalL8SkeRw+qit1Mo+yuvMPAKwWg/1hA==",
 			"dev": true
 		},
+		"trim-repeated": {
+			"version": "1.0.0",
+			"resolved": "https://registry.npmjs.org/trim-repeated/-/trim-repeated-1.0.0.tgz",
+			"integrity": "sha1-42RqLqTokTEr9+rObPsFOAvAHCE=",
+			"requires": {
+				"escape-string-regexp": "^1.0.2"
+			}
+		},
 		"trim-right": {
 			"version": "1.0.1",
 			"resolved": "https://registry.npmjs.org/trim-right/-/trim-right-1.0.1.tgz",

package.json

@@ -189,6 +189,7 @@
 		"express-rate-limit": "^5.1.3",
 		"fibers": "4.0.3",
 		"file-type": "^10.11.0",
+		"filenamify": "^4.2.0",
 		"filesize": "^3.6.1",
 		"googleapis": "^25.0.0",
 		"grapheme-splitter": "^1.0.4",

server/lib/fileUtils.tests.ts

@@ -0,0 +1,28 @@
+/* eslint-env mocha */
+import { expect } from 'chai';
+
+import { fileName, joinPath } from './fileUtils';
+
+describe('File utils', () => {
+	it('should return a valid file name', () => {
+		expect(fileName('something')).to.equal('something');
+		expect(fileName('some@thing')).to.equal('some@thing');
+		expect(fileName('/something')).to.equal('something');
+		expect(fileName('/some/thing')).to.equal('some-thing');
+		expect(fileName('/some/thing/')).to.equal('some-thing');
+		expect(fileName('///some///thing///')).to.equal('some-thing');
+		expect(fileName('some/thing')).to.equal('some-thing');
+		expect(fileName('some:"thing"')).to.equal('some-thing');
+		expect(fileName('some:"thing".txt')).to.equal('some-thing-.txt');
+		expect(fileName('some"thing"')).to.equal('some-thing');
+		expect(fileName('some\u0000thing')).to.equal('some-thing');
+	});
+	it('should return a valid joined path', () => {
+		expect(joinPath('/app', 'some@thing')).to.equal('/app/some@thing');
+		expect(joinPath('../app', 'something')).to.equal('../app/something');
+		expect(joinPath('../app/', 'something')).to.equal('../app/something');
+		expect(joinPath('../app/', '/something')).to.equal('../app/something');
+		expect(joinPath('/app', '/something')).to.equal('/app/something');
+		expect(joinPath('/app', '/../some/thing')).to.equal('/app/..-some-thing');
+	});
+});

server/lib/fileUtils.ts

@@ -0,0 +1,11 @@
+import path from 'path';
+
+import filenamify from 'filenamify';
+
+export function fileName(name: string): string {
+	return filenamify(name, { replacement: '-' });
+}
+
+export function joinPath(base: string, name: string): string {
+	return path.join(base, fileName(name));
+}