Skip to content

Commit

Permalink
Merge pull request #3184 from huridocs/3092-original-filename-on-down…
Browse files Browse the repository at this point in the history
…load

3092 Original filename at downloading files
  • Loading branch information
konzz authored Sep 24, 2020
2 parents cdcee03 + 1cfca6d commit 684f9be
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 4 deletions.
8 changes: 7 additions & 1 deletion app/api/files/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,14 +68,20 @@ export default (app: Application) => {

async (req, res, next) => {
try {
const [file = { filename: '' }] = await files.get({ filename: req.params.filename });
const [file = { filename: '', originalname: undefined }] = await files.get({
filename: req.params.filename,
});

const filename = file.filename || '';

if (!filename || !(await fileExists(uploadsPath(filename)))) {
throw createError('file not found', 404);
}

if (file.originalname) {
res.setHeader('Content-Disposition', `filename=${file.originalname}`);
}

res.sendFile(uploadsPath(filename));
} catch (e) {
next(e);
Expand Down
4 changes: 2 additions & 2 deletions app/api/files/specs/downloadRoute.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,10 @@ describe('files routes download', () => {
afterAll(async () => db.disconnect());

describe('GET/', () => {
it('should send the file the file', async () => {
it('should send the file', async () => {
const response: SuperTestResponse = await request(app).get(`/api/files/${fileName1}`);

expect(response.header['content-disposition']).not.toBeDefined();
expect(response.header['content-disposition']).toBe('filename=upload1');
expect(response.body instanceof Buffer).toBe(true);
});

Expand Down
12 changes: 11 additions & 1 deletion app/api/files/specs/routes.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { setUpApp } from 'api/utils/testingRoutes';
import connections from 'api/relationships';

import { FileType } from 'shared/types/fileType';
import { fixtures, uploadId, uploadId2 } from './fixtures';
import { fileName1, fixtures, uploadId, uploadId2 } from './fixtures';
import { files } from '../files';
import uploadRoutes from '../routes';

Expand Down Expand Up @@ -62,6 +62,16 @@ describe('files routes', () => {
'upload2',
]);
});

it('should set the original filename as content-disposition header', async () => {
const response: SuperTestResponse = await request(app).get(`/api/files/${fileName1}`);
expect(response.get('Content-Disposition')).toBe('filename=upload1');
});

it('should not set content-disposition header when the file does not have an original name', async () => {
const response: SuperTestResponse = await request(app).get('/api/files/fileNotInDisk');
expect(response.get('Content-Disposition')).toBeUndefined();
});
});

describe('DELETE/api/files', () => {
Expand Down

0 comments on commit 684f9be

Please sign in to comment.