Problem/Motivation
Uncaught PHP Exception Drupal\Core\Entity\EntityStorageException: "SQLSTATE[22003]: Numeric value out of range: 1264 Out of range value for column 'field_date_value'
No, i should not have entered the year 3333 while putting in test data, user input should never cause a white screen of death— this should be caught in form validation.
Steps to reproduce
Enter a date more than a thousand or so years in the future.
Proposed resolution
We could do that based on the underlying storage; not sure if some introspection would be needed for that, or we could just add a feature to limit the allowed date range and start it off by default at something reasonable, and sites that change that have to know what they are doing.
Hard-coding limits for the expected MySQL/MariaDB limits would also be fine for now i think. Again, an error message is fine. A WSOD is not.
Seems core just fixed this by making it "big" – #2016787: Far dates causes PDOException 22003? I still think the correct solution is that, no matter what the allowed range, no user input, no matter how unreasonable, should break a site.
Remaining tasks
User interface changes
API changes
Data model changes
Issue fork smart_date-3304395
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
Comment #2
mandclu commentedThanks for raising this, and I think this is a valid perspective. One nuance I will throw in here is that as of Drupal 10.1 the 2038 problem will be solved, so the validation you have in mind would need to know whether or not the site is using the current, problematic timestamp storage, or the storage we'll have later, which allows for far-future dates to be stored without issue.
Comment #3
mlncn commentedThe date field fix that went into Drupal core goes from storing as an "INT" to storing as a "BIGINT"— going from 68 years before or after 1970 to uh 292,471,208,677 years which should cover slightly more than human recorded history and make it really hard to put in an out-of-range value. As the estimated age of the universe is only 14,000,000,000 it's got a good order of magnitude on that even for fat-fingered cosmologists.
This is solving itself. We could put a check on that extremely broad range and it'll actually be useful in the extreme edge fuzz-testing cases for post 10.1 sites.
As Smart Date allows its widgets to be used by different fields is there anything else we should (and can?) look out for?
Comment #4
mlncn commentedI guess it won't actually be fixed for timestamp fields until #2885413: Timestamp field items are affected by 2038 bug?
And core is working on a parallel bug about not allowing out of range input into date fields, #2840220: "Date and time" Form API element allows entry beyond min/max values
I feel like those two issues cannot be both an issue at the same time now that #2016787 landed?
Comment #5
mandclu commentedThanks for your follow-up on this issue, in particular the links to the similar core issues. Having looked through the changes in #2885413: Timestamp field items are affected by 2038 bug, I believe that we will need to:
- Update the SmartDateItem class to explicitly use
'type' => 'big'for value and end_value columns- Write a post-update hook to update existing fields to use the larger storage
Hopefully this will address the potential for user input to create a WSOD. It's also worth noting that Smart Date fields do also allow for minimum and maximum dates to be set, though it shouldn't be necessary to populate to prevent a fatal error.
Comment #7
mandclu commentedHere's an MR that works on my local to make all new fields 2038 ready; Also it should convert any existing fields.
Comment #8
mandclu commentedComment #9
jurgenhaasI have successfully tested this on Drupal 9.5.10 with smart_date 3.7.2 and bookable calendar with over 500 open instances, i.e. over 500 records in the DB.
The update was applied successfully and the DB field
date__valueanddate__end_valuechanged their type from int to bigint. The values remained unchanged. So, to me this look like RTBC.Only one, most likely theoretical question: the field
date__durationis a mediumint. Should that also become a bigint? Only if an event has a duration of over 68 years. On the other hand, if someone used that for age calculation, this could easily happen.Comment #10
mandclu commentedHmmm interesting question. According to the integer types documentation an unsigned medium integer can go as high as 16777215, which converted from minutes to years is ~31.9. A normal integer could go as high as 4294967295, or ~8166 years.
The only logical use case that comes to mind for needing to store durations of 32 years or longer would be storing people's life spans in a single field, which seems like an edge case. I understand the motivation to potentially update the duration while we're making database updates, but I'm inclined to say let's hold off increasing the duration data storage for the vast majority of site that don't need it. If somehow we end up we a groundswell of sites that require much longer durations, we can create another database update without much extra effort.
Comment #11
jurgenhaasAbsolutely, that's fine. So, there is that RTBC +1 from my side, not sure if anyone else wanted to give it another review before changing status.
Comment #12
mlncn commentedAgree this should go in as is.
But very good point jurgenhaas!
We have a potential project where artifact time periods might be stored as a date range. Might not, also, but there will definitely be a date start and a date end, where one item might be associated with a single day, or a year, or a decade, or a couple centuries for artifacts from back in BCE timescales.
Comment #13
mandclu commentedMerging this into a new 4.1.x branch so that this can incorporated into a beta release.
Comment #15
mandclu commentedMerged in. Thanks for the input on this.