refactor: improve feature string parsing (#23130)

* test: add pre-change snapshot of new-window event

* move to .ts file for easier diff

* refactor: improve feature string parsing logic

* test: update snapshots

* update type names per review

* update comma-separated parse test

* use for loop instead of reduce per review

* tighten up types

* avoid variable guest contents id returnValue in test snapshot

filenames.auto.gni

@@ -274,7 +274,7 @@ auto_filenames = {
     "lib/common/define-properties.ts",
     "lib/common/electron-binding-setup.ts",
     "lib/common/init.ts",
-    "lib/common/parse-features-string.js",
+    "lib/common/parse-features-string.ts",
     "lib/common/reset-search-paths.ts",
     "lib/common/type-utils.ts",
     "lib/common/web-view-methods.ts",

lib/browser/api/web-contents.js

@@ -11,7 +11,7 @@ const { internalWindowOpen } = require('@electron/internal/browser/guest-window-
 const NavigationController = require('@electron/internal/browser/navigation-controller');
 const { ipcMainInternal } = require('@electron/internal/browser/ipc-main-internal');
 const ipcMainUtils = require('@electron/internal/browser/ipc-main-internal-utils');
-const { convertFeaturesString } = require('@electron/internal/common/parse-features-string');
+const { parseFeatures } = require('@electron/internal/common/parse-features-string');
 const { MessagePortMain } = require('@electron/internal/browser/message-port-main');
 
 // session is not used here, the purpose is to make sure session is initalized
@@ -511,11 +511,13 @@ WebContents.prototype._init = function () {
     // Make new windows requested by links behave like "window.open".
     this.on('-new-window', (event, url, frameName, disposition,
       rawFeatures, referrer, postData) => {
-      const { options, additionalFeatures } = convertFeaturesString(rawFeatures, frameName);
+      const { options, webPreferences, additionalFeatures } = parseFeatures(rawFeatures);
       const mergedOptions = {
         show: true,
         width: 800,
         height: 600,
+        title: frameName,
+        webPreferences,
         ...options
       };
 
@@ -533,12 +535,14 @@ WebContents.prototype._init = function () {
         return;
       }
 
-      const { options, additionalFeatures } = convertFeaturesString(rawFeatures, frameName);
+      const { options, webPreferences, additionalFeatures } = parseFeatures(rawFeatures);
       const mergedOptions = {
         show: true,
         width: 800,
         height: 600,
         webContents,
+        title: frameName,
+        webPreferences,
         ...options
       };
 

lib/browser/guest-view-manager.js

@@ -3,7 +3,7 @@
 const { webContents } = require('electron');
 const { ipcMainInternal } = require('@electron/internal/browser/ipc-main-internal');
 const ipcMainUtils = require('@electron/internal/browser/ipc-main-internal-utils');
-const { parseFeaturesString } = require('@electron/internal/common/parse-features-string');
+const { parseWebViewWebPreferences } = require('@electron/internal/common/parse-features-string');
 const { syncMethods, asyncMethods, properties } = require('@electron/internal/common/web-view-methods');
 const { serialize } = require('@electron/internal/common/type-utils');
 
@@ -184,6 +184,13 @@ const attachGuest = function (event, embedderFrameId, elementInstanceId, guestIn
     }
   }
 
+  // parse the 'webpreferences' attribute string, if set
+  // this uses the same parsing rules as window.open uses for its features
+  const parsedWebPreferences =
+    typeof params.webpreferences === 'string'
+      ? parseWebViewWebPreferences(params.webpreferences)
+      : null;
+
   const webPreferences = {
     guestInstanceId: guestInstanceId,
     nodeIntegration: params.nodeintegration != null ? params.nodeintegration : false,
@@ -194,21 +201,10 @@ const attachGuest = function (event, embedderFrameId, elementInstanceId, guestIn
     disablePopups: !params.allowpopups,
     webSecurity: !params.disablewebsecurity,
     enableBlinkFeatures: params.blinkfeatures,
-    disableBlinkFeatures: params.disableblinkfeatures
+    disableBlinkFeatures: params.disableblinkfeatures,
+    ...parsedWebPreferences
   };
 
-  // parse the 'webpreferences' attribute string, if set
-  // this uses the same parsing rules as window.open uses for its features
-  if (typeof params.webpreferences === 'string') {
-    parseFeaturesString(params.webpreferences, function (key, value) {
-      if (value === undefined) {
-        // no value was specified, default it to true
-        value = true;
-      }
-      webPreferences[key] = value;
-    });
-  }
-
   if (params.preload) {
     webPreferences.preloadURL = params.preload;
   }

lib/browser/guest-window-manager.js

@@ -5,7 +5,7 @@ const { BrowserWindow } = electron;
 const { isSameOrigin } = process.electronBinding('v8_util');
 const { ipcMainInternal } = require('@electron/internal/browser/ipc-main-internal');
 const ipcMainUtils = require('@electron/internal/browser/ipc-main-internal-utils');
-const { convertFeaturesString } = require('@electron/internal/common/parse-features-string');
+const { parseFeatures } = require('@electron/internal/common/parse-features-string');
 
 const hasProp = {}.hasOwnProperty;
 const frameToGuest = new Map();
@@ -224,7 +224,10 @@ ipcMainInternal.on('ELECTRON_GUEST_WINDOW_MANAGER_WINDOW_OPEN', (event, url, fra
   if (features == null) features = '';
 
   const disposition = 'new-window';
-  const { options, additionalFeatures } = convertFeaturesString(features, frameName);
+  const { options, webPreferences, additionalFeatures } = parseFeatures(features);
+  if (!options.title) options.title = frameName;
+  options.webPreferences = webPreferences;
+
   const referrer = { url: '', policy: 'default' };
   internalWindowOpen(event, url, referrer, frameName, disposition, options, additionalFeatures, null);
 });

lib/common/parse-features-string.js

@@ -1,85 +0,0 @@
-'use strict';
-
-// parses a feature string that has the format used in window.open()
-// - `features` input string
-// - `emit` function(key, value) - called for each parsed KV
-function parseFeaturesString (features, emit) {
-  features = `${features}`.trim();
-  // split the string by ','
-  features.split(/\s*,\s*/).forEach((feature) => {
-    // expected form is either a key by itself or a key/value pair in the form of
-    // 'key=value'
-    let [key, value] = feature.split(/\s*=\s*/);
-    if (!key) return;
-
-    // interpret the value as a boolean, if possible
-    value = (value === 'yes' || value === '1') ? true : (value === 'no' || value === '0') ? false : value;
-
-    // emit the parsed pair
-    emit(key, value);
-  });
-}
-
-function convertFeaturesString (features, frameName) {
-  const options = {};
-
-  const ints = ['x', 'y', 'width', 'height', 'minWidth', 'maxWidth', 'minHeight', 'maxHeight', 'zoomFactor'];
-  const webPreferences = ['zoomFactor', 'nodeIntegration', 'enableRemoteModule', 'preload', 'javascript', 'contextIsolation', 'webviewTag'];
-
-  // Used to store additional features
-  const additionalFeatures = [];
-
-  // Parse the features
-  parseFeaturesString(features, function (key, value) {
-    if (value === undefined) {
-      additionalFeatures.push(key);
-    } else {
-      // Don't allow webPreferences to be set since it must be an object
-      // that cannot be directly overridden
-      if (key === 'webPreferences') return;
-
-      if (webPreferences.includes(key)) {
-        if (options.webPreferences == null) {
-          options.webPreferences = {};
-        }
-        options.webPreferences[key] = value;
-      } else {
-        options[key] = value;
-      }
-    }
-  });
-
-  if (options.left) {
-    if (options.x == null) {
-      options.x = options.left;
-    }
-  }
-  if (options.top) {
-    if (options.y == null) {
-      options.y = options.top;
-    }
-  }
-  if (options.title == null) {
-    options.title = frameName;
-  }
-  if (options.width == null) {
-    options.width = 800;
-  }
-  if (options.height == null) {
-    options.height = 600;
-  }
-
-  for (const name of ints) {
-    if (options[name] != null) {
-      options[name] = parseInt(options[name], 10);
-    }
-  }
-
-  return {
-    options, additionalFeatures
-  };
-}
-
-module.exports = {
-  parseFeaturesString, convertFeaturesString
-};

lib/common/parse-features-string.ts

@@ -0,0 +1,110 @@
+/**
+ * Utilities to parse comma-separated key value pairs used in browser APIs.
+ * For example: "x=100,y=200,width=500,height=500"
+ */
+import { BrowserWindowConstructorOptions } from 'electron';
+
+type RequiredBrowserWindowConstructorOptions = Required<BrowserWindowConstructorOptions>;
+type IntegerBrowserWindowOptionKeys = {
+  [K in keyof RequiredBrowserWindowConstructorOptions]:
+    RequiredBrowserWindowConstructorOptions[K] extends number ? K : never
+}[keyof RequiredBrowserWindowConstructorOptions];
+
+// This could be an array of keys, but an object allows us to add a compile-time
+// check validating that we haven't added an integer property to
+// BrowserWindowConstructorOptions that this module doesn't know about.
+const keysOfTypeNumberCompileTimeCheck: { [K in IntegerBrowserWindowOptionKeys] : true } = {
+  x: true,
+  y: true,
+  width: true,
+  height: true,
+  minWidth: true,
+  maxWidth: true,
+  minHeight: true,
+  maxHeight: true,
+  opacity: true
+};
+// Note `top` / `left` are special cases from the browser which we later convert
+// to y / x.
+const keysOfTypeNumber = ['top', 'left', ...Object.keys(keysOfTypeNumberCompileTimeCheck)];
+
+/**
+ * Note that we only allow "0" and "1" boolean conversion when the type is known
+ * not to be an integer.
+ *
+ * The coercion of yes/no/1/0 represents best effort accordance with the spec:
+ * https://html.spec.whatwg.org/multipage/window-object.html#concept-window-open-features-parse-boolean
+ */
+type CoercedValue = string | number | boolean;
+function coerce (key: string, value: string): CoercedValue {
+  if (keysOfTypeNumber.includes(key)) {
+    return Number(value);
+  }
+
+  switch (value) {
+    case 'true':
+    case '1':
+    case 'yes':
+    case undefined:
+      return true;
+    case 'false':
+    case '0':
+    case 'no':
+      return false;
+    default:
+      return value;
+  }
+}
+
+export function parseCommaSeparatedKeyValue (source: string, useSoonToBeDeprecatedBehaviorForBareKeys: boolean) {
+  const bareKeys = [] as string[];
+  const parsed = {} as { [key: string]: any };
+  for (const keyValuePair of source.split(',')) {
+    const [key, value] = keyValuePair.split('=').map(str => str.trim());
+    if (useSoonToBeDeprecatedBehaviorForBareKeys && value === undefined) {
+      bareKeys.push(key);
+      continue;
+    }
+    parsed[key] = coerce(key, value);
+  }
+
+  return { parsed, bareKeys };
+}
+
+export function parseWebViewWebPreferences (preferences: string) {
+  return parseCommaSeparatedKeyValue(preferences, false).parsed;
+}
+
+const allowedWebPreferences = ['zoomFactor', 'nodeIntegration', 'enableRemoteModule', 'preload', 'javascript', 'contextIsolation', 'webviewTag'] as const;
+type AllowedWebPreference = (typeof allowedWebPreferences)[number];
+
+/**
+ * Parses a feature string that has the format used in window.open().
+ *
+ * `useSoonToBeDeprecatedBehaviorForBareKeys` — In the html spec, windowFeatures keys
+ * without values are interpreted as `true`. Previous versions of Electron did
+ * not respect this. In order to not break any applications, this will be
+ * flipped in the next major version.
+ */
+export function parseFeatures (
+  features: string,
+  useSoonToBeDeprecatedBehaviorForBareKeys: boolean = true
+) {
+  const { parsed, bareKeys } = parseCommaSeparatedKeyValue(features, useSoonToBeDeprecatedBehaviorForBareKeys);
+
+  const webPreferences: { [K in AllowedWebPreference]?: any } = {};
+  allowedWebPreferences.forEach((key) => {
+    if (parsed[key] === undefined) return;
+    webPreferences[key] = parsed[key];
+    delete parsed[key];
+  });
+
+  if (parsed.left !== undefined) parsed.x = parsed.left;
+  if (parsed.top !== undefined) parsed.y = parsed.top;
+
+  return {
+    options: parsed as Omit<BrowserWindowConstructorOptions, 'webPreferences'> & { [key: string]: CoercedValue },
+    webPreferences,
+    additionalFeatures: bareKeys
+  };
+}

spec-main/fixtures/snapshots/native-window-open.snapshot.txt

@@ -0,0 +1,184 @@
+[
+  [
+    "top=5,left=10,resizable=no",
+    {
+      "sender": "[WebContents]",
+      "returnValue": "placeholder-guest-contents-id"
+    },
+    "about:blank",
+    "frame name",
+    "new-window",
+    {
+      "show": true,
+      "width": 800,
+      "height": 600,
+      "webContents": "[WebContents]",
+      "title": "frame name",
+      "webPreferences": {
+        "nativeWindowOpen": true,
+        "sandbox": true,
+        "backgroundColor": "blue",
+        "nodeIntegration": false,
+        "webviewTag": false,
+        "nodeIntegrationInSubFrames": false
+      },
+      "top": 5,
+      "left": 10,
+      "resizable": false,
+      "x": 10,
+      "y": 5,
+      "backgroundColor": "blue",
+      "focusable": false
+    },
+    [],
+    {
+      "url": "",
+      "policy": "no-referrer-when-downgrade"
+    },
+    null
+  ],
+  [
+    "zoomFactor=2,resizable=0,x=0,y=10",
+    {
+      "sender": "[WebContents]",
+      "returnValue": "placeholder-guest-contents-id"
+    },
+    "about:blank",
+    "frame name",
+    "new-window",
+    {
+      "show": true,
+      "width": 800,
+      "height": 600,
+      "webContents": "[WebContents]",
+      "title": "frame name",
+      "webPreferences": {
+        "zoomFactor": "2",
+        "nativeWindowOpen": true,
+        "sandbox": true,
+        "backgroundColor": "blue",
+        "nodeIntegration": false,
+        "webviewTag": false,
+        "nodeIntegrationInSubFrames": false
+      },
+      "resizable": false,
+      "x": 0,
+      "y": 10,
+      "backgroundColor": "blue",
+      "focusable": false
+    },
+    [],
+    {
+      "url": "",
+      "policy": "no-referrer-when-downgrade"
+    },
+    null
+  ],
+  [
+    "backgroundColor=gray,webPreferences=0,x=100,y=100",
+    {
+      "sender": "[WebContents]",
+      "returnValue": "placeholder-guest-contents-id"
+    },
+    "about:blank",
+    "frame name",
+    "new-window",
+    {
+      "show": true,
+      "width": 800,
+      "height": 600,
+      "webContents": "[WebContents]",
+      "title": "frame name",
+      "webPreferences": {
+        "nativeWindowOpen": true,
+        "sandbox": true,
+        "backgroundColor": "gray",
+        "nodeIntegration": false,
+        "webviewTag": false,
+        "nodeIntegrationInSubFrames": false
+      },
+      "backgroundColor": "gray",
+      "x": 100,
+      "y": 100,
+      "focusable": false
+    },
+    [],
+    {
+      "url": "",
+      "policy": "no-referrer-when-downgrade"
+    },
+    null
+  ],
+  [
+    "x=50,y=20,title=sup",
+    {
+      "sender": "[WebContents]",
+      "returnValue": "placeholder-guest-contents-id"
+    },
+    "about:blank",
+    "frame name",
+    "new-window",
+    {
+      "show": true,
+      "width": 800,
+      "height": 600,
+      "webContents": "[WebContents]",
+      "title": "sup",
+      "webPreferences": {
+        "nativeWindowOpen": true,
+        "sandbox": true,
+        "backgroundColor": "blue",
+        "nodeIntegration": false,
+        "webviewTag": false,
+        "nodeIntegrationInSubFrames": false
+      },
+      "x": 50,
+      "y": 20,
+      "backgroundColor": "blue",
+      "focusable": false
+    },
+    [],
+    {
+      "url": "",
+      "policy": "no-referrer-when-downgrade"
+    },
+    null
+  ],
+  [
+    "show=false,top=1,left=1",
+    {
+      "sender": "[WebContents]",
+      "returnValue": "placeholder-guest-contents-id"
+    },
+    "about:blank",
+    "frame name",
+    "new-window",
+    {
+      "show": false,
+      "width": 800,
+      "height": 600,
+      "webContents": "[WebContents]",
+      "title": "frame name",
+      "webPreferences": {
+        "nativeWindowOpen": true,
+        "sandbox": true,
+        "backgroundColor": "blue",
+        "nodeIntegration": false,
+        "webviewTag": false,
+        "nodeIntegrationInSubFrames": false
+      },
+      "top": 1,
+      "left": 1,
+      "x": 1,
+      "y": 1,
+      "backgroundColor": "blue",
+      "focusable": false
+    },
+    [],
+    {
+      "url": "",
+      "policy": "no-referrer-when-downgrade"
+    },
+    null
+  ]
+]

spec-main/fixtures/snapshots/proxy-window-open.snapshot.txt

@@ -0,0 +1,161 @@
+[
+  [
+    "top=5,left=10,resizable=no",
+    {
+      "sender": "[WebContents]",
+      "frameId": 1
+    },
+    "about:blank",
+    "frame name",
+    "new-window",
+    {
+      "top": 5,
+      "left": 10,
+      "resizable": false,
+      "x": 10,
+      "y": 5,
+      "title": "frame name",
+      "webPreferences": {
+        "nodeIntegration": false,
+        "webviewTag": false,
+        "nodeIntegrationInSubFrames": false,
+        "openerId": "placeholder-opener-id"
+      },
+      "width": 800,
+      "height": 600,
+      "show": false
+    },
+    [],
+    {
+      "url": "",
+      "policy": "default"
+    },
+    null
+  ],
+  [
+    "zoomFactor=2,resizable=0,x=0,y=10",
+    {
+      "sender": "[WebContents]",
+      "frameId": 1
+    },
+    "about:blank",
+    "frame name",
+    "new-window",
+    {
+      "resizable": false,
+      "x": 0,
+      "y": 10,
+      "title": "frame name",
+      "webPreferences": {
+        "zoomFactor": "2",
+        "nodeIntegration": false,
+        "webviewTag": false,
+        "nodeIntegrationInSubFrames": false,
+        "openerId": "placeholder-opener-id"
+      },
+      "width": 800,
+      "height": 600,
+      "show": false
+    },
+    [],
+    {
+      "url": "",
+      "policy": "default"
+    },
+    null
+  ],
+  [
+    "backgroundColor=gray,webPreferences=0,x=100,y=100",
+    {
+      "sender": "[WebContents]",
+      "frameId": 1
+    },
+    "about:blank",
+    "frame name",
+    "new-window",
+    {
+      "backgroundColor": "gray",
+      "webPreferences": {
+        "nodeIntegration": false,
+        "webviewTag": false,
+        "nodeIntegrationInSubFrames": false,
+        "openerId": "placeholder-opener-id",
+        "backgroundColor": "gray"
+      },
+      "x": 100,
+      "y": 100,
+      "title": "frame name",
+      "width": 800,
+      "height": 600,
+      "show": false
+    },
+    [],
+    {
+      "url": "",
+      "policy": "default"
+    },
+    null
+  ],
+  [
+    "x=50,y=20,title=sup",
+    {
+      "sender": "[WebContents]",
+      "frameId": 1
+    },
+    "about:blank",
+    "frame name",
+    "new-window",
+    {
+      "x": 50,
+      "y": 20,
+      "title": "sup",
+      "webPreferences": {
+        "nodeIntegration": false,
+        "webviewTag": false,
+        "nodeIntegrationInSubFrames": false,
+        "openerId": "placeholder-opener-id"
+      },
+      "width": 800,
+      "height": 600,
+      "show": false
+    },
+    [],
+    {
+      "url": "",
+      "policy": "default"
+    },
+    null
+  ],
+  [
+    "show=false,top=1,left=1",
+    {
+      "sender": "[WebContents]",
+      "frameId": 1
+    },
+    "about:blank",
+    "frame name",
+    "new-window",
+    {
+      "show": false,
+      "top": 1,
+      "left": 1,
+      "x": 1,
+      "y": 1,
+      "title": "frame name",
+      "webPreferences": {
+        "nodeIntegration": false,
+        "webviewTag": false,
+        "nodeIntegrationInSubFrames": false,
+        "openerId": "placeholder-opener-id"
+      },
+      "width": 800,
+      "height": 600
+    },
+    [],
+    {
+      "url": "",
+      "policy": "default"
+    },
+    null
+  ]
+]

spec-main/guest-window-manager-spec.ts

@@ -0,0 +1,109 @@
+import { BrowserWindow } from 'electron';
+import { writeFileSync, readFileSync } from 'fs';
+import { resolve } from 'path';
+import { expect } from 'chai';
+import { closeAllWindows } from './window-helpers';
+
+function genSnapshot (browserWindow: BrowserWindow, features: string) {
+  return new Promise((resolve) => {
+    browserWindow.webContents.on('new-window', (...args: any[]) => {
+      resolve([features, ...args]);
+    });
+    browserWindow.webContents.executeJavaScript(`window.open('about:blank', 'frame name', '${features}')`);
+  });
+}
+
+describe('new-window event', () => {
+  const testConfig = {
+    native: {
+      snapshotFileName: 'native-window-open.snapshot.txt',
+      browserWindowOptions: {
+        show: false,
+        width: 200,
+        title: 'cool',
+        backgroundColor: 'blue',
+        focusable: false,
+        webPreferences: {
+          nativeWindowOpen: true,
+          sandbox: true
+        }
+      }
+    },
+    proxy: {
+      snapshotFileName: 'proxy-window-open.snapshot.txt',
+      browserWindowOptions: {
+        show: false
+      }
+    }
+  };
+
+  for (const testName of Object.keys(testConfig) as (keyof typeof testConfig)[]) {
+    const { snapshotFileName, browserWindowOptions } = testConfig[testName];
+
+    describe(`for ${testName} window opening`, () => {
+      const snapshotFile = resolve(__dirname, 'fixtures', 'snapshots', snapshotFileName);
+      let browserWindow: BrowserWindow;
+      let existingSnapshots: any[];
+
+      before(() => {
+        existingSnapshots = parseSnapshots(readFileSync(snapshotFile, { encoding: 'utf8' }));
+      });
+
+      beforeEach((done) => {
+        browserWindow = new BrowserWindow(browserWindowOptions);
+        browserWindow.loadURL('about:blank');
+        browserWindow.on('ready-to-show', () => done());
+      });
+
+      afterEach(closeAllWindows);
+
+      const newSnapshots: any[] = [];
+      [
+        'top=5,left=10,resizable=no',
+        'zoomFactor=2,resizable=0,x=0,y=10',
+        'backgroundColor=gray,webPreferences=0,x=100,y=100',
+        'x=50,y=20,title=sup',
+        'show=false,top=1,left=1'
+      ].forEach((features, index) => {
+        /**
+         * ATTN: If this test is failing, you likely just need to change
+         * `shouldOverwriteSnapshot` to true and then evaluate the snapshot diff
+         * to see if the change is harmless.
+         */
+        it(`matches snapshot for ${features}`, async () => {
+          const newSnapshot = await genSnapshot(browserWindow, features);
+          newSnapshots.push(newSnapshot);
+          // TODO: The output when these fail could be friendlier.
+          expect(stringifySnapshots(newSnapshot)).to.equal(stringifySnapshots(existingSnapshots[index]));
+        });
+      });
+
+      after(() => {
+        const shouldOverwriteSnapshot = false;
+        if (shouldOverwriteSnapshot) writeFileSync(snapshotFile, stringifySnapshots(newSnapshots, true));
+      });
+    });
+  }
+});
+
+function stringifySnapshots (snapshots: any, pretty = false) {
+  return JSON.stringify(snapshots, (key, value) => {
+    if (['sender', 'webContents'].includes(key)) {
+      return '[WebContents]';
+    }
+    if (key === 'openerId' && typeof value === 'number') {
+      return 'placeholder-opener-id';
+    }
+    if (key === 'returnValue') {
+      return 'placeholder-guest-contents-id';
+    }
+    return value;
+  }, pretty ? 2 : undefined);
+}
+
+function parseSnapshots (snapshotsJson: string) {
+  return JSON.parse(snapshotsJson, (key, value) => {
+    if (key === 'openerId' && value === 'placeholder-opener-id') return 1;
+    return value;
+  });
+}

spec-main/internal-spec.ts

@@ -2,10 +2,9 @@ import { expect } from 'chai';
 
 describe('feature-string parsing', () => {
   it('is indifferent to whitespace around keys and values', () => {
-    const { parseFeaturesString } = require('../lib/common/parse-features-string');
+    const { parseCommaSeparatedKeyValue } = require('../lib/common/parse-features-string');
     const checkParse = (string: string, parsed: Record<string, string | boolean>) => {
-      const features: Record<string, string | boolean> = {};
-      parseFeaturesString(string, (k: string, v: any) => { features[k] = v; });
+      const features = parseCommaSeparatedKeyValue(string, true).parsed;
       expect(features).to.deep.equal(parsed);
     };
     checkParse('a=yes,c=d', { a: true, c: 'd' });