From fa6edecadbecf9cba3e169bfcac9e25a3d58d41a Mon Sep 17 00:00:00 2001 From: Anders Evenrud Date: Tue, 2 Jan 2024 18:41:59 +0100 Subject: [PATCH 1/2] fix(vfs): defer stream creation in vfs requests Defers the creation of streams in the VFS process to avoid any cleanups that ends up in exceptions because they are handled outside the scope/context. Fixes #79 --- src/vfs.js | 63 +++++++++++++++++++++++++++--------------------------- 1 file changed, 31 insertions(+), 32 deletions(-) diff --git a/src/vfs.js b/src/vfs.js index 75c09ba..55a8ca7 100644 --- a/src/vfs.js +++ b/src/vfs.js @@ -43,10 +43,10 @@ const { const respondNumber = result => typeof result === 'number' ? result : -1; const respondBoolean = result => typeof result === 'boolean' ? result : !!result; -const requestPath = req => ([sanitize(req.fields.path)]); -const requestSearch = req => ([sanitize(req.fields.root), req.fields.pattern]); -const requestCross = req => ([sanitize(req.fields.from), sanitize(req.fields.to)]); -const requestFile = req => ([sanitize(req.fields.path), streamFromRequest(req)]); +const requestPath = req => [sanitize(req.fields.path)]; +const requestSearch = req => [sanitize(req.fields.root), req.fields.pattern]; +const requestFile = req => [sanitize(req.fields.path), () => streamFromRequest(req)]; +const requestCross = req => [sanitize(req.fields.from), sanitize(req.fields.to)]; /* * Parses the range request headers @@ -65,7 +65,10 @@ const onDone = (req, res) => { if (req.files) { for (let fieldname in req.files) { try { - fs.removeSync(req.files[fieldname].path); + const n = req.files[fieldname].path; + if (fs.existsSync(n)) { + fs.removeSync(n); + } } catch (e) { console.warn('Failed to unlink temporary file', e); } @@ -77,26 +80,18 @@ const onDone = (req, res) => { * Wraps a vfs adapter request */ const wrapper = fn => (req, res, next) => fn(req, res) - .then(result => { + .then(result => new Promise((resolve, reject) => { if (result instanceof Stream) { - result.on('error', error => { - next(error); - }); - - result.on('end', () => { - onDone(req, res); - }); - + result.once('error', reject); + result.once('end', resolve); result.pipe(res); } else { res.json(result); - onDone(req, res); + resolve(); } - }) - .catch(error => { - next(error); - onDone(req, res); - }); + })) + .catch(error => next(error)) + .finally(() => onDone(req, res)); /** * Creates the middleware @@ -147,27 +142,31 @@ const createOptions = req => { // Standard request with only a target const createRequestFactory = findMountpoint => (getter, method, readOnly, respond) => async (req, res) => { const options = createOptions(req); - const args = [...getter(req, res), options]; + const [target, ...rest] = getter(req, res); + + const found = await findMountpoint(target); + const attributes = found.mount || {}; + const strict = attributes.strictGroups !== false; - const found = await findMountpoint(args[0]); if (method === 'search') { - if (found.mount.attributes && found.mount.attributes.searchable === false) { + if (attributes.searchable === false) { return []; } } - const {attributes} = found.mount; - const strict = attributes.strictGroups !== false; - const ranges = (!attributes.adapter || attributes.adapter === 'system') || attributes.ranges === true; - const vfsMethodWrapper = m => found.adapter[m] - ? found.adapter[m](found)(...args) - : Promise.reject(new Error(`Adapter does not support ${m}`)); - const readstat = () => vfsMethodWrapper('stat').catch(() => ({})); await checkMountpointPermission(req, res, method, readOnly, strict)(found); + const vfsMethodWrapper = m => { + const args = [target, ...rest.map(r => typeof r === 'function' ? r() : r), options]; + return found.adapter[m] + ? found.adapter[m](found)(...args) + : Promise.reject(new Error(`Adapter does not support ${m}`)); + }; + const result = await vfsMethodWrapper(method); if (method === 'readfile') { - const stat = await readstat(); + const ranges = (!attributes.adapter || attributes.adapter === 'system') || attributes.ranges === true; + const stat = await vfsMethodWrapper('stat').catch(() => ({})); if (ranges && options.range) { try { @@ -192,7 +191,7 @@ const createRequestFactory = findMountpoint => (getter, method, readOnly, respon } if (options.download) { - const filename = encodeURIComponent(path.basename(args[0])); + const filename = encodeURIComponent(path.basename(target)); res.append('Content-Disposition', `attachment; filename*=utf-8''${filename}`); } } From 68a5cb9881027edc037b2a4e7e3bae08840e259f Mon Sep 17 00:00:00 2001 From: Anders Evenrud Date: Tue, 2 Jan 2024 20:16:39 +0100 Subject: [PATCH 2/2] fix(vfs): typo in attributes lookup --- src/vfs.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vfs.js b/src/vfs.js index 55a8ca7..2f56ff6 100644 --- a/src/vfs.js +++ b/src/vfs.js @@ -145,7 +145,7 @@ const createRequestFactory = findMountpoint => (getter, method, readOnly, respon const [target, ...rest] = getter(req, res); const found = await findMountpoint(target); - const attributes = found.mount || {}; + const attributes = found.mount.attributes || {}; const strict = attributes.strictGroups !== false; if (method === 'search') {