-
Notifications
You must be signed in to change notification settings - Fork 17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
interacting with a toot/microblog with invalid time data fails #622
Comments
Main problem is probably that the post says it is posted in 2000 years from now, so yeah we should not accept created dates in the future. I don't know why that leads to an error when liking the post though |
I think it would be best to preserve the invalid timestamp in one form or another, and to expose it to the user, but allow the actual timestamp used for computation to not exceed the time the post was first created/federated. |
What benefit is in displaying the invalid timestamp we got to the user? |
My suggestion to display the invalid time (in addition to the timestamp the backend assigned to the post for computation) is informed by my dislike for hard-coding assumptions about what the user does and doesn't want or need to see. Personally as a user, knowing that the post linked above has a timestamp for 2000 years into the future, I wouldn't want that information being hidden from me. Another argument for keeping and exposing the invalid timestamp is to ensure parity between different fediverse software. The post above was made on a platform other than Mbin and on that platform, the invalid timestamp is displayed. Not displaying that timestamp on Mbin could lead to confusion. |
Um, looks pretty serious, I was doing some interaction, tried replying and then after three attempts got this
|
Can't the timestamp be fixed when it's in the future? Inconsistent yes, but what's the point of having a timestamp from the future. Maybe add a line to the post saying the timestamp was fixed? |
added a cap on creation timestamp used in ranking calculation to not exceeds the current time while performing calculation to cope with posts' that have funny created date far into the future, also cap the final score value to not exceed int32 max size (2**31 - 1) the current ranking function involves using the posts' object creation timestamp as part of the calculation, but if the datetime is too far in the future (e.g. 4200-06-09, as seen in [this example][ex1]), it can cause the ranking score value to exceeds the ranking score field size, which appears to be int32 sized technically we'll still be running into year 2038 problem on this field, but there's some discussion on adjusting the ranking score and keep that below int32 sized altogether, so that's left for the future exercise also see #622 for more context [ex1]: https://myfriendsare.gay/notice/APx5K7aeglknMfm1A0
I was thinking about having an asterisk behind the valid timestamp, which when hovered over/clicked on, brings up a context field, reading "Alleged timestamp: [invalid timestamp]". But I also understand that simply replacing the invalid timestamp with a valid one upon first receiving the post through federation is far easier to implement, akin to 2-3 lines of code. |
I changed the labels because the bug part of the issue has been solved by asdfzdfj |
I'd argue that, while the change might prevent exceptions to occur, it still gives an unfair advantage to posts that have their creation date set to the future, as this fix will weight them as if they have been created right now, which still gives those posts an advantage when sorting by 'new' over regular posts. In my mind, having posts with invalid timestamps perpetually come out on top when sorting by 'new' is undesirable and therefore still a bug to the user. |
That is a 100% true 🤔 |
Unless I am mistaken about when and how often the weight for each post is (re)calculated. If the weight is calculated only when receiving the post, then this fix is fine, as the weight will never update. |
No you were right, this is a problem, because the timestamp that is supplied is used to calculate the weight (or |
This issue is stale because it has been open 50 days with no activity. Remove stale label or comment or this will be closed in 6 days. |
This issue is stale because it has been open 50 days with no activity. Remove stale label or comment or this will be closed in 6 days. |
Describe the bug
Trying to comment on or upvote a toot with invalid time data leads to server errors.
Edit: Also, the toot/microblog seems to have been made over a year ago, yet was shown first when sorting by
new
. I think erroneous fields in posts/microblogs should be safeguarded against.On which Mbin instance did you find the bug?
fedia.io
Which Mbin version was running on the instance?
1.4.1
To Reproduce
Steps to reproduce the behavior:
Expected behavior
Better error handling with an explicit error message.
Alternatively, ignore invalid time data and allow normal interaction.
The text was updated successfully, but these errors were encountered: