Fix to allow * attributes too if specific also specified

The attributes schema allows both specific tag names (say, `a`) and a
generic catch all (`*`).
Previously, if a strict specific schema was allowed (such as that only
`/^language-./` classes are allowed on `code`), and someone allowed all
classes on the `*` wildcard, then this latter intent was not honored.
Now, it works, either works.

Closes GH-27.
Closes GH-28.

Author: wooorm

lib/index.js

@@ -363,7 +363,7 @@ function element(state, unsafe) {
   let safeElement = false
 
   if (
-    name.length > 0 &&
+    name &&
     name !== '*' &&
     (!state.schema.tagNames || state.schema.tagNames.includes(name))
   ) {
@@ -511,21 +511,20 @@ function properties(state, properties) {
 
   for (key in props) {
     if (own.call(props, key)) {
-      /** @type {Readonly<PropertyDefinition> | undefined} */
-      let definition
-
-      if (specific) definition = findDefinition(specific, key)
-      if (!definition && defaults) definition = findDefinition(defaults, key)
-
-      if (definition) {
-        const unsafe = props[key]
-        const safe = Array.isArray(unsafe)
-          ? propertyValues(state, definition, key, unsafe)
-          : propertyValue(state, definition, key, unsafe)
+      const unsafe = props[key]
+      let safe = propertyValue(
+        state,
+        findDefinition(specific, key),
+        key,
+        unsafe
+      )
+
+      if (safe === null || safe === undefined) {
+        safe = propertyValue(state, findDefinition(defaults, key), key, unsafe)
+      }
 
-        if (safe !== null && safe !== undefined) {
-          result[key] = safe
-        }
+      if (safe !== null && safe !== undefined) {
+        result[key] = safe
       }
     }
   }
@@ -543,6 +542,28 @@ function properties(state, properties) {
   return result
 }
 
+/**
+ * Sanitize a property value.
+ *
+ * @param {State} state
+ *   Info passed around.
+ * @param {Readonly<PropertyDefinition> | undefined} definition
+ *   Definition.
+ * @param {string} key
+ *   Field name.
+ * @param {Readonly<unknown>} value
+ *   Unsafe value (but an array).
+ * @returns {Array<number | string> | boolean | number | string | undefined}
+ *   Safe value.
+ */
+function propertyValue(state, definition, key, value) {
+  return definition
+    ? Array.isArray(value)
+      ? propertyValueMany(state, definition, key, value)
+      : propertyValuePrimitive(state, definition, key, value)
+    : undefined
+}
+
 /**
  * Sanitize a property value which is a list.
  *
@@ -557,13 +578,13 @@ function properties(state, properties) {
  * @returns {Array<number | string>}
  *   Safe value.
  */
-function propertyValues(state, definition, key, values) {
+function propertyValueMany(state, definition, key, values) {
   let index = -1
   /** @type {Array<number | string>} */
   const result = []
 
   while (++index < values.length) {
-    const value = propertyValue(state, definition, key, values[index])
+    const value = propertyValuePrimitive(state, definition, key, values[index])
 
     if (typeof value === 'number' || typeof value === 'string') {
       result.push(value)
@@ -574,7 +595,7 @@ function propertyValues(state, definition, key, values) {
 }
 
 /**
- * Sanitize a property value.
+ * Sanitize a property value which is a primitive.
  *
  * @param {State} state
  *   Info passed around.
@@ -587,7 +608,7 @@ function propertyValues(state, definition, key, values) {
  * @returns {boolean | number | string | undefined}
  *   Safe value.
  */
-function propertyValue(state, definition, key, value) {
+function propertyValuePrimitive(state, definition, key, value) {
   if (
     typeof value !== 'boolean' &&
     typeof value !== 'number' &&
@@ -713,7 +734,7 @@ function patch(node, unsafe) {
 
 /**
  *
- * @param {Readonly<Array<PropertyDefinition>>} definitions
+ * @param {Readonly<Array<PropertyDefinition>> | undefined} definitions
  * @param {string} key
  * @returns {Readonly<PropertyDefinition> | undefined}
  */
@@ -722,15 +743,17 @@ function findDefinition(definitions, key) {
   let dataDefault
   let index = -1
 
-  while (++index < definitions.length) {
-    const entry = definitions[index]
-    const name = typeof entry === 'string' ? entry : entry[0]
+  if (definitions) {
+    while (++index < definitions.length) {
+      const entry = definitions[index]
+      const name = typeof entry === 'string' ? entry : entry[0]
 
-    if (name === key) {
-      return entry
-    }
+      if (name === key) {
+        return entry
+      }
 
-    if (name === 'data*') dataDefault = entry
+      if (name === 'data*') dataDefault = entry
+    }
   }
 
   if (key.length > 4 && key.slice(0, 4).toLowerCase() === 'data') {

lib/schema.js

@@ -34,11 +34,8 @@ export const defaultSchema = {
     del: ['cite'],
     div: ['itemScope', 'itemType'],
     dl: [...aria],
-    // Note: these 2 are used by GFM footnotes, they *sometimes* work.
-    h2: [
-      ['id', 'footnote-label'],
-      ['className', 'sr-only']
-    ],
+    // Note: this is used by GFM footnotes.
+    h2: [['className', 'sr-only']],
     img: [...aria, 'longDesc', 'src'],
     // Note: `input` is not normally allowed by GH, when manually writing
     // it in markdown, they add it from tasklists some other way.
@@ -89,7 +86,10 @@ export const defaultSchema = {
       'coords',
       'dateTime',
       'dir',
-      'disabled',
+      // Note: `disabled` is technically allowed on all elements by GH.
+      // But it is useless on everything except `input`.
+      // Because `input`s are normally not allowed, but we allow them for
+      // checkboxes due to tasklists, we allow `disabled` only there.
       'encType',
       'frame',
       'hSpace',

test/index.js

@@ -9,6 +9,34 @@ import {u} from 'unist-builder'
 const own = {}.hasOwnProperty
 
 test('sanitize()', async function (t) {
+  await t.test('check', async function () {
+    const attributes = defaultSchema.attributes
+    assert(attributes)
+
+    const generic = new Set(
+      attributes['*'].map((d) => (typeof d === 'string' ? d : d[0]))
+    )
+
+    for (const tagName in attributes) {
+      if (tagName !== '*' && Object.hasOwn(attributes, tagName)) {
+        const allowed = attributes[tagName]
+
+        for (const attribute of allowed) {
+          const name = typeof attribute === 'string' ? attribute : attribute[0]
+
+          assert(
+            generic.has(name) === false,
+            'unexpected attribute `' +
+              name +
+              '` in both `*` and `' +
+              tagName +
+              '`, they conflict, please use one or the other'
+          )
+        }
+      }
+    }
+  })
+
   await t.test('should expose the public api', async function () {
     assert.deepEqual(Object.keys(await import('hast-util-sanitize')).sort(), [
       'defaultSchema',
@@ -739,6 +767,47 @@ test('`element`', async function (t) {
       )
     }
   )
+
+  await t.test(
+    'should allow specific values allowed by either a specific or generic attribute definition (1)',
+    async function () {
+      assert.deepEqual(
+        sanitize(
+          h(null, [h('a', {x: 'y'}), h('a', {x: 'z'}), h('a', {x: 'w'})]),
+          {...defaultSchema, attributes: {a: [['x', 'y']], '*': [['x', 'z']]}}
+        ),
+        h(null, [h('a', {x: 'y'}), h('a', {x: 'z'}), h('a')])
+      )
+    }
+  )
+
+  await t.test(
+    'should allow specific values allowed by either a specific or generic attribute definition (2)',
+    async function () {
+      assert.deepEqual(
+        sanitize(
+          h(null, [h('a', {x: 'y'}), h('a', {x: 'z'}), h('a', {x: 'w'})]),
+          // `x` on `*` can contain anything:
+          {...defaultSchema, attributes: {a: ['x'], '*': [['x', 'z']]}}
+        ),
+        h(null, [h('a', {x: 'y'}), h('a', {x: 'z'}), h('a', {x: 'w'})])
+      )
+    }
+  )
+
+  await t.test(
+    'should allow specific values allowed by either a specific or generic attribute definition (3)',
+    async function () {
+      assert.deepEqual(
+        sanitize(
+          h(null, [h('a', {x: 'y'}), h('a', {x: 'z'}), h('a', {x: 'w'})]),
+          // `x` on `*` can contain anything:
+          {...defaultSchema, attributes: {a: [['x', 'y']], '*': ['x']}}
+        ),
+        h(null, [h('a', {x: 'y'}), h('a', {x: 'z'}), h('a', {x: 'w'})])
+      )
+    }
+  )
 })
 
 test('`root`', async function (t) {