Filter enforcement and raw HTML output

Created on 16 June 2025, 8 days ago

While testing the Markdown Easy module, I encountered two issues related to the "Limit allowed HTML tags" filter.

Issue 1: Enforced dependency

The documentation suggests that the "Limit allowed HTML tags and correct faulty HTML" filter is strongly recommended, but optional. For example, the filter tips state:

"The Markdown Easy filter should run before the 'Limit allowed HTML tags and correct faulty HTML' filter. It is strongly recommended to use these filters together."

However, the module currently enforces this dependency. Around line 85 in markdown_easy.module, the following condition prevents saving the text format unless the HTML filter is enabled:

if (!isset($subsequent['status']) || !$subsequent['status'] || ...)

Similarly, in the README.md (lines 10–12), it says:

"It is strongly suggested ..."

This behavior appears to contradict the documentation. For trusted content, like my personal blog, I would prefer not to enable HTML filtering. Requiring it feels overly restrictive, but maybe I'm missing something?

2) Validation error displays raw HTML

When the above validation fails, the error message displays literal HTML, like:

This is likely caused by calling ->render() on a translatable string (around lines 100 and 108). In Drupal form validation, error messages should remain as TranslatableMarkup objects or be cast with (string) to avoid triggering the render system unnecessarily. For reference, see comments #21 and #22 on πŸ“Œ Support additional extensions Active .

Proposed solution

  1. Remove the requirement to enable the HTML filter (make it truly optional), or clarify the documentation to reflect the enforced dependency. Consider validating filter weights only when both filters are enabled.
  2. Fix the raw HTML display by avoiding use of ->render() in form validation error messages.
πŸ› Bug report
Status

Active

Version

1.0

Component

Code

Created by

πŸ‡§πŸ‡ͺBelgium Dries

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @Dries
  • πŸ‡§πŸ‡ͺBelgium Dries
  • πŸ‡ΊπŸ‡ΈUnited States ultimike Florida, USA

    Going backwards...

    Issue 2 should be resolved in #3493938 πŸŽ‰

    Issue 1 is something I'm not unaware of. Perhaps it is something that can be corrected with some updated language. When I originally wrote the module, I wanted it to be secure by default, and not easy to make insecure (by not using the "Limit HTML" filter.) With the "strongly recommended" language, I meant that it can't be made insecure via configuration - only by implementing a hook (which I documented here β†’ .)

    Note to self, once I tag a 2.0.0 release, I need to update the documentation page.

    So, we could change the language to be more clear on this point, or add a configuration option to remove this dependency? If we do the latter, should it be a global configuration (which I kind of like because it would not be on the text format add/edit page and thus a slightly higher hurdle) or a per-text-format option? Or, is there another option to be considered?

    thoughts?

    -mike

  • πŸ‡§πŸ‡ͺBelgium Dries

    I appreciate the intent behind making things secure by default. That is a great philosophy.

    Maybe this is a 2-step process?

    Step 1 (short-term): Update the documentation and help text. Right now, the messaging reads like a strong suggestion, but the code enforces it as a hard requirement. That mismatch can be confusing, as it was for me. A small change could prevent some head-scratching.

    Step 2 (longer term): I wonder if there is an opportunity to bring some of this thinking back to Core. If the goal is to ensure that all text formats are secure by default, maybe Core should issue a warning when the "Limit allowed HTML tags" filter is disabled. That would make things more consistent and remove the need for individual modules like Markdown Easy to enforce that logic on their own. Just a thought.

    To your question about adding a config option to make the filter optional: personally, I wouldn't. The current approach works. When configured correctly, the extra filter doesn't really get in the way. For some users, it might be a bit annoying at first, but once it's configured correctly, you can forget about it.

    In short, my vote would be: (1) clarify the documentation and help text, (2) keep the current behavior without adding a setting, and (3) consider whether secure-by-default enforcement belongs in Core for all text formats.

  • πŸ‡ΊπŸ‡ΈUnited States ultimike Florida, USA

    Changing to 2.0.x

  • πŸ‡ΊπŸ‡ΈUnited States ultimike Florida, USA

    Changing my mind - going back to 1.0.x πŸ˜ƒ

  • πŸ‡ΊπŸ‡ΈUnited States ultimike Florida, USA

    ultimike β†’ changed the visibility of the branch 3530329-filter-enforcement-and to hidden.

  • πŸ‡ΊπŸ‡ΈUnited States ultimike Florida, USA

    ultimike β†’ changed the visibility of the branch 3530329-filter-enforcement-and to active.

  • πŸ‡ΊπŸ‡ΈUnited States ultimike Florida, USA

    ultimike β†’ changed the visibility of the branch 3530329-filter-enforcement-and to hidden.

  • πŸ‡ΊπŸ‡ΈUnited States ultimike Florida, USA

    ultimike β†’ changed the visibility of the branch 3530329-filter-enforcement-and to active.

  • πŸ‡ΊπŸ‡ΈUnited States ultimike Florida, USA

    ultimike β†’ changed the visibility of the branch 1.0.x to hidden.

  • πŸ‡ΊπŸ‡ΈUnited States ultimike Florida, USA

    Ready for review. These changes are really only useful for the 1.0.x branch because πŸ“Œ Improve interaction with HTML filter Active , ✨ Create new "power user" flag to disable warnings and validation Active , and πŸ“Œ Remove/lesson dependency on "Convert line breaks into HTML" Active will modify this behavior.

    As suggested above, core issue created: ✨ Provide warning on Site Status page when "Limit allowed HTML tags" filter is not used? Active

    -mike

  • πŸ‡§πŸ‡ͺBelgium Dries

    I reviewed this merge request and think it looks great. It's a clear improvement. In my opinion, it's ready to be merged.

Production build 0.71.5 2024