Describe the bug
The current SanitizeHtml settings' hints don't tell the whole story and enabling/disabling it causes potentially unwanted behavior.
- The hints only refer to preventing custom scripts, which is true, but it does much more by default. This, in itself is trivial to fix. However:
- This setting also governs whether Liquid markup is rendered:
SanitizeHtml = false means Liquid is rendered. This prevents a simple use-case where (script) sanitization is wanted, but Liquid should be rendered too.
- Regardless of the setting, Liquid code is always validated. This is problematic when Liquid code is included as a code sample (in a
<code> block) and that code is invalid, e.g. because it references a Liquid tag, which is not available in the current system.
This affects HtmlBodyPart, HtmlField, MarkdownPart, and MarkdownField.
Orchard Core version
2.2.1 and below (where SanitizeHtml is available).
To Reproduce
- Set up site with Blog recipe.
- Optional: Add
HtmlBodyPart instead of MarkdownBodyPart to the BlogPost content type and modify Content-BlogPost.liquid the render it.
- Add any Liquid code in the Markdown/HTML body, e.g.
<h2>{{ "Orchard Core uses Liquid!" }}</h2>. It's only rendered as Liquid when HtmlSanitize is false.
- Also add
{% href "https://orchardcore.net" %} anywhere in the body. It will prevent saving the item, regardless of the SanitizeHtml settings's value.
Expected behavior
SanitizeHtml fields' hints are worded more clearly regarding their behavior.
- Having a
RenderLiquid setting, which should work independently from SanitizeHtml.
- Liquid code should be only be validated, if
RenderLiquid is enabled.
Nice to haves later
- Liquid code inside a
<code> tag should not be rendered/validated (just displayed raw), even if RenderLiquid is enabled. Not sure how easy that is to achieve (in terms of separation of concerns); might be out of scope for this issue.
HtmlMenuItemPart could also support Liquid syntax/rendering, same as HtmlBodyPart and HtmlField.
Design questions
MarkdownBodyPartDisplayDriver (same for Field) states that "The liquid rendering is for backwards compatibility and can be removed in a future version.".
I removed these notices; there's nothing wrong with MarkdownField/Part being able to render Liquid, especially now that it's not competing with sanitization.
Seeing that MarkdownBodyPart and MarkdownField are both found in the OrchardCore.Markdown module, I suggest moving HtmlField (from OrchardCore.ContentFields) and HtmlMenuItemPart (from OrchardCore.Menu) to OrchardCore.Html. This would allow some DRYing and make it easier to maintain these components (i.e. keep their changes in sync).
This seems too much hassle for little gain.
Describe the bug
The current
SanitizeHtmlsettings' hints don't tell the whole story and enabling/disabling it causes potentially unwanted behavior.SanitizeHtml = falsemeans Liquid is rendered. This prevents a simple use-case where (script) sanitization is wanted, but Liquid should be rendered too.<code>block) and that code is invalid, e.g. because it references a Liquid tag, which is not available in the current system.This affects
HtmlBodyPart,HtmlField,MarkdownPart, andMarkdownField.Orchard Core version
2.2.1 and below (where
SanitizeHtmlis available).To Reproduce
HtmlBodyPartinstead ofMarkdownBodyPartto theBlogPostcontent type and modifyContent-BlogPost.liquidthe render it.<h2>{{ "Orchard Core uses Liquid!" }}</h2>. It's only rendered as Liquid whenHtmlSanitizeis false.{% href "https://orchardcore.net" %}anywhere in the body. It will prevent saving the item, regardless of theSanitizeHtmlsettings's value.Expected behavior
SanitizeHtmlfields' hints are worded more clearly regarding their behavior.RenderLiquidsetting, which should work independently fromSanitizeHtml.RenderLiquidis enabled.Nice to haves later
<code>tag should not be rendered/validated (just displayed raw), even ifRenderLiquidis enabled. Not sure how easy that is to achieve (in terms of separation of concerns); might be out of scope for this issue.HtmlMenuItemPartcould also support Liquid syntax/rendering, same asHtmlBodyPartandHtmlField.Design questions
MarkdownBodyPartDisplayDriver(same for Field) states that "The liquid rendering is for backwards compatibility and can be removed in a future version.".I removed these notices; there's nothing wrong with MarkdownField/Part being able to render Liquid, especially now that it's not competing with sanitization.
Seeing thatMarkdownBodyPartandMarkdownFieldare both found in theOrchardCore.Markdownmodule, I suggest movingHtmlField(fromOrchardCore.ContentFields) andHtmlMenuItemPart(fromOrchardCore.Menu) toOrchardCore.Html. This would allow some DRYing and make it easier to maintain these components (i.e. keep their changes in sync).This seems too much hassle for little gain.