From e0703c734985640b3f2f5cabd4b5705a924163d2 Mon Sep 17 00:00:00 2001 From: Paul Kinlan Date: Tue, 14 Apr 2020 01:28:42 +0100 Subject: [PATCH 1/8] Addresses #50 - parses delimeters that all allow embedded spaces --- src/worker-script/index.js | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/src/worker-script/index.js b/src/worker-script/index.js index 57ebfef..125a086 100644 --- a/src/worker-script/index.js +++ b/src/worker-script/index.js @@ -45,6 +45,36 @@ const FS = ({ } }; +const parseArgs = (command) => { + let args = []; + let nextDelimiter = 0; + let prevDelimiter = 0; + while((nextDelimiter = command.indexOf(' ', prevDelimiter)) >= 0) { + let arg = command.substring(prevDelimiter, nextDelimiter) + + if (arg[0] === '\'' || arg[0] === '\"') { + const delimeter = arg[0]; + const newNext = command.indexOf(delimeter, prevDelimiter + 1); + + if (newNext < 0) throw `Bad command espcape ${delimeter} sequence near ${nextDelimiter}` + + arg = command.substring(prevDelimiter+1, newNext); + prevDelimiter = newNext + 2; + } + else { + prevDelimiter = nextDelimiter + 1; + + if (arg === "") { + continue; + } + } + + args.push(arg) + } + + return args; +} + const run = ({ payload: { args: _args, @@ -53,7 +83,7 @@ const run = ({ if (Module === null) { throw NO_LOAD_ERROR; } else { - const args = [...defaultArgs, ..._args.trim().split(' ')].filter((s) => s.length !== 0); + const args = [...defaultArgs, ...parseArgs(_args))].filter((s) => s.length !== 0); ffmpeg(args.length, strList2ptr(Module, args)); res.resolve({ message: `Complete ${args.join(' ')}`, From 8b7c356798d33f1a6215bd3ff4115d301383ca5b Mon Sep 17 00:00:00 2001 From: Paul Kinlan Date: Tue, 14 Apr 2020 01:33:27 +0100 Subject: [PATCH 2/8] makes the expection make sense --- src/worker-script/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/worker-script/index.js b/src/worker-script/index.js index 125a086..e363626 100644 --- a/src/worker-script/index.js +++ b/src/worker-script/index.js @@ -56,7 +56,7 @@ const parseArgs = (command) => { const delimeter = arg[0]; const newNext = command.indexOf(delimeter, prevDelimiter + 1); - if (newNext < 0) throw `Bad command espcape ${delimeter} sequence near ${nextDelimiter}` + if (newNext < 0) throw `Bad command espcape sequence ${delimeter} near ${nextDelimiter}` arg = command.substring(prevDelimiter+1, newNext); prevDelimiter = newNext + 2; From 376f3132674b2c8f0c3854f3b60fc7c9e6441f45 Mon Sep 17 00:00:00 2001 From: Paul Kinlan Date: Tue, 14 Apr 2020 01:36:36 +0100 Subject: [PATCH 3/8] renaming newNext to make more sense in the context --- src/worker-script/index.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/worker-script/index.js b/src/worker-script/index.js index e363626..177f5e6 100644 --- a/src/worker-script/index.js +++ b/src/worker-script/index.js @@ -54,12 +54,12 @@ const parseArgs = (command) => { if (arg[0] === '\'' || arg[0] === '\"') { const delimeter = arg[0]; - const newNext = command.indexOf(delimeter, prevDelimiter + 1); + const endDelimeter = command.indexOf(delimeter, prevDelimiter + 1); - if (newNext < 0) throw `Bad command espcape sequence ${delimeter} near ${nextDelimiter}` + if (endDelimeter < 0) throw `Bad command espcape sequence ${delimeter} near ${nextDelimiter}` - arg = command.substring(prevDelimiter+1, newNext); - prevDelimiter = newNext + 2; + arg = command.substring(prevDelimiter+1, endDelimeter); + prevDelimiter = endDelimeter + 2; } else { prevDelimiter = nextDelimiter + 1; From dcdc8c8e83c6051f6183c80274fbb4787fe1b05a Mon Sep 17 00:00:00 2001 From: Paul Kinlan Date: Tue, 14 Apr 2020 01:39:20 +0100 Subject: [PATCH 4/8] fixing spelling of delimiter --- src/worker-script/index.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/worker-script/index.js b/src/worker-script/index.js index 177f5e6..74182db 100644 --- a/src/worker-script/index.js +++ b/src/worker-script/index.js @@ -53,13 +53,13 @@ const parseArgs = (command) => { let arg = command.substring(prevDelimiter, nextDelimiter) if (arg[0] === '\'' || arg[0] === '\"') { - const delimeter = arg[0]; - const endDelimeter = command.indexOf(delimeter, prevDelimiter + 1); + const delimiter = arg[0]; + const endDelimiter = command.indexOf(delimeter, prevDelimiter + 1); - if (endDelimeter < 0) throw `Bad command espcape sequence ${delimeter} near ${nextDelimiter}` - - arg = command.substring(prevDelimiter+1, endDelimeter); - prevDelimiter = endDelimeter + 2; + if (endDelimiter < 0) throw `Bad command espcape sequence ${delimeter} near ${nextDelimiter}` + + arg = command.substring(prevDelimiter+1, endDelimiter); + prevDelimiter = endDelimiter + 2; } else { prevDelimiter = nextDelimiter + 1; From b35eeba94c2650558667f5921b5a4270ee5131fc Mon Sep 17 00:00:00 2001 From: Paul Kinlan Date: Tue, 14 Apr 2020 01:57:55 +0100 Subject: [PATCH 5/8] Ensuring that the last parameter is parsed - fixes tests --- src/worker-script/index.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/worker-script/index.js b/src/worker-script/index.js index 74182db..b94eb59 100644 --- a/src/worker-script/index.js +++ b/src/worker-script/index.js @@ -72,6 +72,10 @@ const parseArgs = (command) => { args.push(arg) } + if (prevDelimiter != command.length) { + args.push(command.substring(prevDelimiter)) + } + return args; } @@ -83,7 +87,7 @@ const run = ({ if (Module === null) { throw NO_LOAD_ERROR; } else { - const args = [...defaultArgs, ...parseArgs(_args))].filter((s) => s.length !== 0); + const args = [...defaultArgs, ...parseArgs(_args)].filter((s) => s.length !== 0); ffmpeg(args.length, strList2ptr(Module, args)); res.resolve({ message: `Complete ${args.join(' ')}`, From 917880e1b7aaceec88206129a60672a6524c754e Mon Sep 17 00:00:00 2001 From: Paul Kinlan Date: Tue, 14 Apr 2020 03:30:17 +0100 Subject: [PATCH 6/8] Adding two tests for command parser. * Test 1: quotes appear at start of command and has a space in * Test 2: quote appears in a command, and has a space. --- .gitignore | 1 + src/worker-script/index.js | 56 ++++++++++++++++++++++++-------------- tests/constants.js | 3 ++ tests/ffmpeg.test.js | 24 ++++++++++++++++ 4 files changed, 64 insertions(+), 20 deletions(-) diff --git a/.gitignore b/.gitignore index b7b5d21..064213f 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,4 @@ node_modules dist /.nyc_output +.DS_Store \ No newline at end of file diff --git a/src/worker-script/index.js b/src/worker-script/index.js index b94eb59..e195c39 100644 --- a/src/worker-script/index.js +++ b/src/worker-script/index.js @@ -46,38 +46,54 @@ const FS = ({ }; const parseArgs = (command) => { - let args = []; + const args = []; let nextDelimiter = 0; let prevDelimiter = 0; - while((nextDelimiter = command.indexOf(' ', prevDelimiter)) >= 0) { - let arg = command.substring(prevDelimiter, nextDelimiter) + while ((nextDelimiter = command.indexOf(' ', prevDelimiter)) >= 0) { + let arg = command.substring(prevDelimiter, nextDelimiter); + let quoteIndex = arg.indexOf('\''); + let doubleQuoteIndex = arg.indexOf('"'); - if (arg[0] === '\'' || arg[0] === '\"') { + if (quoteIndex === 0 || doubleQuoteIndex === 0) { + /* The argument has a quote at the start i.e, 'id=0,streams=0 id=1,streams=1' */ const delimiter = arg[0]; - const endDelimiter = command.indexOf(delimeter, prevDelimiter + 1); + const endDelimiter = command.indexOf(delimiter, prevDelimiter + 1); - if (endDelimiter < 0) throw `Bad command espcape sequence ${delimeter} near ${nextDelimiter}` - - arg = command.substring(prevDelimiter+1, endDelimiter); - prevDelimiter = endDelimiter + 2; - } - else { - prevDelimiter = nextDelimiter + 1; - - if (arg === "") { - continue; + if (endDelimiter < 0) { + throw new Error(`Bad command escape sequence ${delimiter} near ${nextDelimiter}`); } - } - args.push(arg) + arg = command.substring(prevDelimiter + 1, endDelimiter); + prevDelimiter = endDelimiter + 2; + args.push(arg); + } else if (quoteIndex > 0 || doubleQuoteIndex > 0) { + /* The argument has a quote in it, it must be ended correctly i,e. title='test' */ + if (quoteIndex === -1) quoteIndex = Infinity; + if (doubleQuoteIndex === -1) doubleQuoteIndex = Infinity; + const delimiter = (quoteIndex < doubleQuoteIndex) ? '\'' : '"'; + const endDelimiter = command.indexOf(delimiter, prevDelimiter + Math.min(quoteIndex, doubleQuoteIndex) + 1); + + if (endDelimiter < 0) { + throw new Error(`Bad command escape sequence ${delimiter} near ${nextDelimiter}`); + } + + arg = command.substring(prevDelimiter, endDelimiter + 1); + prevDelimiter = endDelimiter + 2; + args.push(arg); + } else if (arg !== '') { + args.push(arg); + prevDelimiter = nextDelimiter + 1; + } else { + prevDelimiter = nextDelimiter + 1; + } } - if (prevDelimiter != command.length) { - args.push(command.substring(prevDelimiter)) + if (prevDelimiter !== command.length) { + args.push(command.substring(prevDelimiter)); } return args; -} +}; const run = ({ payload: { diff --git a/tests/constants.js b/tests/constants.js index af74d63..5d3eb39 100644 --- a/tests/constants.js +++ b/tests/constants.js @@ -4,8 +4,10 @@ const IS_BROWSER = typeof window !== 'undefined' && typeof window.document !== ' const OPTIONS = { corePath: '../node_modules/@ffmpeg/core/ffmpeg-core.js', ...(IS_BROWSER ? { workerPath: '../dist/worker.dev.js' } : {}), + logger: ({ message }) => console.log(message), }; const FLAME_MP4_LENGTH = 100374; +const META_FLAME_MP4_LENGTH = 100408; if (typeof module !== 'undefined') { module.exports = { @@ -14,5 +16,6 @@ if (typeof module !== 'undefined') { IS_BROWSER, OPTIONS, FLAME_MP4_LENGTH, + META_FLAME_MP4_LENGTH, }; } diff --git a/tests/ffmpeg.test.js b/tests/ffmpeg.test.js index 56258e9..ce33ea9 100644 --- a/tests/ffmpeg.test.js +++ b/tests/ffmpeg.test.js @@ -18,3 +18,27 @@ describe('transcode()', () => { )); }); }); + +describe('run()', () => { + describe('should run a command with quoted parameters at start and a space in between', () => { + ['flame.avi'].forEach((name) => ( + it(`run ${name}`, async () => { + await worker.write(name, `${BASE_URL}/${name}`); + await worker.run(`-y -i ${name} -metadata 'title="my title"' output.mp4`); + const { data } = await worker.read('output.mp4'); + expect(data.length).to.be(META_FLAME_MP4_LENGTH); + }).timeout(TIMEOUT) + )); + }); + + describe('should run a command with name quoted parameters and a space in between', () => { + ['flame.avi'].forEach((name) => ( + it(`run ${name}`, async () => { + await worker.write(name, `${BASE_URL}/${name}`); + await worker.run(`-y -i ${name} -metadata title="my title" output.mp4`); + const { data } = await worker.read('output.mp4'); + expect(data.length).to.be(META_FLAME_MP4_LENGTH); + }).timeout(TIMEOUT) + )); + }); +}); From 5ae3330d2ef0b2ce34905399cd479d9ca1a9890e Mon Sep 17 00:00:00 2001 From: Paul Kinlan Date: Tue, 14 Apr 2020 03:34:07 +0100 Subject: [PATCH 7/8] Adding an extra test to ensure quote params work even if no spaces --- tests/constants.js | 2 ++ tests/ffmpeg.test.js | 11 +++++++++++ 2 files changed, 13 insertions(+) diff --git a/tests/constants.js b/tests/constants.js index 5d3eb39..5810085 100644 --- a/tests/constants.js +++ b/tests/constants.js @@ -8,6 +8,7 @@ const OPTIONS = { }; const FLAME_MP4_LENGTH = 100374; const META_FLAME_MP4_LENGTH = 100408; +const META_FLAME_MP4_LENGTH_NO_SPACE = 100404; if (typeof module !== 'undefined') { module.exports = { @@ -17,5 +18,6 @@ if (typeof module !== 'undefined') { OPTIONS, FLAME_MP4_LENGTH, META_FLAME_MP4_LENGTH, + META_FLAME_MP4_LENGTH_NO_SPACE, }; } diff --git a/tests/ffmpeg.test.js b/tests/ffmpeg.test.js index ce33ea9..5a2243b 100644 --- a/tests/ffmpeg.test.js +++ b/tests/ffmpeg.test.js @@ -20,6 +20,17 @@ describe('transcode()', () => { }); describe('run()', () => { + describe('should run a command with quoted parameters at start no spaces', () => { + ['flame.avi'].forEach((name) => ( + it(`run ${name}`, async () => { + await worker.write(name, `${BASE_URL}/${name}`); + await worker.run(`-y -i ${name} -metadata 'title="test"' output.mp4`); + const { data } = await worker.read('output.mp4'); + expect(data.length).to.be(META_FLAME_MP4_LENGTH_NO_SPACE); + }).timeout(TIMEOUT) + )); + }); + describe('should run a command with quoted parameters at start and a space in between', () => { ['flame.avi'].forEach((name) => ( it(`run ${name}`, async () => { From 0603c41eb57373c55fbcbe22a9fb61e47eaba334 Mon Sep 17 00:00:00 2001 From: Paul Kinlan Date: Tue, 14 Apr 2020 03:38:20 +0100 Subject: [PATCH 8/8] Fixing linting errors --- src/worker-script/index.js | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/src/worker-script/index.js b/src/worker-script/index.js index e195c39..8fb98fc 100644 --- a/src/worker-script/index.js +++ b/src/worker-script/index.js @@ -45,39 +45,41 @@ const FS = ({ } }; -const parseArgs = (command) => { +const parseArgs = (cmd) => { const args = []; let nextDelimiter = 0; let prevDelimiter = 0; - while ((nextDelimiter = command.indexOf(' ', prevDelimiter)) >= 0) { - let arg = command.substring(prevDelimiter, nextDelimiter); - let quoteIndex = arg.indexOf('\''); - let doubleQuoteIndex = arg.indexOf('"'); + // eslint-disable-next-line no-cond-assign + while ((nextDelimiter = cmd.indexOf(' ', prevDelimiter)) >= 0) { + let arg = cmd.substring(prevDelimiter, nextDelimiter); + let quoteIdx = arg.indexOf('\''); + let dblQuoteIdx = arg.indexOf('"'); - if (quoteIndex === 0 || doubleQuoteIndex === 0) { + if (quoteIdx === 0 || dblQuoteIdx === 0) { /* The argument has a quote at the start i.e, 'id=0,streams=0 id=1,streams=1' */ const delimiter = arg[0]; - const endDelimiter = command.indexOf(delimiter, prevDelimiter + 1); + const endDelimiter = cmd.indexOf(delimiter, prevDelimiter + 1); if (endDelimiter < 0) { throw new Error(`Bad command escape sequence ${delimiter} near ${nextDelimiter}`); } - arg = command.substring(prevDelimiter + 1, endDelimiter); + arg = cmd.substring(prevDelimiter + 1, endDelimiter); prevDelimiter = endDelimiter + 2; args.push(arg); - } else if (quoteIndex > 0 || doubleQuoteIndex > 0) { + } else if (quoteIdx > 0 || dblQuoteIdx > 0) { /* The argument has a quote in it, it must be ended correctly i,e. title='test' */ - if (quoteIndex === -1) quoteIndex = Infinity; - if (doubleQuoteIndex === -1) doubleQuoteIndex = Infinity; - const delimiter = (quoteIndex < doubleQuoteIndex) ? '\'' : '"'; - const endDelimiter = command.indexOf(delimiter, prevDelimiter + Math.min(quoteIndex, doubleQuoteIndex) + 1); + if (quoteIdx === -1) quoteIdx = Infinity; + if (dblQuoteIdx === -1) dblQuoteIdx = Infinity; + const delimiter = (quoteIdx < dblQuoteIdx) ? '\'' : '"'; + const quoteOffset = Math.min(quoteIdx, dblQuoteIdx); + const endDelimiter = cmd.indexOf(delimiter, prevDelimiter + quoteOffset + 1); if (endDelimiter < 0) { throw new Error(`Bad command escape sequence ${delimiter} near ${nextDelimiter}`); } - arg = command.substring(prevDelimiter, endDelimiter + 1); + arg = cmd.substring(prevDelimiter, endDelimiter + 1); prevDelimiter = endDelimiter + 2; args.push(arg); } else if (arg !== '') { @@ -88,8 +90,8 @@ const parseArgs = (command) => { } } - if (prevDelimiter !== command.length) { - args.push(command.substring(prevDelimiter)); + if (prevDelimiter !== cmd.length) { + args.push(cmd.substring(prevDelimiter)); } return args;