Server: Resolves #5286: Set better filename and mime type for files downloaded via published notes

packages/lib/path-utils.ts

@@ -58,20 +58,27 @@ export function safeFilename(e: string, maxLength: number = null, allowSpaces: b
 	return output.substr(0, maxLength);
 }
 
-let friendlySafeFilename_blackListChars = '/<>:\'"\\|?*#';
+let friendlySafeFilename_blackListChars = '/\n\r<>:\'"\\|?*#';
 for (let i = 0; i < 32; i++) {
 	friendlySafeFilename_blackListChars += String.fromCharCode(i);
 }
 
 const friendlySafeFilename_blackListNames = ['.', '..', 'CON', 'PRN', 'AUX', 'NUL', 'COM1', 'COM2', 'COM3', 'COM4', 'COM5', 'COM6', 'COM7', 'COM8', 'COM9', 'LPT1', 'LPT2', 'LPT3', 'LPT4', 'LPT5', 'LPT6', 'LPT7', 'LPT8', 'LPT9'];
 
-export function friendlySafeFilename(e: string, maxLength: number = null) {
+export function friendlySafeFilename(e: string, maxLength: number = null, preserveExtension: boolean = false) {
 	// Although Windows supports paths up to 255 characters, but that includes the filename and its
 	// parent directory path. Also there's generally no good reason for dir or file names
 	// to be so long, so keep it at 50, which should prevent various errors.
 	if (maxLength === null) maxLength = 50;
 	if (!e || !e.replace) return _('Untitled');
 
+	let fileExt = '';
+
+	if (preserveExtension) {
+		fileExt = `.${safeFileExtension(fileExtension(e))}`;
+		e = filename(e);
+	}
+
 	let output = '';
 	for (let i = 0; i < e.length; i++) {
 		const c = e[i];
@@ -106,9 +113,9 @@ export function friendlySafeFilename(e: string, maxLength: number = null) {
 		}
 	}
 
-	if (!output) return _('Untitled');
+	if (!output) return _('Untitled') + fileExt;
 
-	return output.substr(0, maxLength);
+	return output.substr(0, maxLength) + fileExt;
 }
 
 export function toFileProtocolPath(filePathEncode: string, os: string = null) {

packages/lib/pathUtils.test.js

@@ -15,6 +15,7 @@ describe('pathUtils', function() {
 			['no space at the end ', 'no space at the end'],
 			['nor dots...', 'nor dots'],
 			['  no space before either', 'no space before either'],
+			['no\nnewline\n\rplease', 'no_newline__please'],
 			['thatsreallylongthatsreallylongthatsreallylongthatsreallylongthatsreallylongthatsreallylongthatsreallylongthatsreallylongthatsreallylongthatsreallylongthatsreallylongthatsreallylongthatsreallylongthatsreallylongthatsreallylongthatsreallylongthatsreallylongthatsreallylong', 'thatsreallylongthatsreallylongthatsreallylongthats'],
 		];
 
@@ -25,6 +26,11 @@ describe('pathUtils', function() {
 
 		expect(!!friendlySafeFilename('')).toBe(true);
 		expect(!!friendlySafeFilename('...')).toBe(true);
+
+		// Check that it optionally handles filenames with extension
+		expect(friendlySafeFilename('  testing.md', null, true)).toBe('testing.md');
+		expect(friendlySafeFilename('testing.safe??ext##', null, true)).toBe('testing.safeext');
+		expect(friendlySafeFilename('thatsreallylongthatsreallylongthatsreallylongthatsreallylongthatsreallylongthatsreallylongthatsreallylongthatsreallylongthatsreallylongthatsreallylongthatsreallylongthatsreallylongthatsreallylongthatsreallylongthatsreallylongthatsreallylongthatsreallylongthatsreallylong.md', null, true)).toBe('thatsreallylongthatsreallylongthatsreallylongthats.md');
 	}));
 
 	it('should quote and unquote paths', (async () => {

packages/server/src/routes/index/shares.ts

@@ -6,6 +6,7 @@ import { ErrorForbidden, ErrorNotFound } from '../../utils/errors';
 import { Item, Share } from '../../db';
 import { ModelType } from '@joplin/lib/BaseModel';
 import { FileViewerResponse, renderItem as renderJoplinItem } from '../../utils/joplinUtils';
+import { friendlySafeFilename } from '@joplin/lib/path-utils';
 
 async function renderItem(context: AppContext, item: Item, share: Share): Promise<FileViewerResponse> {
 	if (item.jop_type === ModelType.Note) {
@@ -16,6 +17,7 @@ async function renderItem(context: AppContext, item: Item, share: Share): Promis
 		body: item.content,
 		mime: item.mime_type,
 		size: item.content_size,
+		filename: '',
 	};
 }
 
@@ -44,6 +46,7 @@ router.get('shares/:id', async (path: SubPath, ctx: AppContext) => {
 	ctx.response.body = result.body;
 	ctx.response.set('Content-Type', result.mime);
 	ctx.response.set('Content-Length', result.size.toString());
+	if (result.filename) ctx.response.set('Content-disposition', `attachment; filename="${friendlySafeFilename(result.filename)}"`);
 	return new Response(ResponseType.KoaResponse, ctx.response);
 }, RouteType.UserContent);
 

packages/server/src/utils/joplinUtils.ts

@@ -24,13 +24,15 @@ import { themeStyle } from '@joplin/lib/theme';
 import Setting from '@joplin/lib/models/Setting';
 import { Models } from '../models/factory';
 import MustacheService from '../services/MustacheService';
+import Logger from '@joplin/lib/Logger';
 
-// const logger = Logger.create('JoplinUtils');
+const logger = Logger.create('JoplinUtils');
 
 export interface FileViewerResponse {
 	body: any;
 	mime: string;
 	size: number;
+	filename: string;
 }
 
 interface ResourceInfo {
@@ -153,11 +155,25 @@ async function noteLinkedItemInfos(userId: Uuid, itemModel: ItemModel, note: Not
 	return output;
 }
 
-async function renderResource(item: Item, content: any): Promise<FileViewerResponse> {
+async function renderResource(userId: string, resourceId: string, item: Item, content: any): Promise<FileViewerResponse> {
+	// The item passed to this function is the resource blob, which is
+	// sufficient to download the resource. However, if we want a more user
+	// friendly download, we need to know the resource original name and mime
+	// type. So below, we try to get that information.
+	let jopItem: any = null;
+
+	try {
+		const resourceItem = await models_.item().loadByJopId(userId, resourceId);
+		jopItem = await models_.item().loadAsJoplinItem(resourceItem.id);
+	} catch (error) {
+		logger.error(`Could not load Joplin item ${resourceId} associated with item: ${item.id}`);
+	}
+
 	return {
 		body: content,
-		mime: item.mime_type,
+		mime: jopItem ? jopItem.mime : item.mime_type,
 		size: item.content_size,
+		filename: jopItem ? jopItem.title : '',
 	};
 }
 
@@ -219,6 +235,7 @@ async function renderNote(share: Share, note: NoteEntity, resourceInfos: Resourc
 		body: bodyHtml,
 		mime: 'text/html',
 		size: Buffer.byteLength(bodyHtml, 'utf-8'),
+		filename: '',
 	};
 }
 
@@ -234,28 +251,34 @@ export async function renderItem(userId: Uuid, item: Item, share: Share, query:
 	const linkedItemInfos: LinkedItemInfos = await noteLinkedItemInfos(userId, models_.item(), rootNote);
 	const resourceInfos = await getResourceInfos(linkedItemInfos);
 
-	const fileToRender = {
+	interface FileToRender {
+		item: Item;
+		content: any;
+		jopItemId: string;
+	}
+
+	const fileToRender: FileToRender = {
 		item: item,
 		content: null as any,
-		itemId: rootNote.id,
+		jopItemId: rootNote.id,
 	};
 
 	if (query.resource_id) {
 		const resourceItem = await models_.item().loadByName(userId, resourceBlobPath(query.resource_id), { fields: ['*'] });
 		fileToRender.item = resourceItem;
 		fileToRender.content = resourceItem.content;
-		fileToRender.itemId = query.resource_id;
+		fileToRender.jopItemId = query.resource_id;
 	}
 
-	if (fileToRender.item !== item && !linkedItemInfos[fileToRender.itemId]) {
+	if (fileToRender.item !== item && !linkedItemInfos[fileToRender.jopItemId]) {
-		throw new ErrorNotFound(`Item "${fileToRender.itemId}" does not belong to this note`);
+		throw new ErrorNotFound(`Item "${fileToRender.jopItemId}" does not belong to this note`);
 	}
 
-	const itemToRender = fileToRender.item === item ? rootNote : linkedItemInfos[fileToRender.itemId].item;
+	const itemToRender = fileToRender.item === item ? rootNote : linkedItemInfos[fileToRender.jopItemId].item;
 	const itemType: ModelType = itemToRender.type_;
 
 	if (itemType === ModelType.Resource) {
-		return renderResource(fileToRender.item, fileToRender.content);
+		return renderResource(userId, fileToRender.jopItemId, fileToRender.item, fileToRender.content);
 	} else if (itemType === ModelType.Note) {
 		return renderNote(share, itemToRender, resourceInfos, linkedItemInfos);
 	} else {