From c8ff569985f7aa16d37b48fd60ba8b2342ef8f9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Sch=C3=BCtte?= Date: Mon, 21 Feb 2022 17:58:37 +0100 Subject: [PATCH 1/2] feat(common): extend streamable-file header support Provide option to specify additional headers when using StreamableFile. No need to access the native response option. BREAKING CHANGE: not specifying content-disposition header and StreamableFile option disposition would send header value 'null' instead of not sending the header at all. This changes to not sending the header if no value is specified. This commit closes issue #9229. --- integration/send-files/e2e/express.spec.ts | 11 +++++-- integration/send-files/e2e/fastify.spec.ts | 22 +++++++++++-- integration/send-files/src/app.service.ts | 4 +++ .../common/file-stream/streamable-file.ts | 17 ++++++++-- .../streamable-options.interface.ts | 3 ++ .../test/file-stream/streamable-file.spec.ts | 32 +++++++++++++++++++ .../adapters/express-adapter.ts | 28 ++++++++++++++-- .../adapters/fastify-adapter.ts | 28 ++++++++++++++-- 8 files changed, 134 insertions(+), 11 deletions(-) diff --git a/integration/send-files/e2e/express.spec.ts b/integration/send-files/e2e/express.spec.ts index 425ce669587..1ca4602b624 100644 --- a/integration/send-files/e2e/express.spec.ts +++ b/integration/send-files/e2e/express.spec.ts @@ -9,7 +9,8 @@ import { join } from 'path'; import * as request from 'supertest'; import { AppModule } from '../src/app.module'; -const readmeString = readFileSync(join(process.cwd(), 'Readme.md')).toString(); +const readme = readFileSync(join(process.cwd(), 'Readme.md')); +const readmeString = readme.toString(); describe('Express FileSend', () => { let app: NestExpressApplication; @@ -53,12 +54,18 @@ describe('Express FileSend', () => { expect(res.body.toString()).to.be.eq(readmeString); }); }); - it('should return a file with correct content type and disposition', async () => { + it('should return a file with correct headers', async () => { return request(app.getHttpServer()) .get('/file/with/headers') .expect(200) .expect('Content-Type', 'text/markdown') .expect('Content-Disposition', 'attachment; filename="Readme.md"') + .expect('Content-Length', readme.byteLength.toString()) + .expect('Accept-Ranges', 'bytes') + .expect( + 'Content-Range', + `bytes 0-${readme.byteLength - 1}/${readme.byteLength}`, + ) .expect(res => { expect(res.text).to.be.eq(readmeString); }); diff --git a/integration/send-files/e2e/fastify.spec.ts b/integration/send-files/e2e/fastify.spec.ts index 9e7ee5b5e92..e16bfac2758 100644 --- a/integration/send-files/e2e/fastify.spec.ts +++ b/integration/send-files/e2e/fastify.spec.ts @@ -4,11 +4,12 @@ import { } from '@nestjs/platform-fastify'; import { Test } from '@nestjs/testing'; import { expect } from 'chai'; -import { readFileSync } from 'fs'; +import { read, readFileSync } from 'fs'; import { join } from 'path'; import { AppModule } from '../src/app.module'; -const readmeString = readFileSync(join(process.cwd(), 'Readme.md')).toString(); +const readme = readFileSync(join(process.cwd(), 'Readme.md')); +const readmeString = readme.toString(); describe('Fastify FileSend', () => { let app: NestFastifyApplication; @@ -67,4 +68,21 @@ describe('Fastify FileSend', () => { expect(payload.toString()).to.be.eq(readmeString); }); }); + it('should return a file with correct headers', async () => { + return app + .inject({ url: '/file/with/headers', method: 'get' }) + .then(({ statusCode, headers, payload }) => { + expect(statusCode).to.equal(200); + expect(headers['content-type']).to.equal('text/markdown'); + expect(headers['content-disposition']).to.equal( + 'attachment; filename="Readme.md"', + ); + expect(headers['content-length']).to.equal(readme.byteLength); + expect(headers['accept-ranges']).to.equal('bytes'); + expect(headers['content-range']).to.equal( + `bytes 0-${readme.byteLength - 1}/${readme.byteLength}`, + ); + expect(payload).to.equal(readmeString); + }); + }); }); diff --git a/integration/send-files/src/app.service.ts b/integration/send-files/src/app.service.ts index a83e6a4f63d..e7bcb7735f8 100644 --- a/integration/send-files/src/app.service.ts +++ b/integration/send-files/src/app.service.ts @@ -25,11 +25,15 @@ export class AppService { } getFileWithHeaders(): StreamableFile { + const file = readFileSync(join(process.cwd(), 'Readme.md')); return new StreamableFile( createReadStream(join(process.cwd(), 'Readme.md')), { type: 'text/markdown', disposition: 'attachment; filename="Readme.md"', + length: file.byteLength, + acceptRanges: 'bytes', + range: `bytes 0-${file.byteLength - 1}/${file.byteLength}`, }, ); } diff --git a/packages/common/file-stream/streamable-file.ts b/packages/common/file-stream/streamable-file.ts index b0b0ddab08b..e6d19bacc99 100644 --- a/packages/common/file-stream/streamable-file.ts +++ b/packages/common/file-stream/streamable-file.ts @@ -26,8 +26,19 @@ export class StreamableFile { } getHeaders() { - const { type = 'application/octet-stream', disposition = null } = - this.options; - return { type, disposition }; + const { + type = 'application/octet-stream', + disposition = undefined, + acceptRanges = undefined, + length = undefined, + range = undefined, + } = this.options; + return { + type, + disposition, + acceptRanges, + length, + range, + }; } } diff --git a/packages/common/file-stream/streamable-options.interface.ts b/packages/common/file-stream/streamable-options.interface.ts index 8b9bd24a721..2bc25d7a0ec 100644 --- a/packages/common/file-stream/streamable-options.interface.ts +++ b/packages/common/file-stream/streamable-options.interface.ts @@ -1,4 +1,7 @@ export interface StreamableFileOptions { type?: string; disposition?: string; + length?: number; + range?: string; + acceptRanges?: string; } diff --git a/packages/common/test/file-stream/streamable-file.spec.ts b/packages/common/test/file-stream/streamable-file.spec.ts index c4cb075b5fb..eadf4e5afa1 100644 --- a/packages/common/test/file-stream/streamable-file.spec.ts +++ b/packages/common/test/file-stream/streamable-file.spec.ts @@ -17,6 +17,38 @@ describe('StreamableFile', () => { expect(streamableFile.getStream()).to.equal(stream); }); }); + describe('when options is empty', () => { + it('should return application/octet-stream for type and undefined for others', () => { + const stream = new Readable(); + const streamableFile = new StreamableFile(stream); + expect(streamableFile.getHeaders()).to.deep.equal({ + type: 'application/octet-stream', + disposition: undefined, + acceptRanges: undefined, + length: undefined, + range: undefined, + }); + }); + }); + describe('when options is defined', () => { + it('should pass provided headers', () => { + const stream = new Readable(); + const streamableFile = new StreamableFile(stream, { + type: 'application/pdf', + acceptRanges: '123', + disposition: 'inline', + length: 100, + range: '456', + }); + expect(streamableFile.getHeaders()).to.deep.equal({ + type: 'application/pdf', + acceptRanges: '123', + disposition: 'inline', + length: 100, + range: '456', + }); + }); + }); describe('otherwise', () => { describe('when input is a Buffer instance', () => { it('should create a readable stream and push the input buffer', () => { diff --git a/packages/platform-express/adapters/express-adapter.ts b/packages/platform-express/adapters/express-adapter.ts index afe1b782e0e..1a89d497005 100644 --- a/packages/platform-express/adapters/express-adapter.ts +++ b/packages/platform-express/adapters/express-adapter.ts @@ -48,12 +48,36 @@ export class ExpressAdapter extends AbstractHttpAdapter { } if (body instanceof StreamableFile) { const streamHeaders = body.getHeaders(); - if (response.getHeader('Content-Type') === undefined) { + if ( + response.getHeader('Content-Type') === undefined && + streamHeaders.type + ) { response.setHeader('Content-Type', streamHeaders.type); } - if (response.getHeader('Content-Disposition') === undefined) { + if ( + response.getHeader('Content-Disposition') === undefined && + streamHeaders.disposition + ) { response.setHeader('Content-Disposition', streamHeaders.disposition); } + if ( + response.getHeader('Content-Length') === undefined && + streamHeaders.length + ) { + response.setHeader('Content-Length', streamHeaders.length); + } + if ( + response.getHeader('Content-Range') === undefined && + streamHeaders.range + ) { + response.setHeader('Content-Range', streamHeaders.range); + } + if ( + response.getHeader('Accept-Ranges') === undefined && + streamHeaders.acceptRanges + ) { + response.setHeader('Accept-Ranges', streamHeaders.acceptRanges); + } return body.getStream().pipe(response); } return isObject(body) ? response.json(body) : response.send(String(body)); diff --git a/packages/platform-fastify/adapters/fastify-adapter.ts b/packages/platform-fastify/adapters/fastify-adapter.ts index b2867926d80..4509542abb8 100644 --- a/packages/platform-fastify/adapters/fastify-adapter.ts +++ b/packages/platform-fastify/adapters/fastify-adapter.ts @@ -282,12 +282,36 @@ export class FastifyAdapter< } if (body instanceof StreamableFile) { const streamHeaders = body.getHeaders(); - if (fastifyReply.getHeader('Content-Type') === undefined) { + if ( + fastifyReply.getHeader('Content-Type') === undefined && + streamHeaders.type + ) { fastifyReply.header('Content-Type', streamHeaders.type); } - if (fastifyReply.getHeader('Content-Disposition') === undefined) { + if ( + fastifyReply.getHeader('Content-Disposition') === undefined && + streamHeaders.disposition + ) { fastifyReply.header('Content-Disposition', streamHeaders.disposition); } + if ( + fastifyReply.getHeader('Content-Length') === undefined && + streamHeaders.length + ) { + fastifyReply.header('Content-Length', streamHeaders.length); + } + if ( + fastifyReply.getHeader('Content-Range') === undefined && + streamHeaders.range + ) { + fastifyReply.header('Content-Range', streamHeaders.range); + } + if ( + fastifyReply.getHeader('Accept-Ranges') === undefined && + streamHeaders.acceptRanges + ) { + fastifyReply.header('Accept-Ranges', streamHeaders.acceptRanges); + } body = body.getStream(); } return fastifyReply.send(body); From 9ca2a9a1725681933f7c97c03d24f19f7dc4ae4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Sch=C3=BCtte?= Date: Wed, 23 Feb 2022 11:13:59 +0100 Subject: [PATCH 2/2] feat(common): add length header to streamable-file Provide option to specify content-length headers when using StreamableFile. No need to access the native response object. BREAKING CHANGE: not specifying content-disposition header and StreamableFile option disposition would send header value 'null' instead of not sending the header at all. This changes to not sending the header if no value is specified. This commit closes issue #9229. --- integration/send-files/e2e/express.spec.ts | 5 ----- integration/send-files/e2e/fastify.spec.ts | 6 +----- integration/send-files/src/app.service.ts | 2 -- packages/common/file-stream/streamable-file.ts | 4 ---- .../streamable-options.interface.ts | 2 -- .../test/file-stream/streamable-file.spec.ts | 6 ------ .../adapters/express-adapter.ts | 18 +++--------------- .../adapters/fastify-adapter.ts | 18 +++--------------- 8 files changed, 7 insertions(+), 54 deletions(-) diff --git a/integration/send-files/e2e/express.spec.ts b/integration/send-files/e2e/express.spec.ts index 1ca4602b624..1657d137c39 100644 --- a/integration/send-files/e2e/express.spec.ts +++ b/integration/send-files/e2e/express.spec.ts @@ -61,11 +61,6 @@ describe('Express FileSend', () => { .expect('Content-Type', 'text/markdown') .expect('Content-Disposition', 'attachment; filename="Readme.md"') .expect('Content-Length', readme.byteLength.toString()) - .expect('Accept-Ranges', 'bytes') - .expect( - 'Content-Range', - `bytes 0-${readme.byteLength - 1}/${readme.byteLength}`, - ) .expect(res => { expect(res.text).to.be.eq(readmeString); }); diff --git a/integration/send-files/e2e/fastify.spec.ts b/integration/send-files/e2e/fastify.spec.ts index e16bfac2758..8b821ba7ce4 100644 --- a/integration/send-files/e2e/fastify.spec.ts +++ b/integration/send-files/e2e/fastify.spec.ts @@ -4,7 +4,7 @@ import { } from '@nestjs/platform-fastify'; import { Test } from '@nestjs/testing'; import { expect } from 'chai'; -import { read, readFileSync } from 'fs'; +import { readFileSync } from 'fs'; import { join } from 'path'; import { AppModule } from '../src/app.module'; @@ -78,10 +78,6 @@ describe('Fastify FileSend', () => { 'attachment; filename="Readme.md"', ); expect(headers['content-length']).to.equal(readme.byteLength); - expect(headers['accept-ranges']).to.equal('bytes'); - expect(headers['content-range']).to.equal( - `bytes 0-${readme.byteLength - 1}/${readme.byteLength}`, - ); expect(payload).to.equal(readmeString); }); }); diff --git a/integration/send-files/src/app.service.ts b/integration/send-files/src/app.service.ts index e7bcb7735f8..ba5849a754f 100644 --- a/integration/send-files/src/app.service.ts +++ b/integration/send-files/src/app.service.ts @@ -32,8 +32,6 @@ export class AppService { type: 'text/markdown', disposition: 'attachment; filename="Readme.md"', length: file.byteLength, - acceptRanges: 'bytes', - range: `bytes 0-${file.byteLength - 1}/${file.byteLength}`, }, ); } diff --git a/packages/common/file-stream/streamable-file.ts b/packages/common/file-stream/streamable-file.ts index e6d19bacc99..b392a532372 100644 --- a/packages/common/file-stream/streamable-file.ts +++ b/packages/common/file-stream/streamable-file.ts @@ -29,16 +29,12 @@ export class StreamableFile { const { type = 'application/octet-stream', disposition = undefined, - acceptRanges = undefined, length = undefined, - range = undefined, } = this.options; return { type, disposition, - acceptRanges, length, - range, }; } } diff --git a/packages/common/file-stream/streamable-options.interface.ts b/packages/common/file-stream/streamable-options.interface.ts index 2bc25d7a0ec..5d167b6b048 100644 --- a/packages/common/file-stream/streamable-options.interface.ts +++ b/packages/common/file-stream/streamable-options.interface.ts @@ -2,6 +2,4 @@ export interface StreamableFileOptions { type?: string; disposition?: string; length?: number; - range?: string; - acceptRanges?: string; } diff --git a/packages/common/test/file-stream/streamable-file.spec.ts b/packages/common/test/file-stream/streamable-file.spec.ts index eadf4e5afa1..7bfd433012b 100644 --- a/packages/common/test/file-stream/streamable-file.spec.ts +++ b/packages/common/test/file-stream/streamable-file.spec.ts @@ -24,9 +24,7 @@ describe('StreamableFile', () => { expect(streamableFile.getHeaders()).to.deep.equal({ type: 'application/octet-stream', disposition: undefined, - acceptRanges: undefined, length: undefined, - range: undefined, }); }); }); @@ -35,17 +33,13 @@ describe('StreamableFile', () => { const stream = new Readable(); const streamableFile = new StreamableFile(stream, { type: 'application/pdf', - acceptRanges: '123', disposition: 'inline', length: 100, - range: '456', }); expect(streamableFile.getHeaders()).to.deep.equal({ type: 'application/pdf', - acceptRanges: '123', disposition: 'inline', length: 100, - range: '456', }); }); }); diff --git a/packages/platform-express/adapters/express-adapter.ts b/packages/platform-express/adapters/express-adapter.ts index 1a89d497005..0afa505dd4b 100644 --- a/packages/platform-express/adapters/express-adapter.ts +++ b/packages/platform-express/adapters/express-adapter.ts @@ -50,34 +50,22 @@ export class ExpressAdapter extends AbstractHttpAdapter { const streamHeaders = body.getHeaders(); if ( response.getHeader('Content-Type') === undefined && - streamHeaders.type + streamHeaders.type !== undefined ) { response.setHeader('Content-Type', streamHeaders.type); } if ( response.getHeader('Content-Disposition') === undefined && - streamHeaders.disposition + streamHeaders.disposition !== undefined ) { response.setHeader('Content-Disposition', streamHeaders.disposition); } if ( response.getHeader('Content-Length') === undefined && - streamHeaders.length + streamHeaders.length !== undefined ) { response.setHeader('Content-Length', streamHeaders.length); } - if ( - response.getHeader('Content-Range') === undefined && - streamHeaders.range - ) { - response.setHeader('Content-Range', streamHeaders.range); - } - if ( - response.getHeader('Accept-Ranges') === undefined && - streamHeaders.acceptRanges - ) { - response.setHeader('Accept-Ranges', streamHeaders.acceptRanges); - } return body.getStream().pipe(response); } return isObject(body) ? response.json(body) : response.send(String(body)); diff --git a/packages/platform-fastify/adapters/fastify-adapter.ts b/packages/platform-fastify/adapters/fastify-adapter.ts index 4509542abb8..d1121c5e645 100644 --- a/packages/platform-fastify/adapters/fastify-adapter.ts +++ b/packages/platform-fastify/adapters/fastify-adapter.ts @@ -284,34 +284,22 @@ export class FastifyAdapter< const streamHeaders = body.getHeaders(); if ( fastifyReply.getHeader('Content-Type') === undefined && - streamHeaders.type + streamHeaders.type !== undefined ) { fastifyReply.header('Content-Type', streamHeaders.type); } if ( fastifyReply.getHeader('Content-Disposition') === undefined && - streamHeaders.disposition + streamHeaders.disposition !== undefined ) { fastifyReply.header('Content-Disposition', streamHeaders.disposition); } if ( fastifyReply.getHeader('Content-Length') === undefined && - streamHeaders.length + streamHeaders.length !== undefined ) { fastifyReply.header('Content-Length', streamHeaders.length); } - if ( - fastifyReply.getHeader('Content-Range') === undefined && - streamHeaders.range - ) { - fastifyReply.header('Content-Range', streamHeaders.range); - } - if ( - fastifyReply.getHeader('Accept-Ranges') === undefined && - streamHeaders.acceptRanges - ) { - fastifyReply.header('Accept-Ranges', streamHeaders.acceptRanges); - } body = body.getStream(); } return fastifyReply.send(body);