fix: correct escaping for filters, related to #5

src/command.ts

@@ -17,6 +17,7 @@ import { probe, ProbeOptions, ProbeResult } from './probe';
 import { FFmpegProcess, Process } from './process';
 import {
   escapeConcatFile,
+  escapeFilterDescription,
   escapeTeeComponent,
   stringifyFilterDescription,
   stringifyObjectColonSeparated
@@ -417,8 +418,10 @@ class Command implements FFmpegCommand {
     this.args(overwrite !== false ? '-y' : '-n');
     if (progress !== false)
       this.args('-progress', 'pipe:1', '-nostats');
-    if (!isNullish(logger))
-      this.args('-loglevel', `+repeat+level+${logger.logLevel ?? LogLevel.Error}`);
+    if (!isNullish(logger)) {
+      const { logLevel } = logger;
+      this.args('-loglevel', `+repeat+level${isNullish(logLevel) ? '' : `+${logLevel}`}`);
+    }
   }
   #args: string[] = [];
   #inputs: Input[] = [];
@@ -467,16 +470,18 @@ class Command implements FFmpegCommand {
     inputStreams.push([path, stream]);
     const input = new Input(getSocketUrl(path), true, stream);
 
+    const { safe, protocols } = options;
+
     // Add extra arguments to the input based on the given options
     // the option safe is NOT enabled by default because it doesn't
     // allow streams or protocols other than the currently used one,
     // which, depending on the platform, may be `file` (on Windows)
     // or `unix` (on every other platform).
-    input.args('-safe', options.safe ? '1' : '0');
+    input.args('-safe', safe ? '1' : '0');
     // Protocol whitelist enables certain protocols in the ffconcat
     // file dynamically created by this method.
-    if (options.protocols && options.protocols.length > 0)
-      input.args('-protocol_whitelist', options.protocols.join(','));
+    if (!isNullish(protocols) && protocols.length > 0)
+      input.args('-protocol_whitelist', protocols.join(','));
 
     this.#inputs.push(input);
     return input;
@@ -740,8 +745,10 @@ class Output implements FFmpegOutput {
     const audioFilters = this.#audioFilters;
     return [
       ...this.#args,
-      ...(videoFilters.length > 0 ? ['-filter:V', videoFilters.join(',')] : []),
-      ...(audioFilters.length > 0 ? ['-filter:a', audioFilters.join(',')] : []),
+      ...(videoFilters.length > 0
+        ? ['-filter:V', videoFilters.map(escapeFilterDescription).join(',')] : []),
+      ...(audioFilters.length > 0
+        ? ['-filter:a', audioFilters.map(escapeFilterDescription).join(',')] : []),
       this.#url,
     ];
   }

src/string.ts

@@ -1,5 +1,7 @@
 /**
- * Escape special characters in FFmpeg
+ * This module handles escaping strings and stringifying JavaScript values into various FFmpeg
+ * syntaxes. All code here is based on the FFmpeg docs or, sometimes, on FFmpeg's C sources.
+ * The following functions are currently not considered part of the public API.
  * @see https://ffmpeg.org/ffmpeg-utils.html#Quoting-and-escaping
  * @see https://ffmpeg.org/ffmpeg-filters.html#Filtergraph-syntax-1
  * @see https://ffmpeg.org/ffmpeg-all.html#concat-1
@@ -8,19 +10,22 @@
 import { isNullish } from './utils';
 
 /**
- * Stringifies a given filter with its options to ffmpeg's filter graph syntax.
- * @param filter The filter's name, {@link VideoFilter} {@link AudioFilter}
- * @param options The filter's options.
+ * Stringify a filter with options into an FFmpeg filter description. `options` is stringified to
+ * a `:`-separated list of `key=value` pairs if object, or to a `:`-separated list of `value`.
+ * Nullish values (`null` or `undefined`) are ignored. `Date` objects are turned into an ISO string,
+ * other non-string values are coerced to a string. All values are escaped using
+ * {@link escapeFilterValue}.
+ *
  * @see https://ffmpeg.org/ffmpeg-filters.html#Filtergraph-syntax-1
  */
 export function stringifyFilterDescription(filter: string, options?: Record<string, any> | any[]) {
   if (isNullish(options))
     return filter;
   if (Array.isArray(options)) {
-    const values = options.filter((value) => !isNullish(value) && value !== '');
+    const values = options.filter((value) => !isNullish(value));
     if (values.length === 0)
       return filter;
-    return `${filter}=${values.map(escapeFilterValue).join(':')}`;
+    return `${filter}=${values.map((value) => escapeFilterValue(stringifyValue(value))).join(':')}`;
   } else {
     const opts = stringifyObjectColonSeparated(options);
     if (opts === '')
@@ -29,25 +34,43 @@ export function stringifyFilterDescription(filter: string, options?: Record<stri
   }
 }
 
+/**
+ * Turn an object into a `:`-separated list of `key=value` pairs. Values which are `null`,
+ * `undefined` or `''` (empty string) are ignored. Values are escaped using {@link escapeFilterValue}.
+ * No checks are applied to keys, they are assumed to be valid in FFmpeg.
+ *
+ * @returns A string containing a list of `:`-separated list of `key=value` pairs, may be `''`
+ * (empty string) if the object is empty or if all of it's values are ignored.
+ */
 export function stringifyObjectColonSeparated(object: Record<string, any>) {
   return Object.entries(object)
     .filter(([, value]) => !isNullish(value) && value !== '')
-    .map(([key, value]) => `${key}=${escapeFilterValue(value)}`)
+    .map(([key, value]) => `${key}=${escapeFilterValue(stringifyValue(value))}`)
     .join(':');
 }
 
-export function escapeConcatFile(s: string) {
-  return ('' + s).replace(/[\\' ]/g, (c: string) => `\\${c}`);
-}
-
-export function escapeTeeComponent(s: string) {
-  return ('' + s).replace(/[\\' |[\]]/g, (c: string) => `\\${c}`);
+/**
+ * Turn an arbitrary JavaScript value `x` to a string, all values but `Date`s are coerced to a
+ * string. `Date` objects are converted to an ISO string (`1970-01-01T00:00:00.000Z`) which is a
+ * valid date format in FFmpeg.
+ * @see https://ffmpeg.org/ffmpeg-utils.html#Date
+ */
+export function stringifyValue(x: any) {
+  return Object.prototype.toString.call(x) === '[object Date]' ? (x as Date).toISOString() : '' + x;
 }
 
 export function escapeFilterValue(s: string) {
-  return ('' + s).replace(/[\\':]/g, (c: string) => `\\${c}`);
+  return ('' + s).replace(/[\\':]/g, (c) => `\\${c}`);
 }
 
 export function escapeFilterDescription(s: string) {
-  return ('' + s).replace(/[\\'[\],;]/g, (c: string) => `\\${c}`);
+  return ('' + s).replace(/[\\'[\],;]/g, (c) => `\\${c}`);
+}
+
+export function escapeConcatFile(s: string) {
+  return ('' + s).replace(/[\\' ]/g, (c) => `\\${c}`);
+}
+
+export function escapeTeeComponent(s: string) {
+  return ('' + s).replace(/[\\' |[\]]/g, (c) => `\\${c}`);
 }

test/string_test.ts

@@ -1,36 +1,59 @@
-import { escapeConcatFile, escapeFilterValue, stringifyFilterDescription } from '../src/string';
+import { escapeConcatFile, escapeFilterValue, escapeTeeComponent, stringifyFilterDescription, stringifyValue } from '../src/string';
 
 describe('string', function () {
-  describe('escapeConcatFile()', function () {
-    it('should escape special characters', function () {
-      const unescaped = "I'm a string;, with many[special]: characters";
-      const escaped = "I\\'m\\ a\\ string;,\\ with\\ many[special]:\\ characters";
-      expect(escapeConcatFile(unescaped)).toBe(escaped);
-    });
+  it('escapeFilterValue()', function () {
+    const unescaped = "I'm a string;, with many[special]: characters";
+    const escaped = "I\\'m a string;, with many[special]\\: characters";
+    expect(escapeFilterValue(unescaped)).toBe(escaped);
+  });
+  it('escapeConcatFile()', function () {
+    const unescaped = "I'm a string;, with many[special]: characters";
+    const escaped = "I\\'m\\ a\\ string;,\\ with\\ many[special]:\\ characters";
+    expect(escapeConcatFile(unescaped)).toBe(escaped);
   });
-  describe('escapeFilterComponent()', function () {
-    it('should escape special characters', function () {
-      const unescaped = "I'm a string;, with many[special]: characters";
-      const escaped = "I\\'m\\ a\\ string\\;\\,\\ with\\ many\\[special\\]\\:\\ characters";
-      expect(escapeFilterValue(unescaped)).toBe(escaped);
+  it('escapeTeeComponent()', function () {
+    const unescaped = "I'm a string;| with many[special]: characters";
+    const escaped = "I\\'m\\ a\\ string;\\|\\ with\\ many\\[special\\]:\\ characters";
+    expect(escapeTeeComponent(unescaped)).toBe(escaped);
+  });
+  describe('stringifyValue()', function () {
+    it('should stringify Date to an ISO string', function () {
+      const s = '1970-01-01T00:00:00.000Z';
+      const date = new Date(s);
+      expect(stringifyValue(date)).toBe(s);
+    });
+    it('should coerce non-matching objects to string', function () {
+      expect(stringifyValue(true)).toBe('true');
+      expect(stringifyValue(1)).toBe('1');
     });
   });
-  describe('stringifySimpleFilterGraph()', function () {
+  describe('stringifyFilterDescription()', function () {
     it('should stringify options array', function () {
       expect(stringifyFilterDescription('my_filter', ['opt1', 'opt2'])).toBe('my_filter=opt1:opt2');
-      expect(stringifyFilterDescription('my_filter', ['opt1:', 'opt2'])).toBe('my_filter=opt1\\::opt2');
+      expect(stringifyFilterDescription('my_filter', [`chars ' which \\ can : cause problems`]))
+        .toBe(`my_filter=chars \\' which \\\\ can \\: cause problems`);
     });
-    it('should stringify options record', function () {
-      expect(stringifyFilterDescription('my_filter', { opt1: 'val1', opt2: 'val2' })).toBe('my_filter=opt1=val1:opt2=val2');
-      expect(stringifyFilterDescription('my_filter', { opt1: 'val1:', opt2: 'val2' })).toBe('my_filter=opt1=val1\\::opt2=val2');
+    it('should stringify options object', function () {
+      expect(stringifyFilterDescription('my_filter', { a: '1', b: '2' })).toBe('my_filter=a=1:b=2');
+      expect(stringifyFilterDescription('my_filter', {
+        a: `chars ' which \\ can : cause problems`,
+      })).toBe(`my_filter=a=chars \\' which \\\\ can \\: cause problems`);
     });
     it('should stringify non-string values', function () {
       expect(stringifyFilterDescription('my_filter', [1, 2])).toBe('my_filter=1:2');
-      expect(stringifyFilterDescription('my_filter', { opt1: 1, opt2: 2 })).toBe('my_filter=opt1=1:opt2=2');
+      expect(stringifyFilterDescription('my_filter', {
+        a: 1,
+        b: 2,
+      })).toBe('my_filter=a=1:b=2');
     });
     it('should stringify empty options', function () {
       expect(stringifyFilterDescription('my_filter', [])).toBe('my_filter');
-      expect(stringifyFilterDescription('my_filter', {})).toBe('my_filter');
+      expect(stringifyFilterDescription('my_filter', [null, void 0, ''])).toBe('my_filter');
+      expect(stringifyFilterDescription('my_filter', {
+        a: null,
+        b: void 0,
+        c: '',
+      })).toBe('my_filter');
       expect(stringifyFilterDescription('my_filter')).toBe('my_filter');
     });
   });