cofoundry-cms / cofoundry

Cofoundry is an extensible and flexible .NET Core CMS & application framework focusing on code first development
https://www.cofoundry.org
MIT License
835 stars 146 forks source link

Images edited in RichText Editor do not receive a valid URL #551

Open felixrichardt opened 3 weeks ago

felixrichardt commented 3 weeks ago

Hi,

I've just started playing around with Cofoundry. I installed the ImageSharp extension and created a view pages/blocks and uploaded some images.

I had a strange behavior that when uploading images without editing them, they are displayed in the content block. When I edit an image, e.g. resize or crop it, the preview shows the newly edited image, but when I save the changes (close the editor), the image disappears. The img tag is still present in the HTML file, but without the URL. In the preview, the url “blob:https://localhost:5001/” is used, but this is lost when saving.

Is there an error here or is further configuration required?

HeyJoel commented 3 weeks ago

It sounds like a bug, but I haven't had tome to check it over yet. I expect that resizing is a TinyMCE editor feature that isn't being handled by the custom Cofoundry image integration.

HeyJoel commented 3 weeks ago

Ok so it looks like an HtmlSanitizer issue. The html editor is converting the edited image into a data uri, which is being saved but cannot be rendered because the default sanitizer ruleset doesn't allow data uris. If you resize using the drag handled in the editor then it works fine because it's just setting the width/height.

You can fix this by customizing the HTML sanitizer ruleset, adding "data" to your AllowedSchemas e.g.

ruleSet.PermittedSchemes = HtmlSanitizerDefaults
        .AllowedSchemes
        .Append("mailto")
        .Append("data")
        .ToImmutableHashSet();

The question is what should be the correct default behavior:

  1. Should we allow inline editing of images? I can't find a way to disable it in TinyMCE yet, but personally I wouldn't want users adding in large uncacheable data URIs into the page.
  2. Should a data URI scheme be included on the default html sanitizer ruleset? I'm now wondering if there should be a couple of default rulesets, a strict version of content of unknown origin and a more permissive set of editor-drive content.
felixrichardt commented 3 weeks ago

What I can say is that it only took the end user on the new test instance of cofoundry a few minutes to ask why the editor supported this feature, but wondered why it wasn't working. It was just a simple block element they were trying to use to format a short article on this page. Either the feature should be supported correctly or there should be a restriction that this is not supported and does not work correctly - or in the best case, it should not be offered as an option at all.