Skip to content

Commit f634619

Browse files
jsumnersgurgunday
jsumners
and
gurgunday
authored
Merge next into master (#533)
* update for Fastify v5 (#505) * fix: make sure the handler resolves in all cases (#507) * fix error in processing file * fix error in processing file * add test * fix close * fix test * lint * revert change * Revert "revert change" This reverts commit 17ec56c. * node 14 :( * node 14 :( * add file * use filePath * delete foo * revert change * remove console.log * add test number * increase timeout * try fixing the error * try fixing the error * try reverting? * Revert "try reverting?" This reverts commit 35aca1e. * 90 * fix error * fix error * let request aborted destroy the file * simplify * simplify * ci * update for Fastify v5 (#505) * fix: make sure the handler resolves in all cases (#507) * fix error in processing file * fix error in processing file * add test * fix close * fix test * lint * revert change * Revert "revert change" This reverts commit 17ec56c. * node 14 :( * node 14 :( * add file * use filePath * delete foo * revert change * remove console.log * add test number * increase timeout * try fixing the error * try fixing the error * try reverting? * Revert "try reverting?" This reverts commit 35aca1e. * 90 * fix error * fix error * let request aborted destroy the file * simplify * simplify * ci * update fastify deps * update for Fastify v5 (#505) * fix: make sure the handler resolves in all cases (#507) * fix error in processing file * fix error in processing file * add test * fix close * fix test * lint * revert change * Revert "revert change" This reverts commit 17ec56c. * node 14 :( * node 14 :( * add file * use filePath * delete foo * revert change * remove console.log * add test number * increase timeout * try fixing the error * try fixing the error * try reverting? * Revert "try reverting?" This reverts commit 35aca1e. * 90 * fix error * fix error * let request aborted destroy the file * simplify * simplify * ci * Update index.js Co-authored-by: Gürgün Dayıoğlu <[email protected]> Signed-off-by: James Sumners <[email protected]> --------- Signed-off-by: James Sumners <[email protected]> Co-authored-by: Gürgün Dayıoğlu <[email protected]> Co-authored-by: Gürgün Dayıoğlu <[email protected]>
1 parent 55b4dd3 commit f634619

11 files changed

+124
-89
lines changed

‎.github/workflows/ci.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ on:
1717

1818
jobs:
1919
test:
20-
uses: fastify/workflows/.github/workflows/plugins-ci.yml@v3
20+
uses: fastify/workflows/.github/workflows/plugins-ci.yml@v4.0.0
2121
with:
2222
license-check: true
2323
lint: true

‎.gitignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,9 @@ lib-cov
2323
coverage
2424
*.lcov
2525

26+
# .tap output
27+
.tap
28+
2629
# nyc test coverage
2730
.nyc_output
2831

‎index.js

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,11 @@ const { createWriteStream } = require('node:fs')
77
const { unlink } = require('node:fs/promises')
88
const path = require('node:path')
99
const { generateId } = require('./lib/generateId')
10-
const util = require('node:util')
1110
const createError = require('@fastify/error')
12-
const sendToWormhole = require('stream-wormhole')
11+
const streamToNull = require('./lib/stream-consumer')
1312
const deepmergeAll = require('@fastify/deepmerge')({ all: true })
14-
const { PassThrough, pipeline, Readable } = require('node:stream')
15-
const pump = util.promisify(pipeline)
13+
const { PassThrough, Readable } = require('node:stream')
14+
const { pipeline: pump } = require('node:stream/promises')
1615
const secureJSON = require('secure-json-parse')
1716

1817
const kMultipart = Symbol('multipart')
@@ -96,7 +95,6 @@ function fastifyMultipart (fastify, options, done) {
9695
const field = req.body[key]
9796

9897
/* Don't modify the body if a field doesn't have a value or an attached buffer */
99-
/* istanbul ignore else */
10098
if (field.value !== undefined) {
10199
body[key] = field.value
102100
} else if (field._buf) {
@@ -143,6 +141,7 @@ function fastifyMultipart (fastify, options, done) {
143141
}
144142

145143
async function append (key, entry) {
144+
/* c8 ignore next: Buffer.isBuffer is not covered and causing `npm test` to fail */
146145
if (entry.type === 'file' || (attachFieldsToBody === 'keyValues' && Buffer.isBuffer(entry))) {
147146
// TODO use File constructor with fs.openAsBlob()
148147
// if attachFieldsToBody is not set
@@ -163,6 +162,7 @@ function fastifyMultipart (fastify, options, done) {
163162
/* istanbul ignore next */
164163
if (!fastify.hasRequestDecorator('formData')) {
165164
fastify.decorateRequest('formData', async function () {
165+
/* c8 ignore next: Next line is not covered and causing `npm test` to fail */
166166
throw new NoFormData()
167167
})
168168
}
@@ -349,7 +349,7 @@ function fastifyMultipart (fastify, options, done) {
349349
// don't overwrite prototypes
350350
if (name in Object.prototype) {
351351
// ensure that stream is consumed, any error is suppressed
352-
sendToWormhole(file)
352+
streamToNull(file).catch(() => {})
353353
onError(new PrototypeViolationError())
354354
return
355355
}
@@ -501,10 +501,10 @@ function fastifyMultipart (fastify, options, done) {
501501
const filepath = this.tmpUploads[i]
502502
try {
503503
await unlink(filepath)
504-
} catch (error) {
505-
/* istanbul ignore next */
504+
} /* c8 ignore start */ catch (error) {
506505
this.log.error(error, 'Could not delete file')
507506
}
507+
/* c8 ignore stop */
508508
}
509509
}
510510

@@ -513,7 +513,6 @@ function fastifyMultipart (fastify, options, done) {
513513
let part
514514
while ((part = await parts()) != null) {
515515
/* Only return a part if the file property exists */
516-
/* istanbul ignore else */
517516
if (part.file) {
518517
return part
519518
}

‎lib/stream-consumer.js

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
'use strict'
2+
3+
module.exports = function streamToNull (stream) {
4+
return new Promise((resolve, reject) => {
5+
stream.on('data', () => {
6+
/* The stream needs a data reader or else it will never end. */
7+
})
8+
stream.on('close', () => {
9+
resolve()
10+
})
11+
stream.on('end', () => {
12+
resolve()
13+
})
14+
stream.on('error', (error) => {
15+
reject(error)
16+
})
17+
})
18+
}

‎package.json

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -9,30 +9,32 @@
99
"@fastify/busboy": "^3.0.0",
1010
"@fastify/deepmerge": "^2.0.0",
1111
"@fastify/error": "^4.0.0",
12-
"fastify-plugin": "^4.0.0",
13-
"secure-json-parse": "^2.4.0",
14-
"stream-wormhole": "^1.1.0"
12+
"fastify-plugin": "^5.0.0-pre.fv5.1",
13+
"secure-json-parse": "^2.7.0"
1514
},
1615
"devDependencies": {
17-
"@fastify/pre-commit": "^2.0.2",
18-
"@fastify/swagger": "^8.10.1",
19-
"@fastify/swagger-ui": "^4.0.0",
20-
"@types/node": "^20.1.0",
16+
"@fastify/pre-commit": "^2.1.0",
17+
"@fastify/swagger": "^9.0.0-pre.fv5.1",
18+
"@fastify/swagger-ui": "^5.0.0-pre.fv5.1",
19+
"@types/node": "^20.11.6",
2120
"@typescript-eslint/eslint-plugin": "^7.1.0",
2221
"@typescript-eslint/parser": "^7.1.0",
2322
"benchmark": "^2.1.4",
2423
"climem": "^2.0.0",
2524
"concat-stream": "^2.0.0",
26-
"eslint": "^8.20.0",
27-
"fastify": "^4.0.0",
25+
"eslint": "^8.56.0",
26+
"eslint-plugin-import": "^2.29.1",
27+
"eslint-plugin-n": "^16.6.2",
28+
"eslint-plugin-promise": "^6.1.1",
29+
"fastify": "^5.0.0-alpha.3",
2830
"form-data": "^4.0.0",
2931
"h2url": "^0.2.0",
3032
"noop-stream": "^0.1.0",
3133
"pump": "^3.0.0",
32-
"readable-stream": "^4.5.1",
34+
"readable-stream": "^4.5.2",
3335
"snazzy": "^9.0.0",
34-
"standard": "^17.0.0",
35-
"tap": "^16.0.0",
36+
"standard": "^17.1.0",
37+
"tap": "^18.6.1",
3638
"tsd": "^0.31.0"
3739
},
3840
"scripts": {
@@ -45,7 +47,7 @@
4547
"start": "CLIMEM=8999 node -r climem ./examples/example",
4648
"test": "npm run test:unit && npm run test:typescript",
4749
"test:typescript": "tsd",
48-
"test:unit": "tap -t 90"
50+
"test:unit": "tap -t 120"
4951
},
5052
"repository": {
5153
"type": "git",

‎test/big.test.js

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ const stream = require('readable-stream')
99
const Readable = stream.Readable
1010
const pump = stream.pipeline
1111
const crypto = require('node:crypto')
12-
const sendToWormhole = require('stream-wormhole')
12+
const streamToNull = require('../lib/stream-consumer')
1313

1414
// skipping on Github Actions because it takes too long
1515
test('should upload a big file in constant memory', { skip: process.env.CI }, function (t) {
@@ -38,7 +38,7 @@ test('should upload a big file in constant memory', { skip: process.env.CI }, fu
3838
t.equal(part.encoding, '7bit')
3939
t.equal(part.mimetype, 'binary/octet-stream')
4040

41-
await sendToWormhole(part.file)
41+
await streamToNull(part.file)
4242
}
4343
}
4444

@@ -78,10 +78,11 @@ test('should upload a big file in constant memory', { skip: process.env.CI }, fu
7878
knownLength
7979
})
8080

81+
const addresses = fastify.addresses()
8182
const opts = {
8283
protocol: 'http:',
83-
hostname: 'localhost',
84-
port: fastify.server.address().port,
84+
hostname: addresses[0].address,
85+
port: addresses[0].port,
8586
path: '/',
8687
headers: form.getHeaders(),
8788
method: 'POST'

‎test/multipart-big-stream.test.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ const multipart = require('..')
77
const http = require('node:http')
88
const crypto = require('node:crypto')
99
const { Readable } = require('readable-stream')
10-
const sendToWormhole = require('stream-wormhole')
10+
const streamToNull = require('../lib/stream-consumer')
1111
const EventEmitter = require('node:events')
1212
const { once } = EventEmitter
1313

@@ -23,7 +23,7 @@ test('should emit fileSize limitation error during streaming', async function (t
2323
fastify.post('/', async function (req, reply) {
2424
t.ok(req.isMultipart())
2525
const part = await req.file({ limits: { fileSize: 16500 } })
26-
await sendToWormhole(part.file)
26+
await streamToNull(part.file)
2727
if (part.file.truncated) {
2828
reply.code(500).send()
2929
} else {

‎test/multipart-http2.test.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ const multipart = require('..')
77
const h2url = require('h2url')
88
const path = require('node:path')
99
const fs = require('node:fs')
10-
const sendToWormhole = require('stream-wormhole')
10+
const streamToNull = require('../lib/stream-consumer')
1111

1212
const filePath = path.join(__dirname, '../README.md')
1313

@@ -21,7 +21,7 @@ test('should respond when all files are processed', function (t) {
2121
const parts = req.files()
2222
for await (const part of parts) {
2323
t.ok(part.file)
24-
await sendToWormhole(part.file)
24+
await streamToNull(part.file)
2525
}
2626
reply.code(200).send()
2727
})

‎test/multipart-small-stream.test.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ const http = require('node:http')
88
const path = require('node:path')
99
const fs = require('node:fs')
1010
const EventEmitter = require('node:events')
11-
const sendToWormhole = require('stream-wormhole')
11+
const streamToNull = require('../lib/stream-consumer')
1212
const { once } = EventEmitter
1313

1414
const filePath = path.join(__dirname, '../README.md')
@@ -26,7 +26,7 @@ test('should throw fileSize limitation error on small payload', { skip: true },
2626
t.ok(req.isMultipart())
2727

2828
const part = await req.file({ limits: { fileSize: 2 } })
29-
await sendToWormhole(part.file)
29+
await streamToNull(part.file)
3030

3131
reply.code(200).send()
3232
})
@@ -71,7 +71,7 @@ test('should not throw and error when throwFileSizeLimit option is false', { ski
7171
t.ok(req.isMultipart())
7272

7373
const part = await req.file({ limits: { fileSize: 2 }, throwFileSizeLimit: false })
74-
await sendToWormhole(part.file)
74+
await streamToNull(part.file)
7575

7676
reply.code(200).send()
7777
})

‎test/multipart.test.js

Lines changed: 8 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ const concat = require('concat-stream')
1212
const stream = require('node:stream')
1313
const { once } = require('node:events')
1414
const pump = util.promisify(stream.pipeline)
15-
const sendToWormhole = require('stream-wormhole')
15+
const streamToNull = require('../lib/stream-consumer')
1616

1717
const filePath = path.join(__dirname, '../README.md')
1818

@@ -89,7 +89,7 @@ test('should respond when all files are processed', function (t) {
8989
for await (const part of parts) {
9090
t.ok(part.file)
9191
t.equal(part.type, 'file')
92-
await sendToWormhole(part.file)
92+
await streamToNull(part.file)
9393
}
9494
reply.code(200).send()
9595
})
@@ -141,7 +141,7 @@ test('should group parts with the same name to an array', function (t) {
141141
t.pass('multiple files are grouped by array')
142142
}
143143
if (part.file) {
144-
await sendToWormhole(part.file)
144+
await streamToNull(part.file)
145145
}
146146
}
147147
reply.code(200).send()
@@ -270,7 +270,7 @@ test('should throw error due to filesLimit (The max number of file fields (Defau
270270
const parts = req.files({ limits: { files: 1 } })
271271
for await (const part of parts) {
272272
t.ok(part.file, 'part received')
273-
await sendToWormhole(part.file)
273+
await streamToNull(part.file)
274274
}
275275
reply.code(200).send()
276276
} catch (error) {
@@ -330,7 +330,7 @@ test('should be able to configure limits globally with plugin register options',
330330
for await (const part of parts) {
331331
t.ok(part.file)
332332
t.equal(part.type, 'file')
333-
await sendToWormhole(part.file)
333+
await streamToNull(part.file)
334334
}
335335
reply.code(200).send()
336336
} catch (error) {
@@ -485,7 +485,7 @@ test('should throw error due to file size limit exceed (Default: true)', functio
485485
for await (const part of parts) {
486486
t.ok(part.file)
487487
t.equal(part.type, 'file')
488-
await sendToWormhole(part.file)
488+
await streamToNull(part.file)
489489
}
490490
reply.code(200).send()
491491
} catch (error) {
@@ -532,7 +532,7 @@ test('should not throw error due to file size limit exceed - files setting (Defa
532532
for await (const part of parts) {
533533
t.ok(part.file)
534534
t.equal(part.type, 'file')
535-
await sendToWormhole(part.file)
535+
await streamToNull(part.file)
536536
}
537537
reply.code(200).send()
538538
})
@@ -635,7 +635,7 @@ test('should not miss fields if part handler takes much time than formdata parsi
635635
t.pass('res ended successfully')
636636
})
637637

638-
test('should not freeze when error is thrown during processing', { skip: process.versions.node.startsWith('14') }, async function (t) {
638+
test('should not freeze when error is thrown during processing', async function (t) {
639639
t.plan(2)
640640
const app = Fastify()
641641

@@ -689,50 +689,3 @@ test('should not freeze when error is thrown during processing', { skip: process
689689

690690
await app.close()
691691
})
692-
693-
const hasGlobalFormData = typeof globalThis.FormData === 'function'
694-
695-
test('no formData', { skip: !hasGlobalFormData }, function (t) {
696-
t.plan(6)
697-
const fastify = Fastify()
698-
t.teardown(fastify.close.bind(fastify))
699-
700-
fastify.register(multipart)
701-
702-
fastify.post('/', async function (req, reply) {
703-
await t.rejects(req.formData())
704-
705-
for await (const part of req.parts()) {
706-
t.equal(part.type, 'field')
707-
t.equal(part.fieldname, 'hello')
708-
t.equal(part.value, 'world')
709-
}
710-
711-
reply.code(200).send()
712-
})
713-
714-
fastify.listen({ port: 0 }, async function () {
715-
// request
716-
const form = new FormData()
717-
const opts = {
718-
protocol: 'http:',
719-
hostname: 'localhost',
720-
port: fastify.server.address().port,
721-
path: '/',
722-
headers: form.getHeaders(),
723-
method: 'POST'
724-
}
725-
726-
const req = http.request(opts, (res) => {
727-
t.equal(res.statusCode, 200)
728-
// consume all data without processing
729-
res.resume()
730-
res.on('end', () => {
731-
t.pass('res ended successfully')
732-
})
733-
})
734-
form.append('hello', 'world')
735-
736-
form.pipe(req)
737-
})
738-
})

0 commit comments

Comments
 (0)