From f85f2637840645ccc6767e6b55b5da0e79d3f4bc Mon Sep 17 00:00:00 2001 From: Anders Evenrud Date: Wed, 3 Jan 2024 22:57:44 +0100 Subject: [PATCH] fix(vfs): defer stream creation in vfs requests (#80) 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..2f56ff6 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.attributes || {}; + 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}`); } }