SanitizerConfig - define valid config #41945
Conversation
github-actions
bot
added
Content:WebAPI
|
|
||
| - Either the `elements` or `removeElements` array may be defined, but not both | ||
| - Either the `attributes` or `removeAttributes` array may be defined, but not both | ||
| - Within an element, either the `attributes` or `removeAttributes` array may be defined, but not both |
There was a problem hiding this comment.
An element can have both attributes and removeAttributes.
There was a problem hiding this comment.
Oh, but not when you have a global removeAttributes, then only per-element attributes or removeAttributes is allowed. WICG/sanitizer-api#305
| - No array may contain duplicate elements or attributes | ||
| - The `replaceWithChildrenElements` array, if defined, may not have any elements in common with `elements` or `removeElements` | ||
| - A global attribute, defined in `attributes`, may not also be defined in an element's `attribute` or `removeAttribute` list. | ||
| - Custom `data-*` attributes may only be specified within element attribute arrays: not in the global `attributes` array, and only if `dataAttributes` is `true`. |
There was a problem hiding this comment.
This is wrong. If dataAttributes is true (which can only happens if you have a global attributes), neither global nor per-element attributes can have data-* attributes. The per-element removeAttributes array can have data-* attributes (so that you can disallow specific data attributes again).
There was a problem hiding this comment.
Actually I am not sure what "Custom data-* attributes may only be specified within element attribute arrays" means either.
You can totally write: new Sanitizer({ attributes: ["data-foo"], dataAttributes: false }).
There was a problem hiding this comment.
If find the best way to think of dataAttributes: true is to imagine there being an implicit attributes: ["data-*"].
There was a problem hiding this comment.
Ah, I get it now. Thank you. dataAttributes: true means that data attributes are accepted without having to specify them in the attributes list, while false means that if you want to allow the attributes you'll have to explicitly list them.
|
|
||
| - Either the `elements` or `removeElements` array may be defined, but not both | ||
| - Either the `attributes` or `removeAttributes` array may be defined, but not both | ||
| - Within an element, either the `attributes` or `removeAttributes` array may be defined, but not both |
There was a problem hiding this comment.
Oh, but not when you have a global removeAttributes, then only per-element attributes or removeAttributes is allowed. WICG/sanitizer-api#305
| - Within an element, either the `attributes` or `removeAttributes` array may be defined, but not both | ||
| - No array may contain duplicate elements or attributes | ||
| - The `replaceWithChildrenElements` array, if defined, may not have any elements in common with `elements` or `removeElements` | ||
| - A global attribute, defined in `attributes`, may not also be defined in an element's `attribute` or `removeAttribute` list. |
There was a problem hiding this comment.
This isn't entirely correct, the condition is more complicated.
One mildly interesting use case to demonstrate this:
attributes: ["class"],
elements: [{ name: "div", removeAttributes: ["class"] }, "span"]
}
Which allows the class attribute for all elements, but not <div>.
- The intersection of
config["attributes"]andelement["attributes"]is empty. element["removeAttributes"]is a subset ofconfig["attributes"].- The intersection of
config["removeAttributes"]andelement["attributes"]is empty. - The intersection of
config["removeAttributes"]andelement["removeAttributes"]empty.
(Copying from https://wicg.github.io/sanitizer-api/#sanitizerconfig-valid)
|
/cc @otherdaniel maybe interesting to you as well. |
hamishwillee
force-pushed
the
ff146_sanitizer_valid
branch
from
5847f78 to
832797f
Compare
| ### Valid configuration | ||
|
|
||
| The configuration object structure allows for the declaration of filter options that are contradictory or redundant, such as specifying an element in both allow and remove lists, or listing an attribute in a list multiple times. | ||
| In order to avoid any ambiguity, methods that take a `SanitizerConfig` instance require that a _valid_ configuration object be passed, and will throw a {{jsxref("TypeError")}} if an invalid configuration is used. | ||
|
|
||
| In a valid sanitizer configuration: | ||
|
|
||
| - Either the `elements` or `removeElements` array may be defined, but not both | ||
| - Either the `attributes` or `removeAttributes` array may be defined, but not both | ||
| - The `replaceWithChildrenElements` array, if defined, may not have any elements in common with `elements` or `removeElements` | ||
| - No array may contain duplicate elements or attributes | ||
| - If the global `attributes` array is defined: | ||
| - An element may define any or none of `attributes` and `removeAttributes` | ||
| - An element's `attributes` must not share any values in common with the global `attributes` array | ||
| - An element's `removeAttributes` array may only contain values that are also present in the global `attributes` array. | ||
| - If `dataAttributes` is `true` the global and element attribute arrays must not contain `data-*` attributes (since these will automatically be allowed). | ||
| - If the global `removeAttributes` array is defined: | ||
| - An element may specify either `attributes` or `removeAttributes`, but not both | ||
| - An element's `attributes` or `removeAttributes` array, depending on which (if either) is defined, must not share any values in common with the global `removeAttributes` array. | ||
| - The global `dataAttributes` array must not be defined. | ||
|
|
||
| The empty object `{}` is a valid configuration. | ||
|
|
||
| > [!NOTE] | ||
| > The conditions above are from the perspective of a web developer. | ||
| > The [validity check defined in the specification](https://wicg.github.io/sanitizer-api/#sanitizerconfig-valid) is slightly different because it is executed after canonicalization of the configuration, such as adding `removeElements` when both are missing, and adding default namespaces. |
There was a problem hiding this comment.
@evilpie Thanks very much - extremely helpful. This is now updated following the spec pattern of splitting the element attribute behaviour based on whether the global attributes or removeAttributes are defined.
There is a slight reduction in repetition, and of course {} is valid in this context.
Following reading this, it seems there have been other changes I haven't track, such as most of the sanitizer methods now returning booleans. I've updated those too. You can ignore those changes if you want - this one is the complicated bit.
| The specified element is added to the [`elements`](/en-US/docs/Web/API/SanitizerConfig#elements) list in this sanitizer's configuration. | ||
| If the element is already present in the list, then the existing entry is first removed and the new definition is appended to the end of the list. |
There was a problem hiding this comment.
Note, for all these methods I have removed comments about what happens under the hood.
A spec author needs to know at this level that the internal configuration might specify either an allow or remove list, and what the update process is different in each case.
A website developer does not - all they need to know at this level is that specifying an element will ensure that it is allowed.
| `true` if the operation changed the configuration to allow the element, and `false` if the configuration already allowed the element. | ||
|
|
||
| Note that `false` might be returned if the internal configuration: | ||
|
|
||
| - defines an [`elements`](/en-US/docs/Web/API/SanitizerConfig#elements) array array and the element is already present (it does not need to be added again) | ||
| - instead defines the [`removeElements`](/en-US/docs/Web/API/SanitizerConfig#removeelements) array and the specified element is not present (and is hence already not filtered) |
There was a problem hiding this comment.
Note that since I last updated this the return type became a boolean. Note that this indicates whether the configuration was changed by the operation - not whether the change worked - it always works.
| Note that if you need both per-element add-attribute and remove-attribute lists, they must be added in a single call to this method (since if done in two calls, the second call will replace the element definition added in the first call). | ||
|
|
||
| The specified element is removed from the sanitizer configuration [`removeElements`](/en-US/docs/Web/API/SanitizerConfig#removeelements) or [`replaceWithChildrenElements`](/en-US/docs/Web/API/SanitizerConfig#replacewithchildrenelements) lists if present. | ||
| The element can be specified with arrays of attributes that are allowed or disallowed on elements of that type. |
There was a problem hiding this comment.
While I think I agree with your general idea of not leaking too many internals. There is one specific case here that should be worth calling out.
The following does nothing and should produce a warning:
s = new Sanitizer({ removeElements: ["div"] })
s.allowElement({name: "div", attributes: ["class"] })
This is because you can't have removeElements combined with per-element attributes/removeAttributes.
|
With the current examples, I think it's not clear that you can have an allow list of elements and a remove list of attributes (and vice versa). It might not really be an interesting use case to call out, though. |
2 participants
Fixes #41634
Defines what a valid SanitizerConfig is more precisely and links to this from the places where an invalid config can throw a TypeError.
Related docs work can be tracked in #41649