From d3a05e937b664f996ae6a46757e5e9bb48522e6e Mon Sep 17 00:00:00 2001 From: Cyril Walle Date: Thu, 19 May 2022 10:34:12 +0200 Subject: [PATCH 01/11] add failing test case --- test/unit/formidable.test.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/unit/formidable.test.js b/test/unit/formidable.test.js index 4fd4357e..e46aaeb0 100644 --- a/test/unit/formidable.test.js +++ b/test/unit/formidable.test.js @@ -94,6 +94,11 @@ function makeHeader(originalFilename) { expect(basename).toHaveLength(30); ext = path.extname(basename); expect(ext).toBe('.QxZs'); + + basename = getBasename('test.pdf.jqlnn.png'); + expect(basename).toBe('test.pdf.jqlnnimgsrcaonerroralert1.png'); + ext = path.extname(basename); + expect(ext).toBe('.png'); }); test(`${name}#_Array parameters support`, () => { From 15afa8a41870c7e466e3710758fca853f53be02c Mon Sep 17 00:00:00 2001 From: Cyril Walle Date: Thu, 19 May 2022 10:34:36 +0200 Subject: [PATCH 02/11] docs: add comment --- src/Formidable.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Formidable.js b/src/Formidable.js index b777121a..7c37280b 100644 --- a/src/Formidable.js +++ b/src/Formidable.js @@ -497,6 +497,9 @@ class IncomingForm extends EventEmitter { return originalFilename; } + // able to get composed extension with multiple dots + // "a.b.c" -> ".b.c" + // as opposed to path.extname -> ".c" _getExtension(str) { if (!str) { return ''; From 703bec4f0265baf26280d5b2c058b91a4c02e869 Mon Sep 17 00:00:00 2001 From: Cyril Walle Date: Thu, 19 May 2022 10:49:29 +0200 Subject: [PATCH 03/11] tests: add comment --- test/unit/formidable.test.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/unit/formidable.test.js b/test/unit/formidable.test.js index e46aaeb0..b9ede019 100644 --- a/test/unit/formidable.test.js +++ b/test/unit/formidable.test.js @@ -65,6 +65,7 @@ function makeHeader(originalFilename) { const getBasename = (part) => path.basename(form._getNewName(part)); + // tests below assume baseline hexoid 25 chars + a few more for the extension let basename = getBasename('fine.jpg?foo=bar'); expect(basename).toHaveLength(29); let ext = path.extname(basename); @@ -97,6 +98,7 @@ function makeHeader(originalFilename) { basename = getBasename('test.pdf.jqlnn.png'); expect(basename).toBe('test.pdf.jqlnnimgsrcaonerroralert1.png'); + expect(basename).toHaveLength(29); ext = path.extname(basename); expect(ext).toBe('.png'); }); From d2bd18d2fea54e864784ff582ed5ef8d5a7809f9 Mon Sep 17 00:00:00 2001 From: Cyril Walle Date: Thu, 19 May 2022 11:34:17 +0200 Subject: [PATCH 04/11] tests: add a test case that proves that the regex was always bad --- test/unit/formidable.test.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test/unit/formidable.test.js b/test/unit/formidable.test.js index b9ede019..e436f22d 100644 --- a/test/unit/formidable.test.js +++ b/test/unit/formidable.test.js @@ -95,9 +95,13 @@ function makeHeader(originalFilename) { expect(basename).toHaveLength(30); ext = path.extname(basename); expect(ext).toBe('.QxZs'); - + basename = getBasename('test.pdf.jqlnn.png'); - expect(basename).toBe('test.pdf.jqlnnimgsrcaonerroralert1.png'); + expect(basename).toHaveLength(29); + ext = path.extname(basename); + expect(ext).toBe('.png'); + + basename = getBasename('test. Date: Thu, 19 May 2022 11:34:55 +0200 Subject: [PATCH 05/11] fix: replace regex with reliable filtering --- src/Formidable.js | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/src/Formidable.js b/src/Formidable.js index 7c37280b..c205fbb7 100644 --- a/src/Formidable.js +++ b/src/Formidable.js @@ -42,6 +42,15 @@ function hasOwnProp(obj, key) { return Object.prototype.hasOwnProperty.call(obj, key); } +const validExtensionChar = (c) => { + const code = c.charCodeAt(0); + return ( + code === 46 || // . + (code >= 65 && code <= 90) || + (code >= 97 && code <= 122) + ); +}; + class IncomingForm extends EventEmitter { constructor(options = {}) { super(); @@ -508,13 +517,13 @@ class IncomingForm extends EventEmitter { const basename = path.basename(str); const firstDot = basename.indexOf('.'); const lastDot = basename.lastIndexOf('.'); - const extname = path.extname(basename).replace(/(\.[a-z0-9]+).*/i, '$1'); + let rawExtname = path.extname(basename); - if (firstDot === lastDot) { - return extname; + if (firstDot !== lastDot) { + rawExtname = basename.slice(firstDot); } - return basename.slice(firstDot, lastDot) + extname; + return Array.from(rawExtname).filter(validExtensionChar).join(''); } _joinDirectoryName(name) { From 78de8492e05f0263fa7b46e839ab4bd3e0152552 Mon Sep 17 00:00:00 2001 From: Cyril Walle Date: Thu, 19 May 2022 11:48:46 +0200 Subject: [PATCH 06/11] feat: stop at first invalid char --- src/Formidable.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/Formidable.js b/src/Formidable.js index c205fbb7..598a1ba1 100644 --- a/src/Formidable.js +++ b/src/Formidable.js @@ -42,9 +42,9 @@ function hasOwnProp(obj, key) { return Object.prototype.hasOwnProperty.call(obj, key); } -const validExtensionChar = (c) => { +const invalidExtensionChar = (c) => { const code = c.charCodeAt(0); - return ( + return !( code === 46 || // . (code >= 65 && code <= 90) || (code >= 97 && code <= 122) @@ -523,7 +523,11 @@ class IncomingForm extends EventEmitter { rawExtname = basename.slice(firstDot); } - return Array.from(rawExtname).filter(validExtensionChar).join(''); + const firstInvalidIndex = Array.from(rawExtname).findIndex(invalidExtensionChar); + if (firstInvalidIndex === -1) { + return rawExtname; + } + return rawExtname.substring(0, firstInvalidIndex); } _joinDirectoryName(name) { From 67c6a3f53d501dd7931bbdf2436b6e837a7a9584 Mon Sep 17 00:00:00 2001 From: Cyril Walle Date: Thu, 19 May 2022 11:49:11 +0200 Subject: [PATCH 07/11] feat: allow numbers in file extensions --- src/Formidable.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Formidable.js b/src/Formidable.js index 598a1ba1..941838a6 100644 --- a/src/Formidable.js +++ b/src/Formidable.js @@ -46,6 +46,7 @@ const invalidExtensionChar = (c) => { const code = c.charCodeAt(0); return !( code === 46 || // . + (code >= 48 && code <= 57) || (code >= 65 && code <= 90) || (code >= 97 && code <= 122) ); From 5103d09c0b8d21a1d1cacfa0b3ef4a0e361fd047 Mon Sep 17 00:00:00 2001 From: Cyril Walle Date: Thu, 19 May 2022 11:57:55 +0200 Subject: [PATCH 08/11] feat: stop extension from being '.' --- src/Formidable.js | 10 ++++++++-- test/unit/formidable.test.js | 8 ++++---- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/Formidable.js b/src/Formidable.js index 941838a6..11aeda3c 100644 --- a/src/Formidable.js +++ b/src/Formidable.js @@ -524,11 +524,17 @@ class IncomingForm extends EventEmitter { rawExtname = basename.slice(firstDot); } + let filtered; const firstInvalidIndex = Array.from(rawExtname).findIndex(invalidExtensionChar); if (firstInvalidIndex === -1) { - return rawExtname; + filtered = rawExtname; + } else { + filtered = rawExtname.substring(0, firstInvalidIndex); + } + if (filtered === '.') { + return ''; } - return rawExtname.substring(0, firstInvalidIndex); + return filtered; } _joinDirectoryName(name) { diff --git a/test/unit/formidable.test.js b/test/unit/formidable.test.js index e436f22d..ab6de22a 100644 --- a/test/unit/formidable.test.js +++ b/test/unit/formidable.test.js @@ -97,14 +97,14 @@ function makeHeader(originalFilename) { expect(ext).toBe('.QxZs'); basename = getBasename('test.pdf.jqlnn.png'); - expect(basename).toHaveLength(29); + expect(basename).toHaveLength(35); ext = path.extname(basename); - expect(ext).toBe('.png'); + expect(ext).toBe('.jqlnn'); basename = getBasename('test. { From 9969c25ab696161d3852a35095ccb25dda48da18 Mon Sep 17 00:00:00 2001 From: Cyril Walle Date: Thu, 19 May 2022 12:06:14 +0200 Subject: [PATCH 09/11] refactor: code style --- src/PersistentFile.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/PersistentFile.js b/src/PersistentFile.js index fec6a4de..af8c7785 100644 --- a/src/PersistentFile.js +++ b/src/PersistentFile.js @@ -1,7 +1,7 @@ /* eslint-disable no-underscore-dangle */ -import { WriteStream, unlink } from 'node:fs'; -import { createHash } from 'node:crypto'; +import fs from 'node:fs'; +import crypto from 'node:crypto'; import { EventEmitter } from 'node:events'; class PersistentFile extends EventEmitter { @@ -15,14 +15,14 @@ class PersistentFile extends EventEmitter { this._writeStream = null; if (typeof this.hashAlgorithm === 'string') { - this.hash = createHash(this.hashAlgorithm); + this.hash = crypto.createHash(this.hashAlgorithm); } else { this.hash = null; } } open() { - this._writeStream = new WriteStream(this.filepath); + this._writeStream = fs.createWriteStream(this.filepath); this._writeStream.on('error', (err) => { this.emit('error', err); }); @@ -80,7 +80,7 @@ class PersistentFile extends EventEmitter { this._writeStream.destroy(); const filepath = this.filepath; setTimeout(function () { - unlink(filepath, () => {}); + fs.unlink(filepath, () => {}); }, 1) } } From 2f553b4757f47740606b299f1fa49becc3b701c4 Mon Sep 17 00:00:00 2001 From: Cyril Walle Date: Thu, 19 May 2022 12:08:27 +0200 Subject: [PATCH 10/11] docs: use slugify in the example --- examples/with-http.js | 21 +++++++++++---------- package.json | 1 + 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/examples/with-http.js b/examples/with-http.js index 4d0fe54a..b19b0712 100644 --- a/examples/with-http.js +++ b/examples/with-http.js @@ -1,4 +1,5 @@ import http from 'node:http'; +import slugify from '@sindresorhus/slugify'; import formidable from '../src/index.js'; @@ -14,16 +15,16 @@ const server = http.createServer((req, res) => { const form = formidable({ // uploadDir: `uploads`, keepExtensions: true, - // filename(/*name, ext, part, form*/) { - // /* name basename of the http originalFilename - // ext with the dot ".txt" only if keepExtensions is true - // */ - // // slugify to avoid invalid filenames - // // substr to define a maximum length - // // return `${slugify(name)}.${slugify(ext, {separator: ''})}`.substr(0, 100); - // return 'yo.txt'; // or completly different name - // // return 'z/yo.txt'; // subdirectory - // }, + filename(name, ext, part, form) { + /* name basename of the http originalFilename + ext with the dot ".txt" only if keepExtensions is true + */ + // slugify to avoid invalid filenames + // substr to define a maximum + return `${slugify(name)}.${slugify(ext, {separator: ''})}`.substr(0, 100); + // return 'yo.txt'; // or completly different name + // return 'z/yo.txt'; // subdirectory + }, // filter: function ({name, originalFilename, mimetype}) { // // keep only images // return mimetype && mimetype.includes("image"); diff --git a/package.json b/package.json index da21d4e8..1897a851 100644 --- a/package.json +++ b/package.json @@ -37,6 +37,7 @@ "devDependencies": { "@commitlint/cli": "8.3.5", "@commitlint/config-conventional": "8.3.4", + "@sindresorhus/slugify": "^2.1.0", "@tunnckocore/prettier-config": "1.3.8", "del-cli": "3.0.0", "eslint": "6.8.0", From 143e473f2989f7efcd918b86a79a96a054f24bfa Mon Sep 17 00:00:00 2001 From: Cyril Walle Date: Thu, 19 May 2022 12:09:50 +0200 Subject: [PATCH 11/11] chore: prepare release --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 1897a851..32df036b 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "formidable", - "version": "3.2.3", + "version": "3.2.4", "license": "MIT", "description": "A node.js module for parsing form data, especially file uploads.", "homepage": "https://github.com/node-formidable/formidable",