While migrating from D6 to D7, the function simplenews_update_7000() threw an error because I was trying to insert a string with 71 characters into a field which only allowed 64 characters. I personally feel that 64 is too short a limit, especially in languages which are more verbose than English (in my use case, French). Further, this will break the upgrade path for anyone with a longer from_name in D6. So, I propose reverting #1498194: Missing length validation on from_name, changing the schema of the table simplenews_category, and changing simplenews_update_7000() accordingly.

If you really believe that this field should be limited to 64 characters, simplenews_update_7000() should truncate this field and print a warning instead of throwing an error.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mvc’s picture

Status: Needs work » Needs review
FileSize
1.95 KB
Berdir’s picture

The email + display name shouldn't be longer than 255 characters, or it will be dropped by SMTP, see http://msdn.microsoft.com/en-us/library/cc765823.aspx.

Not sure if this would be a problem, I guess 128 should work...

Berdir’s picture

Oh, and even if we increase the limit, it wouldn't hurt to ensure that the existing strings aren't longer. variables have no length limit and who knows what people added in there. Maybe other things as well?

mvc’s picture

i didn't know about that limit, but if the name is limited to 128 and the address to 64 then we'll still be fine.

here's a version which actually checks the lengths of strings in D6 variables, truncates them to avoid fatal errors, and prints a useful warning.

Berdir’s picture

Status: Needs review » Fixed

Thanks, commited.

Status: Fixed » Closed (fixed)

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