[config] "allowed-elements", "allowed-markup" should replace, not amend default allowlists

[chendachao]

I found an issue about the markup config, any suggestions would be appreciate.

Step1, add below code to isso server config,

options = strikethrough, superscript, autolink
allowed-elements = img, blockquote, br, code, del, em, h1, h2, h3, h4, h5, h6, hr, ins, ul, li, ol, p, pre, strong, table, tbody, td, th, thead
allowed-attributes = src

Step2, In the website where you refer isso as comment fn, add below comment:
<a href=http://evil.com>clickmetochangethispassword</a>

Result: the comment was successfully rendered.

Expect: transform the a markup.

[ix5]

See server docs

allowed-elements
Additional HTML tags to allow in the generated output, comma-separated.
By default, only a, blockquote, br, code, del, em, h1, h2, h3, h4, h5, h6, hr, ins, li, ol, p, pre, strong, table, tbody, td, th, thead and ul are allowed.

I agree that the config should be named additional-allowed-elements to be more accurate (or replace instead of amend the default list).

The code for that is in isso/utils/html.py:
https://github.com/posativ/isso/blob/9e021cb6263e921ac519f52b82df3f66a297d3d1/isso/utils/html.py#L91-L93
https://github.com/posativ/isso/blob/9e021cb6263e921ac519f52b82df3f66a297d3d1/isso/utils/html.py#L20-L24

But the config has been working that way for quite some time and has been documented to work that way. If you wanted to change this behaviour, you'd most likely break existing setups.

[chendachao]

Hi @ix5, so how could I setup the configurations to exclude markup a, both elements and allowed-elements can't work

[ix5]

Edit isso/utils/html.py and remove the default lists:
https://github.com/posativ/isso/blob/9e021cb6263e921ac519f52b82df3f66a297d3d1/isso/utils/html.py#L20-L27

Change that to:

self.elements = elements
self.attributes = attributes

Note that this is only a temporary fix ("monkey-patching") that you have to apply every time you install/update isso. If you want to change the behaviour for everyone, you should submit a Pull Request to change it.

[chendachao]

Could you submit a pull request to fix it, cos I know nothing about python.

[ix5]

@chendachao even if you do not know anything about python, you could be a big help in sketching how this functionality should work.

Here's an overview of what I think you should do to get this "fixed":

1. Ping some people who you think should be consulted and discuss this with them
2. Create two new properties, named element-allowlist and attribute-allowlist, that defifine all the allowed syntax, instead of amending the default
3. Warn users who still use allowed-elements or allowed-attributes in the isso error log
4. Write up a changelog item (CHANGES.rst) notifying users of this breaking change
5. Suggest/decide whether this change should be part of the next isso release or rather wait until a major version bump

[ix5]

An alternative approach would be to re-use the existing config key names (allowed-elements, allowed-attributes) and strip out the default allowed elements. Then add them to the default isso.conf.

That way, people who used the allowed-* config to define the only items they wanted to allow will not have to change anything and Isso would work the way they probably expected it to work (including you).

Most people would probably leave the default untouched (meaning they didn't overwrite the values in their own isso.conf), thus not being affected by this change.

The only negative impact would be people who understood the meaning of the config keys as they are currently specified in the docs (see server.rst )

[chendachao]

Sorry, I think it's irresponsible to make a PR if I don't understand the source code. Could you make that changes?