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

fix(chatform): sanitize the toxstring to remove characters after the \0 #513

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

Conversation

nickolay168
Copy link

@nickolay168 nickolay168 commented Feb 8, 2025

In Qt6 when we convert QByteArray to string, \0 is considered as a valid character and do not end the string. Some messengers as TRIfA may provide additional information after the 0-th symbol, resulting in unexpected output of non readable symbols.
By default TRIfA v3 message will have the next structure:
message
2 bytes \0 (guard)
32 bytes of random values
4 bytes timestamp
See source
In This PR we are adding option to hide this line, or show it (the content of the line is saved in chat history).


This change is Reviewable

@github-actions github-actions bot added the bug Bug fix for the user, not a fix to a build script label Feb 8, 2025
Copy link

github-actions bot commented Feb 8, 2025

@nickolay168 nickolay168 force-pushed the nickolay168_sanitize_string branch from 893ec25 to c11e1b1 Compare February 9, 2025 00:00
@nickolay168 nickolay168 changed the title fix(chatform) sanitize the toxstring to remove characters after the \0 fix(chatform): sanitize the toxstring to remove characters after the \0 Feb 9, 2025
@iphydf
Copy link
Member

iphydf commented Feb 9, 2025

I don't think we should do this. Getting this right is tricky, because as implemented in this PR, this allows users to add secret messages behind a \0 that only some clients can read and qTox hides from its users. That's particularly relevant in group chats.

@nickolay168
Copy link
Author

nickolay168 commented Feb 9, 2025

I don't think we should do this. Getting this right is tricky, because as implemented in this PR, this allows users to add secret messages behind a \0 that only some clients can read and qTox hides from its users. That's particularly relevant in group chats.

Yes, that sounds right. But how we should handle the TRIFA messages as they will merely always contain symbols after \0? Or may be, we can create an option in UI?

@iphydf
Copy link
Member

iphydf commented Feb 9, 2025

Yeah, and maybe we can find out what format it's sending and make that option more specific so it filters out trifa stuff, not just anything after \0. Also we should still display something (like a coloured [...] to let users know there's something there. Then when the user disables the option (and restarts), the text should be there again, similar to the style options.

@nickolay168
Copy link
Author

Yeah, and maybe we can find out what format it's sending and make that option more specific so it filters out trifa stuff, not just anything after \0. Also we should still display something (like a coloured [...] to let users know there's something there. Then when the user disables the option (and restarts), the text should be there again, similar to the style options.

I have added the new option and corresponding tests, sorry it took so long :)

@nickolay168 nickolay168 marked this pull request as ready for review February 26, 2025 06:04
<property name="text">
<string>Chat log:</string>
<string>Hide TRIfA suffix</string>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it actually TRIfA-specific? Can we call it something more generic, like post-null suffix or something?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug fix for the user, not a fix to a build script
Development

Successfully merging this pull request may close these issues.

2 participants