woboq / qmetaobject-rs

Integrate Qml and Rust by building the QMetaObject at compile time.
MIT License
648 stars 89 forks source link

Fix manipulating UTF-8 strings in QJsonObject #279

Closed direc85 closed 1 year ago

direc85 commented 1 year ago

During developing Whisperfish, I noticed that pushing emojis into QJsonObject and reading them back didn't work correctly.

Code:

fn grouped_reactions(&self) -> QByteArray {
    let mut map = std::collections::HashMap::new();

    for (reaction, _) in &self.reaction_list.pinned().borrow().reactions {
        *map.entry(reaction.emoji.clone()).or_insert(0) += 1;
    }
    log::trace!("{:?}", map);
    let mut qmap: qmetaobject::QJsonObject = qmetaobject::QJsonObject::default();
    for (emoji, count) in map {
        qmap.insert(&emoji, QVariant::from(count).into());
    }
    log::trace!("{:?}", qmap.to_json());
    qmap.to_json()
}

Output before:

[2023-02-18T19:25:19Z TRACE whisperfish::model::reactions] {"😨": 1}
[2023-02-18T19:25:19Z TRACE whisperfish::model::reactions] {"ð

Output after:

[2023-02-18T20:40:38Z TRACE whisperfish::model::reactions] {"😨": 1}
[2023-02-18T20:40:38Z TRACE whisperfish::model::reactions] {"😨":1}

The cause was as I suspected: Usage of QString::fromLatin1(const char *str, qsizetype size) doesn't handle UTF-8 encoding correctly, but using QString::fromUtf8(const char8_t *str, qsizetype size) is able to handle it.

ogoffart commented 1 year ago

Thanks! the patch looks good. Could you write a test?

direc85 commented 1 year ago

First I tried to append the test to the function above, but I couldn't replicate the behavior. I then decided to take the gist of the erroneous code and make a new test out of it.

When I revert the fix, the test fails with

---- test_qjsonobject_utf8 stdout ----
thread 'test_qjsonobject_utf8' panicked at 'assertion failed: `(left == right)`
  left: `"{\"ð\u{9f}¦\u{80}\":1}"`,
 right: `"{\"🦀\":1}"`', qttypes/src/lib.rs:1784:5
ogoffart commented 1 year ago

Thank you!