[8.6] [Reporting] Fix download of files with multi-byte filenames (#153158) (#153506)

# Backport

This will backport the following commits from `main` to `8.6`:
- [[Reporting] Fix download of files with multi-byte filenames
(#153158)](https://github.com/elastic/kibana/pull/153158)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Tim
Sullivan","email":"tsullivan@users.noreply.github.com"},"sourceCommit":{"committedDate":"2023-03-22T23:03:52Z","message":"[Reporting]
Fix download of files with multi-byte filenames (#153158)\n\n##
Summary\r\nCloses
https://github.com/elastic/kibana/issues/152963\r\n\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n\r\n\r\n###
Release note\r\nFixed an issue where it was unable to create a report
based on a\r\ndashboard with multi-byte characters in the
title.","sha":"6eb5d8d401a443aec032c1fa1e1035926b5e2ef1","branchLabelMapping":{"^v8.8.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Feature:Reporting","Team:SharedUX","v8.7.0","v8.8.0","v8.6.3"],"number":153158,"url":"https://github.com/elastic/kibana/pull/153158","mergeCommit":{"message":"[Reporting]
Fix download of files with multi-byte filenames (#153158)\n\n##
Summary\r\nCloses
https://github.com/elastic/kibana/issues/152963\r\n\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n\r\n\r\n###
Release note\r\nFixed an issue where it was unable to create a report
based on a\r\ndashboard with multi-byte characters in the
title.","sha":"6eb5d8d401a443aec032c1fa1e1035926b5e2ef1"}},"sourceBranch":"main","suggestedTargetBranches":["8.7","8.6"],"targetPullRequestStates":[{"branch":"8.7","label":"v8.7.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.8.0","labelRegex":"^v8.8.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/153158","number":153158,"mergeCommit":{"message":"[Reporting]
Fix download of files with multi-byte filenames (#153158)\n\n##
Summary\r\nCloses
https://github.com/elastic/kibana/issues/152963\r\n\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n\r\n\r\n###
Release note\r\nFixed an issue where it was unable to create a report
based on a\r\ndashboard with multi-byte characters in the
title.","sha":"6eb5d8d401a443aec032c1fa1e1035926b5e2ef1"}},{"branch":"8.6","label":"v8.6.3","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->
This commit is contained in:
Tim Sullivan 2023-03-22 17:17:45 -07:00 committed by GitHub
parent e86206b7f4
commit 412d37a317
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 64 additions and 35 deletions

View file

@ -57,8 +57,8 @@ describe('getDocumentPayload', () => {
expect.objectContaining({
contentType: 'application/pdf',
content: expect.any(Readable),
filename: 'Some PDF report.pdf',
headers: expect.objectContaining({
'Content-Disposition': 'attachment; filename="Some PDF report.pdf"',
'Content-Length': '1024',
}),
statusCode: 200,
@ -85,8 +85,8 @@ describe('getDocumentPayload', () => {
expect.objectContaining({
contentType: 'text/csv',
content: expect.any(Readable),
filename: 'Some CSV report.csv',
headers: expect.objectContaining({
'Content-Disposition': 'attachment; filename="Some CSV report.csv"',
'Content-Length': '1024',
'kbn-csv-contains-formulas': true,
'kbn-max-size-reached': true,

View file

@ -24,8 +24,11 @@ interface Payload {
content: string | Stream | ErrorFromPayload;
contentType: string | null;
headers: ResponseHeaders;
filename?: string;
}
export type PayloadCompleted = Payload & { filename: string };
type TaskRunResult = Required<ReportApiJSON>['output'];
const DEFAULT_TITLE = 'report';
@ -67,12 +70,12 @@ export function getDocumentPayloadFactory(reporting: ReportingCore) {
const contentType = output.content_type ?? 'text/plain';
return {
filename,
content,
statusCode: 200,
contentType,
headers: {
...headers,
'Content-Disposition': `attachment; filename="${filename}"`,
'Content-Length': `${output.size ?? ''}`,
},
};

View file

@ -42,21 +42,26 @@ export async function downloadJobResponseHandler(
}
const payload = await getDocumentPayload(doc);
const { contentType, content, filename, statusCode } = payload;
if (!payload.contentType || !ALLOWED_JOB_CONTENT_TYPES.includes(payload.contentType)) {
if (!contentType || !ALLOWED_JOB_CONTENT_TYPES.includes(contentType)) {
return res.badRequest({
body: `Unsupported content-type of ${payload.contentType} specified by job output`,
body: `Unsupported content-type of ${contentType} specified by job output`,
});
}
return res.custom({
body: typeof payload.content === 'string' ? Buffer.from(payload.content) : payload.content,
statusCode: payload.statusCode,
headers: {
...payload.headers,
'content-type': payload.contentType,
},
});
const body = typeof content === 'string' ? Buffer.from(content) : content;
const headers = {
...payload.headers,
'content-type': contentType,
};
if (filename) {
return res.file({ body, headers, filename });
}
return res.custom({ body, headers, statusCode });
} catch (err) {
const { logger } = reporting.getPluginSetupDeps();
logger.error(err);

View file

@ -8,13 +8,14 @@
jest.mock('../../../lib/content_stream', () => ({
getContentStream: jest.fn(),
}));
import type { ElasticsearchClientMock } from '@kbn/core/server/mocks';
import { BehaviorSubject } from 'rxjs';
import { estypes } from '@elastic/elasticsearch';
import { setupServer } from '@kbn/core-test-helpers-test-utils';
import type { ElasticsearchClientMock } from '@kbn/core/server/mocks';
import { licensingMock } from '@kbn/licensing-plugin/server/mocks';
import { BehaviorSubject } from 'rxjs';
import { Readable } from 'stream';
import supertest from 'supertest';
import { ReportingCore } from '../../..';
import { licensingMock } from '@kbn/licensing-plugin/server/mocks';
import { ReportingInternalSetup, ReportingInternalStart } from '../../../core';
import { ContentStream, ExportTypesRegistry, getContentStream } from '../../../lib';
import {
@ -44,7 +45,7 @@ describe('GET /api/reporting/jobs/download', () => {
hits: {
hits: sources.map((source: object) => ({ _source: source })),
},
};
} as estypes.SearchResponseBody;
};
const mockConfigSchema = createMockConfigSchema({ roles: { enabled: false } });
@ -113,7 +114,7 @@ describe('GET /api/reporting/jobs/download', () => {
});
it('fails on malformed download IDs', async () => {
mockEsClient.search.mockResponseOnce(getHits() as any);
mockEsClient.search.mockResponseOnce(getHits());
registerJobInfoRoutes(core);
await server.start();
@ -153,7 +154,7 @@ describe('GET /api/reporting/jobs/download', () => {
});
it('returns 404 if job not found', async () => {
mockEsClient.search.mockResponseOnce(getHits() as any);
mockEsClient.search.mockResponseOnce(getHits());
registerJobInfoRoutes(core);
await server.start();
@ -166,7 +167,7 @@ describe('GET /api/reporting/jobs/download', () => {
getHits({
jobtype: 'invalidJobType',
payload: { title: 'invalid!' },
}) as any
})
);
registerJobInfoRoutes(core);
@ -180,7 +181,7 @@ describe('GET /api/reporting/jobs/download', () => {
getHits({
jobtype: 'base64EncodedJobType',
payload: {}, // payload is irrelevant
}) as any
})
);
registerJobInfoRoutes(core);
@ -195,7 +196,7 @@ describe('GET /api/reporting/jobs/download', () => {
getHits({
jobtype: 'customForbiddenJobType',
payload: {}, // payload is irrelevant
}) as any
})
);
registerJobInfoRoutes(core);
@ -211,7 +212,7 @@ describe('GET /api/reporting/jobs/download', () => {
jobtype: 'unencodedJobType',
status: 'pending',
payload: { title: 'incomplete!' },
}) as any
})
);
registerJobInfoRoutes(core);
@ -231,7 +232,7 @@ describe('GET /api/reporting/jobs/download', () => {
status: 'failed',
output: { content: 'job failure message' },
payload: { title: 'failing job!' },
}) as any
})
);
registerJobInfoRoutes(core);
@ -256,23 +257,23 @@ describe('GET /api/reporting/jobs/download', () => {
status: 'completed',
output: { content_type: outputContentType },
payload: { title },
});
}) as estypes.SearchResponseBody;
};
it('when a known job-type is complete', async () => {
mockEsClient.search.mockResponseOnce(getCompleteHits() as any);
mockEsClient.search.mockResponseOnce(getCompleteHits());
registerJobInfoRoutes(core);
await server.start();
await supertest(httpSetup.server.listener)
.get('/api/reporting/jobs/download/dank')
.expect(200)
.expect('Content-Type', 'text/plain; charset=utf-8')
.expect('content-disposition', 'attachment; filename="report.csv"');
.expect('Content-Type', 'text/csv; charset=utf-8')
.expect('content-disposition', 'attachment; filename=report.csv');
});
it('succeeds when security is not there or disabled', async () => {
mockEsClient.search.mockResponseOnce(getCompleteHits() as any);
mockEsClient.search.mockResponseOnce(getCompleteHits());
// @ts-ignore
core.pluginSetupDeps.security = null;
@ -284,15 +285,15 @@ describe('GET /api/reporting/jobs/download', () => {
await supertest(httpSetup.server.listener)
.get('/api/reporting/jobs/download/dope')
.expect(200)
.expect('Content-Type', 'text/plain; charset=utf-8')
.expect('content-disposition', 'attachment; filename="report.csv"');
.expect('Content-Type', 'text/csv; charset=utf-8')
.expect('content-disposition', 'attachment; filename=report.csv');
});
it('forwards job content stream', async () => {
mockEsClient.search.mockResponseOnce(
getCompleteHits({
jobType: 'unencodedJobType',
}) as any
})
);
registerJobInfoRoutes(core);
@ -300,7 +301,7 @@ describe('GET /api/reporting/jobs/download', () => {
await supertest(httpSetup.server.listener)
.get('/api/reporting/jobs/download/dank')
.expect(200)
.expect('Content-Type', 'text/plain; charset=utf-8')
.expect('Content-Type', 'text/csv; charset=utf-8')
.then(({ text }) => expect(text).toEqual('test'));
});
@ -309,7 +310,7 @@ describe('GET /api/reporting/jobs/download', () => {
getCompleteHits({
jobType: 'unencodedJobType',
outputContentType: 'application/html',
}) as any
})
);
registerJobInfoRoutes(core);
@ -325,6 +326,26 @@ describe('GET /api/reporting/jobs/download', () => {
});
});
});
it('allows multi-byte characters in file names', async () => {
mockEsClient.search.mockResponseOnce(
getCompleteHits({
jobType: 'base64EncodedJobType',
title: '日本語ダッシュボード',
})
);
registerJobInfoRoutes(core);
await server.start();
await supertest(httpSetup.server.listener)
.get('/api/reporting/jobs/download/japanese-dashboard')
.expect(200)
.expect('Content-Type', 'application/pdf')
.expect(
'content-disposition',
'attachment; filename=%E6%97%A5%E6%9C%AC%E8%AA%9E%E3%83%80%E3%83%83%E3%82%B7%E3%83%A5%E3%83%9C%E3%83%BC%E3%83%89.pdf'
);
});
});
describe('Deprecated: role-based access control', () => {

View file

@ -64,7 +64,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
expect(res.status).to.equal(200);
expect(res.get('content-type')).to.equal('application/pdf');
expect(res.get('content-disposition')).to.equal(
'inline; filename="The Very Cool Workpad for PDF Tests.pdf"'
'attachment; filename=The%20Very%20Cool%20Workpad%20for%20PDF%20Tests.pdf'
);
/* Check the value of the PDF data that was generated