-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Wrap XML data into QString to fix encoding #3921
Wrap XML data into QString to fix encoding #3921
Conversation
The QTextStream should be setup to output in UTF-8 encoding. |
It outputs whatever your locale encoding is. On Linux that's usually utf-8. |
That would imply that the issue originator does not have UTF-8 as their locale? |
Possibly. We had similar issues with the password generator on Windows, which I fixed a while ago by forcing the console encoding to Unicode instead of cp850. L Edit: in this specific case, it seems to be a problem with the << operator, which is supposedly fixed on Qt6. Since we are feeding constData() into the TextStream, it's const char*, not a QString. So the problem is not the output of the TextStream, but its input |
Yes indeed - that's why I said that I think the proper fix is to overwrite the |
Why not just use write(..) function instead of the operator |
TBH, I don't like this write method. The << operator is the common idiom used everywhere and would not be clear why sometimes we use one and sometimes the other. I would prefer overloading <<. The distinction between C strings and Qt strings is bad enough as it is, we don't need to reinforce it. |
It seems like I reached the end of my C++ abilities - I'm unable to overwrite the << operator properly. I tried the below, but it doesn't work. Feel free to make changes to this PR. In template<typename T>
TextStream &operator<<(T const& t);
TextStream &operator<<(const char *c); In template<typename T>
TextStream &TextStream::operator<<(const T& t)
{
return static_cast<TextStream&>(static_cast<QTextStream&>(*this) << t);
}
TextStream &TextStream::operator<<(const char* str)
{
return static_cast<TextStream&>(*this << str);
} |
@@ -58,6 +58,11 @@ TextStream::TextStream(const QByteArray& array, QIODevice::OpenMode openMode) | |||
detectCodec(); | |||
} | |||
|
|||
void TextStream::write(const char* str) | |||
{ | |||
*this << QString(str); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a comment above this line explaining that this is a workaround for a Qt issue that is resolved in Qt 6.x.
Done. |
Add write function to TextStream Fix keepassxreboot#3900
973f926
to
afb1eb2
Compare
Added - Custom Light and Dark themes [#4110, #4769, #4791, #4796, #4892, #4915] - Compact mode to use classic Group and Entry line height [#4910] - View menu to quickly switch themes, compact mode, and toggle UI elements [#4910] - Search for groups and scope search to matched groups [#4705] - Save Database Backup feature [#4550] - Sort entries by "natural order" and move lines up/down [#4357] - Option to launch KeePassXC on system startup/login [#4675] - Caps Lock warning on password input fields [#3646] - Add "Size" column to entry view [#4588] - Browser-like tab experience using Ctrl+[Num] (Alt+[Num] on Linux) [#4063, #4305] - Password Generator: Define additional characters to choose from [#3876] - Reports: Database password health check (offline) [#3993] - Reports: HIBP online service to check for breached passwords [#4438] - Auto-Type: DateTime placeholders [#4409] - Browser: Show group name in results sent to browser extension [#4111] - Browser: Ability to define a custom browser location (macOS and Linux only) [#4148] - Browser: Ability to change root group UUID and inline edit connection ID [#4315, #4591] - CLI: `db-info` command [#4231] - CLI: Use wl-clipboard if xclip is not available (Linux) [#4323] - CLI: Incorporate xclip into snap builds [#4697] - SSH Agent: Key file path env substitution, SSH_AUTH_SOCK override, and connection test [#3769, #3801, #4545] - SSH Agent: Context menu actions to add/remove keys [#4290] Changed - Complete replacement of default database icons [#4699] - Complete replacement of application icons [#4066, #4161, #4203, #4411] - Complete rewrite of documentation and manpages using Asciidoctor [#4937] - Complete refactor of config files; separate between local and roaming [#4665] - Complete refactor of browser integration and proxy code [#4680] - Complete refactor of hardware key integration (YubiKey and OnlyKey) [#4584, #4843] - Significantly improve performance when saving and opening databases [#4309, #4833] - Remove read-only detection for database files [#4508] - Overhaul of password fields and password generator [#4367] - Replace instances of "Master Key" with "Database Credentials" [#4929] - Change settings checkboxes to positive phrasing for consistency [#4715] - Improve UX of using entry actions (focus fix) [#3893] - Set expiration time to Now when enabling entry expiration [#4406] - Always show "New Entry" in context menu [#4617] - Issue warning before adding large attachments [#4651] - Improve importing OPVault [#4630] - Improve AutoOpen capability [#3901, #4752] - Check for updates every 7 days even while still running [#4752] - Improve Windows installer UI/UX [#4675] - Improve config file handling of portable distribution [#4131, #4752] - macOS: Hide dock icon when application is hidden to tray [#4782] - Browser: Use unlock dialog to improve UX of opening a locked database [#3698] - Browser: Improve database and entry settings experience [#4392, #4591] - Browser: Improve confirm access dialog [#2143, #4660] - KeeShare: Improve monitoring file changes of shares [#4720] - CLI: Rename `create` command to `db-create` [#4231] - CLI: Cleanup `db-create` options (`--set-key-file` and `--set-password`) [#4313] - CLI: Use stderr for help text and password prompts [#4086, #4623] - FdoSecrets: Display existing secret service process [#4128] Fixed - Fix changing focus around the main window using tab key [#4641] - Fix search field clearing while still using the application [#4368] - Improve search help widget displaying on macOS and Linux [#4236] - Return keyboard focus after editing an entry [#4287] - Reset database path after failed "Save As" [#4526] - Use SHA256 Digest for Windows code signing [#4129] - Improve handling of ccache when building [#4104, #4335] - macOS: Properly re-hide application window after browser integration and Auto-Type usage [#4909] - Auto-Type: Fix crash when performing on new entry [#4132] - Browser: Send legacy HTTP settings to recycle bin [#4589] - Browser: Fix merging browser keys [#4685] - CLI: Fix encoding when exporting database [#3921] - SSH Agent: Improve reliability and underlying code [#3833, #4256, #4549, #4595] - FdoSecrets: Fix crash when editing settings before service is enabled [#4332]
Fix encoding problem when exporting the database into XML format.
Type of change
Description and Context
Fixes #3900
Root case explained here: https://stackoverflow.com/questions/40318671/streaming-utf-8-literals-into-qtextstream
Disclaimer: I do not consider my change a proper fix. It works (correct XML is outputted), but I suspect the proper fix would be to overwrite the
<<
operator of theTextStream
class, and wrap the output into aQString
there. However, I am not 100% sure about the implications of this change, so I just did this small workaround to highlight the problem for you. It fixes the original issue that was raised, but does not address the underlying problem (explained in the stack overflow article).Testing strategy
Checklist:
-DWITH_ASAN=ON
. [REQUIRED]