Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Opening this as a critical bug because I am seeing new issues crop up for this crop up frequently in core and it is likely to be just as prevalent all over contrib.
Examples:
#1180642: PDOException in statistics_exit() when path longer than 255 characters
#218004: Allow aggregator feed URLs longer than 255 characters
#1244116: Feed items with title longer than 255 characters fail to insert into aggregator_item
In Drupal 6 the value would be silently truncated by MySQL to fit, in Drupal 7 you get a PDO exception due to strict mode. This means we have three possibilities, they're not mutually exclusive:
1. Revert strict mode for MySQL for Drupal 7.
2. Convert any column where this might happen from varchar to text (note this can be extremely bad for performance depending on the column and the kinds of queries potentially run on it, see my comment at #1244116-70: Feed items with title longer than 255 characters fail to insert into aggregator_item). So if done, this needs to be carefully evaluated case by case, and we'd need a change request too.
3. Truncate values in PHP before they get inserted. Probably also needs to be evaluated case by case.
Tagging as upgrade path since it's currently a regression from Drupal 6, as well as requiring multiple schema changes if we go with #2.
Comments
Comment #1
catchLinking back to #303054: Enable MySQL strict mode where this was added.
Comment #2
chx CreditAttribution: chx commentedAs I have argued for getting this into core, my original reason was "other databases might balk if you try to store longer strings than allowed so MySQL should not allow it either so we catch bugs". This, I think, stands. We caught bugs. Lots :/
Comment #3
marcingy CreditAttribution: marcingy commentedJust adding #1222434: Writing to accesslog error when node title 255 chars to the list of affected issues.
Comment #4
chx CreditAttribution: chx commentedI just named #1015916: Image field "title" allows more data than database column size. a duplicate.
Comment #5
Anonymous (not verified) CreditAttribution: Anonymous commentedI agree with chx - code that tries to put data that is too long into a column needs fixing, so I'd generally favour 3.
Comment #6
BTMash CreditAttribution: BTMash commentedSince I'm coming at this from #1015916: Image field "title" allows more data than database column size., I favour option 3 to be used by core on the data. Mind you, I would be strongly favouring option 2 on columns of data that are NOT reliant on having queries run against them aside from retrieving their info based off another some other keys (like alt/title data for an image or the aggregator feed urls) - so the module should be allowed to make a case on converting a column to be in a longer format.
Comment #7
catchOK so I think what is needed here is a change notification (for Drupal 7), and then a note somewhere, probably the handbook, explaining that you need to truncate values in PHP before inserting them into varchar if they come from any kind of user input.
And then on top of that, a section that explains why you would use varchar vs. text or find a good external resource to link to.
Then we can refer to those from the 5-6 core issues that have hit this, and go back to dealing with the individual bugs case by case.
Comment #8
EvanDonovan CreditAttribution: EvanDonovan commentedCan this be done as soon as possible then, since the issues which have been cross-linked here oftentimes already have proposed fixes?
Can anyone mark this proposal as RTBC, or does more consensus need to be built first?
Comment #9
brianV CreditAttribution: brianV commentedSubscribing
Comment #10
brianV CreditAttribution: brianV commentedReverting accidental title change.
Comment #11
Damien Tournoud CreditAttribution: Damien Tournoud commentedThis is now a documentation task.
Not completely sure what all the ado is about, as MySQL strict mode is obviously right. Loss of data is unacceptable for a database system.
Comment #12
jhodgdonCan someone please edit the issue summary explaining what needs documenting and where?
Comment #13
catch@Damien. Throwing exceptions on valid data is unacceptable for a PHP application, since we're doing that in several places in core now, that seemed worth a critical issue. Agree this is just docs now though.
This should at least go in a change notice, I'm not sure where else.
Suggested wording:
When using varchar for storage, your code must truncate values to the maximum length before they are sent to the database layer if there is any chance the data may exceed the allowed size.
In Drupal 6, MySQL and older versions of PostgreSQL would silently truncate these values. Drupal 7 runs MySQL in strict mode and includes support for more database types. Instead of truncating, it will throw a PDO Exception if attempting to write data exceeding the maximum length of the varchar.
In some circumstances it may be better to increase the size of the varchar or change the type of the column to text. However note that text columns can negatively affect performance if they are included in a query that creates a temporary table, see http://dev.mysql.com/doc/refman/5.1/en/internal-temporary-tables.html
Comment #14
webchickTagging.
Comment #15
catchPosted http://drupal.org/node/1297592
Marking this fixed. I'll go un-postpone the other issues.
Comment #16
catchAdded an additional note that varchar(255) is the maximum allowable length in Drupal 7 since we support < MySQL 5.0.3.
Comment #17
Damien Tournoud CreditAttribution: Damien Tournoud commentedClarified the requirements on the change request.
Comment #18
jhodgdonSince this is a 7.0 change, do you think we should also add it to the horrible ugly old 6.x/7.x module update page too? (http://drupal.org/update/modules/6/7)? Or should I just add a note to that page to go look at the Updates list as well for 6.x/7.x changes?
Actually, that might be the best idea... thoughts?
Comment #20
xjmComment #21
xjm