From 5d151df8c59329a470b8ffa6e3547aae72a7e55b Mon Sep 17 00:00:00 2001 From: Arda TANRIKULU Date: Mon, 26 Sep 2022 06:41:31 +0300 Subject: [PATCH] Fix FormData and add tests (#140) * Fix FormData and add tests * Better test grouping * Go * Changesets * Prettier * .. * Go * .. * Try --- .changeset/rotten-poets-teach.md | 6 ++ package.json | 2 +- packages/fetch/dist/create-node-ponyfill.js | 16 ----- packages/fetch/dist/getFormDataMethod.js | 4 +- .../fetch/tests/getFormDataMethod.spec.ts | 43 +++++++++++++ packages/server/src/utils.ts | 2 +- packages/server/test/formdata.spec.ts | 64 +++++++++++++++++++ 7 files changed, 117 insertions(+), 20 deletions(-) create mode 100644 .changeset/rotten-poets-teach.md create mode 100644 packages/fetch/tests/getFormDataMethod.spec.ts create mode 100644 packages/server/test/formdata.spec.ts diff --git a/.changeset/rotten-poets-teach.md b/.changeset/rotten-poets-teach.md new file mode 100644 index 0000000000..e3c74e2959 --- /dev/null +++ b/.changeset/rotten-poets-teach.md @@ -0,0 +1,6 @@ +--- +'@whatwg-node/fetch': patch +'@whatwg-node/server': patch +--- + +Fix Request.formData method diff --git a/package.json b/package.json index e823bcf6ae..55486244f2 100644 --- a/package.json +++ b/package.json @@ -47,7 +47,7 @@ "typescript": "4.8.3" }, "lint-staged": { - "packages/**/src/**/*.{ts,tsx}": [ + "packages/**/*.{ts,tsx}": [ "eslint --fix" ], "**/*.{ts,tsx,graphql,yml,md,mdx}": [ diff --git a/packages/fetch/dist/create-node-ponyfill.js b/packages/fetch/dist/create-node-ponyfill.js index 860e4a1ec3..ee6b815a7f 100644 --- a/packages/fetch/dist/create-node-ponyfill.js +++ b/packages/fetch/dist/create-node-ponyfill.js @@ -165,22 +165,6 @@ module.exports = function createNodePonyfill(opts = {}) { constructor(requestOrUrl, options) { if (typeof requestOrUrl === "string") { options = options || {}; - if (options.body != null && options.body.read && options.body.on) { - const readable = options.body; - options.body = new ponyfills.ReadableStream({ - pull(controller) { - const chunk = readable.read(); - if (chunk != null) { - controller.enqueue(chunk); - } else { - controller.close(); - } - }, - close(e) { - readable.destroy(e); - } - }) - } super(requestOrUrl, options); const contentType = this.headers.get("content-type"); if (contentType && contentType.startsWith("multipart/form-data")) { diff --git a/packages/fetch/dist/getFormDataMethod.js b/packages/fetch/dist/getFormDataMethod.js index 0255e37c43..63c54cbb78 100644 --- a/packages/fetch/dist/getFormDataMethod.js +++ b/packages/fetch/dist/getFormDataMethod.js @@ -16,13 +16,13 @@ module.exports = function getFormDataMethod(File, limits) { reject(new Error(`File size limit exceeded: ${limits.fileSize} bytes`)); }) fileStream.on('data', (chunk) => { - chunks.push(chunk); + chunks.push(...chunk); }) fileStream.on('close', () => { if (fileStream.truncated) { reject(new Error(`File size limit exceeded: ${limits.fileSize} bytes`)); } - const file = new File(chunks, filename, { type: mimeType }); + const file = new File([new Uint8Array(chunks)], filename, { type: mimeType }); formData.set(name, file); resolve(file); }); diff --git a/packages/fetch/tests/getFormDataMethod.spec.ts b/packages/fetch/tests/getFormDataMethod.spec.ts new file mode 100644 index 0000000000..2e291aec5c --- /dev/null +++ b/packages/fetch/tests/getFormDataMethod.spec.ts @@ -0,0 +1,43 @@ +import { createFetch } from '@whatwg-node/fetch'; + +describe('getFormDataMethod', () => { + ['fieldsFirst:true', 'fieldsFirst:false'].forEach(fieldsFirstFlag => { + const fetchAPI = createFetch({ + formDataLimits: { + fieldsFirst: fieldsFirstFlag === 'fieldsFirst:true', + }, + }); + describe(fieldsFirstFlag, () => { + it('should parse fields correctly', async () => { + const formData = new fetchAPI.FormData(); + formData.append('greetings', 'Hello world!'); + formData.append('bye', 'Goodbye world!'); + const request = new fetchAPI.Request('http://localhost:8080', { + method: 'POST', + body: formData, + }); + const formdata = await request.formData(); + expect(formdata.get('greetings')).toBe('Hello world!'); + expect(formdata.get('bye')).toBe('Goodbye world!'); + }); + it('should parse and receive text files correctly', async () => { + const formData = new fetchAPI.FormData(); + const greetingsFile = new fetchAPI.File(['Hello world!'], 'greetings.txt', { type: 'text/plain' }); + const byeFile = new fetchAPI.File(['Goodbye world!'], 'bye.txt', { type: 'text/plain' }); + formData.append('greetings', greetingsFile); + formData.append('bye', byeFile); + const request = new fetchAPI.Request('http://localhost:8080', { + method: 'POST', + body: formData, + }); + const formdata = await request.formData(); + const receivedGreetingsFile = formdata.get('greetings') as File; + const receivedGreetingsText = await receivedGreetingsFile.text(); + expect(receivedGreetingsText).toBe('Hello world!'); + const receivedByeFile = formdata.get('bye') as File; + const receivedByeText = await receivedByeFile.text(); + expect(receivedByeText).toBe('Goodbye world!'); + }); + }); + }); +}); diff --git a/packages/server/src/utils.ts b/packages/server/src/utils.ts index 1540bb3895..b1f8bb08a1 100644 --- a/packages/server/src/utils.ts +++ b/packages/server/src/utils.ts @@ -173,8 +173,8 @@ export async function sendNodeResponse( serverResponse.end(body, resolve); } else if (isReadable(body)) { serverResponse.once('close', () => { - body.destroy(); resolve(); + body.destroy(); }); body.pipe(serverResponse); } else if (isAsyncIterable(body)) { diff --git a/packages/server/test/formdata.spec.ts b/packages/server/test/formdata.spec.ts new file mode 100644 index 0000000000..b070ddc256 --- /dev/null +++ b/packages/server/test/formdata.spec.ts @@ -0,0 +1,64 @@ +import { createFetch } from '@whatwg-node/fetch'; +import { createServerAdapter } from '@whatwg-node/server'; +import { createServer, Server } from 'http'; + +describe('FormData', () => { + let server: Server; + let url: string; + afterEach(done => { + server.close(done); + }); + ['fieldsFirst:true', 'fieldsFirst:false'].forEach(fieldsFirstFlag => { + const fetchAPI = createFetch({ + formDataLimits: { + fieldsFirst: fieldsFirstFlag === 'fieldsFirst:true', + }, + }); + + describe(fieldsFirstFlag, () => { + it('should forward formdata correctly', async () => { + let receivedFieldContent: string | undefined; + let receivedFileName: string | undefined; + let receivedFileType: string | undefined; + let receivedFileContent: string | undefined; + const adapter = createServerAdapter(async request => { + try { + const body = await request.formData(); + receivedFieldContent = body.get('foo') as string; + const file = body.get('baz') as File; + receivedFileName = file.name; + receivedFileType = file.type; + receivedFileContent = await file.text(); + } catch (e: any) { + return new fetchAPI.Response(e.stack, { + status: 500, + }); + } + return new fetchAPI.Response(null, { + status: 204, + }); + }, fetchAPI.Request); + server = createServer(adapter); + await new Promise(resolve => { + server.listen(0, () => { + url = `http://localhost:${(server.address() as any).port}`; + resolve(); + }); + }); + const formData = new fetchAPI.FormData(); + formData.append('foo', 'bar'); + formData.append('baz', new fetchAPI.File(['baz'], 'baz.txt', { type: 'text/plain' })); + const response = await fetchAPI.fetch(url, { + method: 'POST', + body: formData, + }); + expect(await response.text()).toBe(''); + expect(response.status).toBe(204); + expect(receivedFieldContent).toBe('bar'); + expect(receivedFileName).toBe('baz.txt'); + expect(receivedFileType).toBe('text/plain'); + expect(receivedFileContent).toBe('baz'); + }); + }); + }); +});