Problem

  • The fallback format concept was originally introduced to allow Filter module to cope with unforeseen edge-case configuration scenarios, but was never intended to be exposed as a configurable text format.

Details

  • The plain_text fallback format was only ever meant as a (literal) fallback format.

  • Its introduction was caused by technical edge-case scenarios only. Specifically:

    1. When all existing formats are accessible to certain user roles only,
    2. and the currently logged-in user does not belong to any of those roles,
    3. but a text-format-enabled text field happens to be exposed to that user,
    4. then Filter module logically has no idea what to do, because it was told to accept user input that needs to be filtered, but there is no format the user is allowed to use.

    That's the one and only reason for why the fallback format exists. There are multiple possible ways to reach that situation, so we were not able to say "Stuff that, that edge-case is caused by a bogus configuration, fix your configuration instead." Hence, we introduced the fallback format to resolve that problem.

  • That is also the reason for why the plain_text fallback format is super-restrictive and escapes all HTML.

    • It is only used in situations when the current user technically does not have access to any format.
    • That's also why the fallback format ID is not configurable from the UI.

    It was a pretty big mistake to expose it as a configurable format in the administrative UI and we need to fix that.

  • The fallback format was not meant to be exposed in the visible way it is today.

Proposed solution

  1. Remove all traces of the fallback format from the administrative Filter module UI.

    This inherently means that the fallback format is no longer configurable.

  2. Add an API-level condition to disallow filter configuration changes to the fallback format.

  3. Since we do not know whether the fallback format was reconfigured on existing sites:

    • Introduce a new fallback format 'none' in D8, as a replacement for the previous plain_text format.
    • For sites upgrading from D7, the plain_text format becomes a regular, configurable format.
    • The new none fallback format will not be exposed in the UI and its configuration cannot be changed.

  4. Remove the additional configuration setting that was introduced with #788114: Unprivileged users should only get one text format by default

Notes

  • With properly set up text formats, anonymous users are able to access (at least) one actual text format.

    • The fallback format is not intended to be used as "default format" for anonymous users.
    • Anonymous users should still get an actual text format along the lines of filtered_html. They should be allowed to enter basic HTML.
    • In most Drupal site use-cases (but certainly not all), anonymous users only get one format and should not see a format selector.

Related issues

Comments

quicksketch’s picture

quicksketch
he/him
commented

Hm, I don't think you can really classify this a bug report. If it were a bug, it means that this would equally be considered a "major bug" for Drupal 7, but the possibility of a backport is practically non-existant considering its potential for breaking existing sites. I would classify this as an inconvenience more than anything else. Even #788114: Unprivileged users should only get one text format by default, which exposed the fallback to ALL users was only considered a task. This is a minor problem that specifically affects administrators.

The original issue that introduced the fallback was #11218: Allow default text formats per role, and integrate text format permissions. The early versions hide the plain text format entirely (one of mine even made it a hard-coded format directly in filter.module). It was added back as a "normal" format for flexibility.

@David_Rothstein rerolled the patch to turn it into a "real format" but hid all the UI components in the second version:

I started off writing this patch with the "no format" option completely hardcoded in, but then eventually realized that didn't seem like the best approach -- it led to too many special cases and was too rigid. So instead, the attached patch implements the "no format" (plain text) case to work mostly like a normal input format (i.e., stored in the database), but with the UI for editing this format removed, and a call to check_plain() hardcoded. The idea is to completely lock editing of this input format within core, but allow a contrib module (some) freedom to change it while still maintaining a good degree security (i.e., making it so you'd really have to go out of your way to turn this "plain text" format into a dangerous one).

I'm not quite clear on when the the format became shown in the UI from reading the issue, but initially it was only visible but not editable. Then later the ability to edit the fallback format was added back to the UI.

So initially, we set up to make the fallback truly a fallback but gave up on it for "flexibility purposes". I honestly would prefer it disappeared entirely and became truly a fallback, but it does make the whole behavior slightly more magical if it's not mentioned anywhere in the UI.

So in the end, I'm ambivalent to this proposal because I think it'll be beneficial in some ways and restrictive in others. I'm for completely hiding the fallback; but I don't think this is a significant problem in the grand scheme of things.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Category: bug » task
  1. Not all bug fixes can be backported, and I wasn't really aware of the havoc that the current fallback format implementation causes, so from a Filter module/API perspective, this is clearly a bug, but I don't really care about this ticket's classification as long as it gets resolved. ;)
  2. Yes, the "flexibility" that was originally added in #11218: Allow default text formats per role, and integrate text format permissions is the big mistake and root cause for the situation we're facing.
  3. The original idea to allow a contrib module to change this format was the bogus train of thought, which caused the "flexibility." We completely missed and ignored the fact that the fallback format is only ever supposed to be a fallback.

From a Filter API/module perspective, this is a major bug in the architectural design.

I'm not talking about any UX or other implications that may happen to be there. Only Filter module's text format API and its reliability and security.

The originally intended concept - as outlined in the issue summary - was sensible. But the current implementation goes way beyond that and that's just simply wrong. It's a typical artifact of applying too many "patterns" and a false-sense of customizability to everything that exists. I'm fairly sure that I'm guilty for some parts of this myself, since as mentioned in #1, the concept was originally introduced with too much flexibility and complexity already, so the entire core contributor army only continued to advance on it, without validating whether this makes any sense (again, myself included).

The fallback format must not be customizable.

I considered already whether we can hard-code this format, so as to make it always available and never configurable. That was the original idea of using check_plain(). However, all text content that happens to use this fallback format has to reference it and a hard-coded format cannot be referenced or looked up through standard facilities, so ultimately, we better have a proper record of the fallback format in configuration, but disallow any changes to it.

And alas, the fact that the current fallback format lives in configuration is also what allows us to easily swap it out today. As long as it is not customizable from the API/UI, we should be fine. We can't protect against manual config files changes, but in general, it looks like manual changes to config files will be considered as "hacks" in D8, following the "don't hack core" mantra, so we should be fine, too.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Issue summary: View changes

Updated issue summary.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

Updated the issue summary and appended a new task.

FWIW, the solution and effective changes here should be fairly straightforward and a no-brainer to implement. To quickly summarize:

  1. Skip over the fallback format in the administrative listing of text formats (do not expose it).
  2. Remove all special-casing related to that.
  3. Rename the default configuration object filter.format.plain_text.yml to filter.format.none.yml
  4. Add a hook_update_N() to filter.install to create the new fallback format. (the existing is upgraded already)
  5. Add a hard-coded check for the new fallback format to FilterFormatStorageController::preSave() and throw an exception whenever someone tries to change the 'filters' definition.
  6. Kill the new configuration option in filter.settings + and its usage in filter_process_format().
tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Version: 8.x-dev » 9.x-dev
Priority: Major » Normal

I personally think of this as a feature request. I don't agree that plain_text should be not-configurable, just non-deletable (as it is).

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Issue summary: View changes

Updated issue summary.

catch’s picture

catch
he/him
Primary language English
commented
Version: 9.x-dev » 8.1.x-dev

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.0-beta1 was released on March 2, 2016, which means new developments and disruptive changes should now be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Issue tags: +missing config dependencies, +Configuration schema, +validation
Related issues: +#3395562: Add validation constraints to filter.settings

FYI this came up again in #3395562: Add validation constraints to filter.settings. filter.settings:fallback_format is impossible to validate.

This issue would allow removing that from config!

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.