Menu ▾ ▴

#1180 Please check your email (security report); also a chat-history data-loss bug and a quote-jump idea

5.0
accepted
bugs (1)
4 days ago
4 days ago
Anonymous
No

Hi Marco,

Please check your email first. I sent a private security report with patches to your Gmail address. If it isn't in your inbox, it may have landed in spam. I'm keeping those details off SourceForge until you've had time to release a fix.

Everything else in this post is fine to make public: a data-loss bug with a patch, and a feature idea.

1. Chat history can be overwritten after a failed load

Version: 5.9.2 (SVN r1609), all platforms. The files affected are beebeep.dat and beebeep.bak.

What happens

If BeeBEEP can't read beebeep.dat at startup, it starts with an empty or partial history. That can happen when the file was cut short by a crash, or when the data folder is on a network drive that's slow to connect at logon. About a minute later, the autosave writes that empty or partial history to beebeep.bak, and on exit it's written to beebeep.dat too. At that point both copies of the real history are gone.

Why it happens

Line numbers refer to r1609.

  • BuildSavedChatList.cpp:121-141: a read error is only logged. Nothing falls back to beebeep.bak, and nothing records that loading failed.
  • SaveChatList.cpp:93-125: the save writes a .tmp file without checking for write errors. It then deletes beebeep.dat and renames the .tmp, so for a moment no file exists at all. The .tmp is also registered as a temporary file, so if the rename fails, the only new copy gets deleted at exit.
  • MessageManager.cpp:119-206: beebeep.off (unsent messages) is truncated and rewritten in place on every change.

Suggested fix (patch attached)

  • Save beebeep.dat, beebeep.bak and beebeep.off with QSaveFile and commit(), checking every write for errors.
  • When a file can't be read, copy it aside as <file>.unreadable-<date> before anything else touches it, then try the other copy.

The patch starts with a note on what it changes and what it leaves alone, and it applies with svn patch from the working-copy root. It builds on macOS with Qt 5.15 with no errors or new warnings. I haven't run it or tested it on Windows, so please treat it as a reviewed suggestion.

One case it doesn't cover: if the network drive is completely unreachable at startup, BeeBEEP can't see the files at all. Handling that would need a "data folder not available, so don't save" check.

2. Feature idea: clicking a quoted message scrolls to the original

Right now, "Quote selected text" copies the selected text into the message box as [quote]…[/quote]. It shows up as a coloured, italic block, but clicking it does nothing. In a busy group chat it's hard to find what someone was replying to. Most chat apps let you click a quote to jump to the original message, and I'd love to see that in BeeBEEP.

What I found in the code

In case it helps:

  • A quote stores only text (GuiChat::quoteSelectedText, GuiChat.cpp:1323). Messages do have an id on the wire (Message::id()), but ChatMessage::createFromMessage drops it, and rendered messages have no anchors.
  • A small version could work like this:
  • Keep the message id in ChatMessage. Ids are only unique per sender, so the key would be author plus id.
  • When quoting, add a reference to the quoted message, for example [quote=<author>:<id>] or a separate data field. Older clients would still show a plain quote.
  • Give each rendered message a named anchor.
  • Show the quote as a link with a private scheme, for example beequote:#<anchor>.
  • In GuiChat::checkAnchorClicked, catch that scheme and call scrollToAnchor().
  • If the original message isn't loaded (in older history, for example), show a short note instead.

A message with an unclosed [quote] leaves the italic style switched on. When the whole chat is redrawn, later messages from other people show up in italics.

Thanks for BeeBEEP!

1 Attachments

Discussion

  • Marco Mastroddi

    Marco Mastroddi - 4 days ago

    Hi,
    Thank you very much for the detailed report, the patch and the feature suggestion. I really appreciate the time you put into this.

    Unfortunately your email never reached me. I checked my inbox and the spam folder too, but there's nothing there. Could you please send it again?

    In the meantime I'll start looking into the chat history issue.

    Thanks again!

     
  • Marco Mastroddi

    Marco Mastroddi - 4 days ago

    Hi,

    Thanks again for the detailed report and the patch. Here's what I've done:

    1. Chat history lost after a failed load (r1610)
      I applied your patch with two small changes:
    2. Chats are now marked as saved only after the file has been written successfully, so if a save fails, the next autosave tries again.
    3. A negative chat count in the file is treated as corrupt data, so the other copy is loaded instead.

    beebeep.dat, beebeep.bak and beebeep.off are now saved with QSaveFile. An unreadable file is copied aside as <file>.unreadable-<date> before the other copy is loaded. As you pointed out, a data folder that is completely unreachable at startup is still not handled. I'll look into it separately.</date></file>

    1. Unclosed [quote] turning later messages to italics (r1611)
      Fixed: only complete quotes are converted now. A [quote] that is never closed gets closed at the end of the message, and a [/quote] with no matching [quote] is dropped.

    Both fixes will be in BeeBEEP 5.9.2.

    1. Clicking a quote to jump to the original message
      It's a good idea, but it will have to wait. Right now I'm focusing on BeeBEEP 6.0.0, which will have quotes and replies working much like WhatsApp or Telegram. Your analysis of the 5.x code will still be useful when I add something similar there.

    Thank you very much for all your support and for the time you've spent on BeeBEEP.

     
  • Anonymous

    Anonymous - 4 days ago

    Hi, I'm so sorry about that. I have just sent the email in within the past 10 minutes. I forgot to hit send this morning. I am a moron.

     

Anonymous
Anonymous

Add attachments
Cancel