search

syntax-tree / hast-util-sanitize

utility to sanitize hast nodes
https://unifiedjs.com
MIT License
48 stars 20 forks source link

`<input type=checkbox>` support #12

Closed arystan-sw closed 5 years ago

arystan-sw commented 6 years ago

added "input" to allowed list because otherwise GFM task list checkboxes would get stripped when I used remark-react

wooorm commented 6 years ago

This looks very similar to what we’ve discussed in https://github.com/syntax-tree/hast-util-sanitize/pull/2. What do you think of our reasoning there?

arystan-sw commented 5 years ago

How about extending the sanitization schema to allow attrs only if they have specific values? Like this:

diff --git a/lib/github.json b/lib/github.json
index 615e1e9..53430a5 100644
--- a/lib/github.json
+++ b/lib/github.json
@@ -127,6 +127,13 @@
     "q": [
       "cite"
     ],
+    "input": [
+      ["type", "checkbox"],
+      ["disabled", ""]
+    ],
+    "li": [
+      ["class", "task-list-item"]
+    ],
     "*": [
       "abbr",
       "accept",
wooorm commented 5 years ago

Interesting! But how would you handle if two values are allowed (type radio and type checkbox for example)?

arystan-sw commented 5 years ago

Can use regex on this occasion:

"input": [
  ["type", /^(checkbox|radio)$/]
]
arystan-sw commented 5 years ago

Since json file doesn't support regex type, the regex needs to be stored as a string. A compact way to differentiate it from literal value would be to check the presence of a / on both sides of the string:

"input": [
  ["type", "/^(checkbox|radio)$/"]
]
wooorm commented 5 years ago

I think I’d prefer an array of values, in this case. But never mind, it doesn’t matter so much for your case. We can add regexes / arrays later (or not), right?

One notable caveat in your example is that in HAST, there is no class, or "" value for disabled, they are className and true.

So:

Interested in working on this?

arystan-sw commented 5 years ago

I'll give it a try.

arystan-sw commented 5 years ago

Correct?

wooorm commented 5 years ago

@arystan-sw So sorry for letting this fall through the cracks!

I fiddled with this yesterday, it looks great but there’s one problem: input[type=checkbox][disabled] is optional but it should be required, input, input[type=checkbox] are both fine as well, but they shouldn’t be.

The current schema does not have a way to require properties. So not sure how to add something like that? What do you think? 🤔