search

zixaphir / Stable-Diffusion-Webui-Civitai-Helper

Stable Diffusion Webui Extension for Civitai, to manage your model much more easily.
175 stars 24 forks source link

Why wrap the already-sanitized model HTML in HTML comments? #78

Closed mx closed 1 month ago

mx commented 4 months ago

The trim_html function in util.py is removing HTML tags that don't match a whitelist. That's great! Following that though, the entire string is wrapped in an HTML comment before being returned. This messes up other extensions that attempt to display the civitai info file (specifically https://github.com/CurtisDS/sd-model-preview-xd, not sure if any others use these civitai info files or not, but that's the one I used where I noticed the problem). Unless there's a particular reason for doing this that I'm missing (which is quite possible!) it feels pretty redundant given that you already removed all non-whitelisted tags.

If my assumption is correct and this is just strictly redundant, could this be removed so that extension works without modification?

mx commented 4 months ago

Obviously you'd need to remove line 387 if you were to make this change. The whole thing does feel a bit fragile attempting to parse out tags with regexes instead of cleaning it with BeautifulSoup (for the allowlist) and bleach or html.escape() (for everything else). If you want, I can send a pull request to do that to ensure it's sanitized properly, and remove the comment-wrapping.

zixaphir commented 4 months ago

The issue is more that raw HTML has no guarantees. I can sanitize it to a degree and have a fair bit of confidence that it will not be outright hostile, but that doesn't mean it will be syntactically correct. I don't like pulling in dependencies (all external libraries used currently are already dependencies of webui). Largely however, my bigger concern is that I don't believe that raw HTML should be displayed. My reason for wrapping everything in HTML comments is because if I don't, webui will display it as raw HTML, and despite my arguments with the webui devs, they refuse to not make this the default behavior. So I'm largely being petty and just wrapping everything. If the user wants to display the HTML, they can unwrap it. It's just text.

zixaphir commented 4 months ago

I did look into other options but I was not satisfied with any of them at the time I decided to just comment everything out