Skip to content
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

client: Fix DayChange invalid index w/future msgs #603

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

digitalcircuit
Copy link
Contributor

In short

  • Fix crash when generating DayChange if there's only messages from the future
    • Can happen with IRCv3 server-time if server's desynced, or if the Quassel client or core have the wrong time
    • Potentially fixes crash reported on Fedora's bug tracker (could not reproduce locally)
  • Add comments to document code
Criteria Rank Reason
Impact ★★☆ 2/3 Fixes seemingly very rare crash
Risk ★☆☆ 1/3 Should only impact situations that always crashed, adds logging
Intrusiveness ★☆☆ 1/3 Small set of changes, shouldn't interfere with other pull requests

Testing

I tried to recreate this by patching the Quassel core to only send timestamps from the future, etc. Unfortunately, I could not reproduce this crash. At worst, this might just be ineffective.

Testing confirms that at least the debug message works if the Quassel client is patched to jump to it.

Fix crash when generating a DayChange message if somehow the message
model only has messages from the future.  Though unlikely, this may
happen with IRCv3 server-time if a server specifies timestamps in the
future.

Potentially fixes https://bugzilla.redhat.com/show_bug.cgi?id=2072127

Add comments to document this code.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant