Skip to content

Commit

Permalink
Reports: Set content-disposition to attachment for report download (#…
Browse files Browse the repository at this point in the history
…144136)

* remove content-disposition

* Fix content-disposition and content-type

* fix tests

Co-authored-by: Jean-Louis Leysens <[email protected]>
  • Loading branch information
tsullivan and jloleysens authored Oct 31, 2022
1 parent 2654c30 commit ebb9dbd
Show file tree
Hide file tree
Showing 6 changed files with 17 additions and 18 deletions.
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -472,7 +472,6 @@
"commander": "^4.1.1",
"compare-versions": "3.5.1",
"constate": "^3.3.2",
"content-disposition": "^0.5.4",
"copy-to-clipboard": "^3.0.8",
"core-js": "^3.25.5",
"cronstrue": "^1.51.0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ describe('getDocumentPayload', () => {
contentType: 'application/pdf',
content: expect.any(Readable),
headers: expect.objectContaining({
'Content-Disposition': 'inline; filename="Some PDF report.pdf"',
'Content-Length': 1024,
'Content-Disposition': 'attachment; filename="Some PDF report.pdf"',
'Content-Length': '1024',
}),
statusCode: 200,
})
Expand All @@ -86,8 +86,8 @@ describe('getDocumentPayload', () => {
contentType: 'text/csv',
content: expect.any(Readable),
headers: expect.objectContaining({
'Content-Disposition': 'inline; filename="Some CSV report.csv"',
'Content-Length': 1024,
'Content-Disposition': 'attachment; filename="Some CSV report.csv"',
'Content-Length': '1024',
'kbn-csv-contains-formulas': true,
'kbn-max-size-reached': true,
}),
Expand Down Expand Up @@ -137,7 +137,7 @@ describe('getDocumentPayload', () => {
contentType: 'text/plain',
content: 'pending',
headers: {
'retry-after': 30,
'retry-after': '30',
},
statusCode: 503,
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,11 @@
* 2.0.
*/

import { ResponseHeaders } from '@kbn/core-http-server';
import { Stream } from 'stream';
// @ts-ignore
import contentDisposition from 'content-disposition';
import { ReportingCore } from '../..';
import { CSV_JOB_TYPE, CSV_JOB_TYPE_DEPRECATED } from '../../../common/constants';
import { ReportApiJSON } from '../../../common/types';
import { ReportingCore } from '../..';
import { getContentStream, statuses } from '../../lib';
import { ExportTypeDefinition } from '../../types';
import { jobsQueryFactory } from './jobs_query';
Expand All @@ -24,7 +23,7 @@ interface Payload {
statusCode: number;
content: string | Stream | ErrorFromPayload;
contentType: string | null;
headers: Record<string, string | number>;
headers: ResponseHeaders;
}

type TaskRunResult = Required<ReportApiJSON>['output'];
Expand Down Expand Up @@ -65,15 +64,16 @@ export function getDocumentPayloadFactory(reporting: ReportingCore) {
const content = await getContentStream(reporting, { id, index }, { encoding });
const filename = getTitle(exportType, title);
const headers = getReportingHeaders(output, exportType);
const contentType = output.content_type ?? 'text/plain';

return {
content,
statusCode: 200,
contentType: output.content_type,
contentType,
headers: {
...headers,
'Content-Disposition': contentDisposition(filename, { type: 'inline' }),
'Content-Length': output.size,
'Content-Disposition': `attachment; filename="${filename}"`,
'Content-Length': `${output.size ?? ''}`,
},
};
}
Expand All @@ -99,7 +99,7 @@ export function getDocumentPayloadFactory(reporting: ReportingCore) {
statusCode: 503,
content: status,
contentType: 'text/plain',
headers: { 'retry-after': 30 },
headers: { 'retry-after': '30' },
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ export async function downloadJobResponseHandler(
statusCode: payload.statusCode,
headers: {
...payload.headers,
'content-type': payload.contentType || '',
'content-type': payload.contentType,
},
});
} catch (err) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ describe('GET /api/reporting/jobs/download', () => {
.get('/api/reporting/jobs/download/dank')
.expect(200)
.expect('Content-Type', 'text/plain; charset=utf-8')
.expect('content-disposition', 'inline; filename="report.csv"');
.expect('content-disposition', 'attachment; filename="report.csv"');
});

it('succeeds when security is not there or disabled', async () => {
Expand All @@ -285,7 +285,7 @@ describe('GET /api/reporting/jobs/download', () => {
.get('/api/reporting/jobs/download/dope')
.expect(200)
.expect('Content-Type', 'text/plain; charset=utf-8')
.expect('content-disposition', 'inline; filename="report.csv"');
.expect('content-disposition', 'attachment; filename="report.csv"');
});

it('forwards job content stream', async () => {
Expand Down
2 changes: 1 addition & 1 deletion yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -10807,7 +10807,7 @@ container-info@^1.0.1:
resolved "https://registry.yarnpkg.com/container-info/-/container-info-1.0.1.tgz#6b383cb5e197c8d921e88983388facb04124b56b"
integrity sha512-wk/+uJvPHOFG+JSwQS+fw6H6yw3Oyc8Kw9L4O2MN817uA90OqJ59nlZbbLPqDudsjJ7Tetee3pwExdKpd2ahjQ==

[email protected], content-disposition@^0.5.4:
[email protected]:
version "0.5.4"
resolved "https://registry.yarnpkg.com/content-disposition/-/content-disposition-0.5.4.tgz#8b82b4efac82512a02bb0b1dcec9d2c5e8eb5bfe"
integrity sha512-FveZTNuGw04cxlAiWbzi6zTAL/lhehaWbTtgluJh4/E95DqMwTmha3KZN1aAWA8cFIhHzMZUvLevkw5Rqk+tSQ==
Expand Down

0 comments on commit ebb9dbd

Please sign in to comment.