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.
Comment | File | Size | Author |
---|---|---|---|
#4 | simplenews-longer-from_name-1585992-4.patch | 4.49 KB | mvc |
#1 | simplenews-longer-from_name-1585992-1.patch | 1.95 KB | mvc |
Comments
Comment #1
mvcComment #2
BerdirThe 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...
Comment #3
BerdirOh, 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?
Comment #4
mvci 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.
Comment #5
BerdirThanks, commited.