Uploaded image for project: 'Qt'
  1. Qt
  2. QTBUG-117902

Dangerously error-prone QStringDecoder/QStringEncoder interfaces

    XMLWordPrintable

Details

    • 21
    • Foundation PM Staging

    Description

      This is a follow-up to QTBUG-117705 as requested in the comments.

      The decode/encode interfaces (the function call operator and functions named "decode" and "encode") of QStringDecoder and QStringEncoder are very error-prone, because they do not actually decode/encode upon call, but instead return objects which does the respective operation in their implicit conversion operator to the result type.

      The current design of these interfaces entails at least the following two problems:

      • the type confusion issue as explained in QTBUG-117705,
      • the fact that the actual encoding/decoding does not happen when the encode/decode functions are called.

      Here's and example of bad code this lazy decoding logic and the misleading function names could result in:

      QByteArray encoded("\xc3\xb5"); // Same as "\u00f5a", Latin small letter O with tilde
      QStringDecoder decoder(QStringDecoder::Utf8);
      
      // User expects decoding to happen here and this to be a QString, but decoding doesn't actually occur:
      auto something = decoder.decode(encoded.left(1));
      
      if (!decoder.hasError()) { // Branch taken, because contrary to user expectations of decode() to decode, no decoding actually occurred
           // Since decoder did not error, the user might expect the next character to decode properly as well, however:
          QString somethingElse = decoder.decode(encoded.mid(1));
          Q_ASSERT(!decoder.hasError()); // Assertion fires
      }
      

      Also, the current implementation has a bug whereby uses like the following would result in undefined behavior (use-after-free):

      auto const decode = [](QByteArray const & encoded) {
        return QStringDecoder(QStringDecoder::Utf8).decode(encoded);
      };
      // The following results in an use-after-free in EncodedData<const QByteArray &>::operator QString(),
      // because decode("ab").decoder points to the QStringDecoder whose lifetime ended on return from the lambda:
      QString decoded = decode("ab");
      

      I propose the EncodedData<T> and DecodedData<T> templates to be burninated as soon as possible, and that the functions would be changed to instead return QString and QByteArray objects directly.

      Attachments

        No reviews matched the request. Check your Options in the drop-down menu of this sections header.

        Activity

          People

            cnn Qt Core & Network
            jotik Jaak Ristioja
            Vladimir Minenko Vladimir Minenko
            Alex Blasche Alex Blasche
            Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

              Created:
              Updated:

              Gerrit Reviews

                There are no open Gerrit changes