Fix encoding of ampersands in attribute values

lib/index.js

@@ -247,7 +247,9 @@ export function directiveToMarkdown(options) {
         // Additionally, join shortcuts: `#a .b.c d="e"` -> `#a.b.c d="e"`
         if (key === 'id') {
           id =
-            settings.preferShortcut !== false && shortcut.test(value)
+            settings.preferShortcut !== false &&
+            shortcut.test(value) &&
+            encodeAmpersands(value) === value
               ? '#' + value
               : quoted('id', value, node, state)
         } else if (key === 'class') {
@@ -259,7 +261,9 @@ export function directiveToMarkdown(options) {
           let index = -1
 
           while (++index < list.length) {
-            ;(settings.preferShortcut !== false && shortcut.test(list[index])
+            ;(settings.preferShortcut !== false &&
+            shortcut.test(list[index]) &&
+            encodeAmpersands(list[index]) === list[index]
               ? classesList
               : classesFullList
             ).push(list[index])
@@ -308,8 +312,10 @@ export function directiveToMarkdown(options) {
   function quoted(key, value, node, state) {
     if (settings.collapseEmptyAttributes !== false && !value) return key
 
-    if (settings.preferUnquoted && unquoted.test(value)) {
-      return key + '=' + value
+    const encoded = encodeAmpersands(value)
+
+    if (settings.preferUnquoted && unquoted.test(encoded)) {
+      return key + '=' + encoded
     }
 
     // If the alternative is less common than `quote`, switch.
@@ -330,7 +336,7 @@ export function directiveToMarkdown(options) {
       key +
       '=' +
       appliedQuote +
-      stringifyEntitiesLight(value, {subset}) +
+      stringifyEntitiesLight(encoded, {subset}) +
       appliedQuote
     )
   }
@@ -560,6 +566,36 @@ function inlineDirectiveLabel(node) {
   )
 }
 
+/**
+ * Encode the ampersands that start character references in an attribute value,
+ * so that they are not decoded when parsed.
+ *
+ * @param {string} value
+ *   Attribute value.
+ * @returns {string}
+ *   Attribute value with those ampersands encoded.
+ */
+function encodeAmpersands(value) {
+  if (!value.includes('&')) return value
+
+  /** @type {Array<string>} */
+  const result = []
+  let start = 0
+
+  parseEntities(value, {
+    attribute: true,
+    reference(_, position) {
+      const {offset} = position.start
+      assert(offset !== undefined, 'expected `offset`')
+      result.push(value.slice(start, offset), '&#x26;')
+      start = offset + 1
+    }
+  })
+
+  result.push(value.slice(start))
+  return result.join('')
+}
+
 /**
  * Determine the fence size for a directive node.
  *

test.js

@@ -616,6 +616,64 @@ test('directiveToMarkdown()', async function (t) {
     }
   )
 
+  await t.test(
+    'should encode the `&` of character references in attribute values',
+    async function () {
+      assert.equal(
+        toMarkdown(
+          {
+            type: 'textDirective',
+            name: 'i',
+            attributes: {title: 'a & b & c &copy d'},
+            children: []
+          },
+          {extensions: [directiveToMarkdown()]}
+        ),
+        ':i{title="a & b & c &copy d"}\n'
+      )
+    }
+  )
+
+  await t.test(
+    'should not encode other `&` in attribute values',
+    async function () {
+      assert.equal(
+        toMarkdown(
+          {
+            type: 'textDirective',
+            name: 'i',
+            attributes: {title: 'a & b&c &copy=d &unknown;'},
+            children: []
+          },
+          {extensions: [directiveToMarkdown()]}
+        ),
+        ':i{title="a & b&c &copy=d &unknown;"}\n'
+      )
+    }
+  )
+
+  await t.test(
+    'should roundtrip character references in attribute values',
+    async function () {
+      const tree = fromMarkdown(
+        'a :b{#c&d .e<f g="😀\r\n& "} h.',
+        {
+          extensions: [directive()],
+          mdastExtensions: [directiveFromMarkdown()]
+        }
+      )
+      const result = fromMarkdown(
+        toMarkdown(tree, {extensions: [directiveToMarkdown()]}),
+        {extensions: [directive()], mdastExtensions: [directiveFromMarkdown()]}
+      )
+
+      removePosition(tree, {force: true})
+      removePosition(result, {force: true})
+
+      assert.deepEqual(result, tree)
+    }
+  )
+
   await t.test(
     'should not use the `id` shortcut if impossible characters exist',
     async function () {
@@ -666,6 +724,42 @@ test('directiveToMarkdown()', async function (t) {
     }
   )
 
+  await t.test(
+    'should not use the `id` shortcut for character references',
+    async function () {
+      assert.equal(
+        toMarkdown(
+          {
+            type: 'textDirective',
+            name: 'i',
+            attributes: {id: 'a&b'},
+            children: []
+          },
+          {extensions: [directiveToMarkdown()]}
+        ),
+        ':i{id="a&b"}\n'
+      )
+    }
+  )
+
+  await t.test(
+    'should not use the `class` shortcut for character references',
+    async function () {
+      assert.equal(
+        toMarkdown(
+          {
+            type: 'textDirective',
+            name: 'i',
+            attributes: {class: 'a b<c'},
+            children: []
+          },
+          {extensions: [directiveToMarkdown()]}
+        ),
+        ':i{.a class="b<c"}\n'
+      )
+    }
+  )
+
   await t.test(
     'should not use the `class` shortcut if impossible characters exist (but should use it for classes that don’t)',
     async function () {
@@ -1468,6 +1562,24 @@ test('directiveToMarkdown()', async function (t) {
       }
     )
 
+    await t.test(
+      'should encode the `&` of character references in `preferUnquoted`',
+      async function () {
+        assert.equal(
+          toMarkdown(
+            {
+              type: 'textDirective',
+              name: 'i',
+              attributes: {title: 'a&b'},
+              children: []
+            },
+            {extensions: [directiveToMarkdown({preferUnquoted: true})]}
+          ),
+          ':i{title=a&b}\n'
+        )
+      }
+    )
+
     await t.test(
       'should not add `=` when omitting quotes on empty values',
       async function () {