Problem/Motivation

#3611070: SQLSTATE[22001] error when inserting DAM media with long alt text — acquia_dam_managed_image_alt column truncation introduces a critical bug that will break existing drupal sites.

Drupal's standard text field widget (string_textfield) only supports string and uri field types (field_types = {"string", "uri"}). It does not support string_long. string_long fields use string_textarea

Any existing site with entity form display configurations (core.entity_form_display.*) referencing string_textfield for acquia_dam_managed_image_alt will break or fail to render the form widget once the field type becomes string_long.

Note, Instead of converting the field type from string to string_long (which triggers form display breakages and schema type migrations), the string field type should have been kept, and instead increase max_length. Setting max_length: 2048 or 4096 on the string field definition will accommodate extremely long DAM metadata strings while avoiding field widget breakages, schema type shifts, and multiline text issues.

Steps to reproduce

Install a site with different media types and Acquia Dam 1.1.15. Update to 1.1.16. Site broken.

Proposed resolution

Create a new update hook to scan and update all core.entity_form_display config objects that reference this field to change their widget plugin from string_textfield to string_textarea (or another string_long-compatible widget).

or
Revert back to the string type, remove the bad update hook, and create a new update hook that correctly resets the type and sets the length to something longer.

Issue fork acquia_dam-3613681

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

japerry created an issue. See original summary.

japerry’s picture

Status: Active » Needs review

This patch sets the length to 4096 and reverts it back to a string type. Given the other tradeoffs of using string_long for something that fundamentally should be a string, I think this is the most straightforward way.

rajeshreeputra made their first commit to this issue’s fork.

rajeshreeputra’s picture

japerry’s picture

Status: Needs work » Needs review

This MR (338) merges some of the work from my MR331 and the issues brought by MR 332. Hopefully this one can resolve all the regressions!

rajeshreeputra’s picture

Requesting review!

  • japerry committed e7661ba0 on 1.1.x
    Resolve #3613681 "Revert acquia_dam_alt_text to a bounded string and...
japerry’s picture

Status: Needs review » Fixed

Looks great! (both manual and automated reviews including tests look great) Thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.