diff options
author | Ulf Hermann <ulf.hermann@qt.io> | 2022-01-14 13:41:20 +0100 |
---|---|---|
committer | Ulf Hermann <ulf.hermann@qt.io> | 2022-01-19 20:06:11 +0100 |
commit | 47d77dd552b8ee1394e94a90f71f44c12f631f7d (patch) | |
tree | 404b1e5d2b60649dd013283338c951fa16ffb16b /src/corelib/serialization/qjsonparser.cpp | |
parent | 5a86aa3b17bd9a6a1bf6368d8d74d17b13bced38 (diff) | |
download | qtbase-47d77dd552b8ee1394e94a90f71f44c12f631f7d.tar.gz |
JSON: When clearing duplicate object entries, also clear containers
Previously, if you had multiple entries with the same name in an object,
and some of them were again objects or arrays, parsing the JSON document
would leak memory.
Also, we use std::stable_sort instead of std::sort now, so that we don't
accidentally randomize the order of elements with equal keys.
[ChangeLog][QtCore][JSON] A memory leak in the JSON parser when reading
objects with duplicate keys was fixed.
Fixes: QTBUG-99799
Change-Id: Ic2065f2e490c2d3506a356745542148ad9c24262
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
(cherry picked from commit 1b4a5ecc91c87ffe7869d233c00a37d1b5b9cb13)
Reviewed-by: Marc Mutz <marc.mutz@qt.io>
Diffstat (limited to 'src/corelib/serialization/qjsonparser.cpp')
-rw-r--r-- | src/corelib/serialization/qjsonparser.cpp | 54 |
1 files changed, 44 insertions, 10 deletions
diff --git a/src/corelib/serialization/qjsonparser.cpp b/src/corelib/serialization/qjsonparser.cpp index 7421902135..da0535cef2 100644 --- a/src/corelib/serialization/qjsonparser.cpp +++ b/src/corelib/serialization/qjsonparser.cpp @@ -379,10 +379,30 @@ error: return QCborValue(); } +// We need to retain the _last_ value for any duplicate keys and we need to deref containers. +// Therefore the manual implementation of std::unique(). +template<typename Iterator, typename Compare, typename Assign> +static Iterator customAssigningUniqueLast(Iterator first, Iterator last, + Compare compare, Assign assign) +{ + first = std::adjacent_find(first, last, compare); + if (first == last) + return last; + + Iterator result = first; + while (++first != last) { + if (!compare(*result, *first)) + ++result; + if (result != first) + assign(*result, *first); + } + + return ++result; +} + static void sortContainer(QCborContainerPrivate *container) { using Forward = QJsonPrivate::KeyIterator; - using Reverse = std::reverse_iterator<Forward>; using Value = Forward::value_type; auto compare = [container](const Value &a, const Value &b) @@ -420,17 +440,31 @@ static void sortContainer(QCborContainerPrivate *container) } }; - std::sort(Forward(container->elements.begin()), Forward(container->elements.end()), - [&compare](const Value &a, const Value &b) { return compare(a, b) < 0; }); + // The elements' containers are owned by the outer container, not by the elements themselves. + auto move = [](Forward::reference target, Forward::reference source) + { + QtCbor::Element &targetValue = target.value(); + + // If the target has a container, deref it before overwriting, so that we don't leak. + if (targetValue.flags & QtCbor::Element::IsContainer) + targetValue.container->deref(); + + // Do not move, so that we can clear the value afterwards. + target = source; + + // Clear the source value, so that we don't store the same container twice. + source.value() = QtCbor::Element(); + }; + + std::stable_sort( + Forward(container->elements.begin()), Forward(container->elements.end()), + [&compare](const Value &a, const Value &b) { return compare(a, b) < 0; }); - // We need to retain the _last_ value for any duplicate keys. Therefore the reverse dance here. - auto it = std::unique(Reverse(container->elements.end()), Reverse(container->elements.begin()), - [&compare](const Value &a, const Value &b) { - return compare(a, b) == 0; - }).base().elementsIterator(); + Forward result = customAssigningUniqueLast( + Forward(container->elements.begin()), Forward(container->elements.end()), + [&compare](const Value &a, const Value &b) { return compare(a, b) == 0; }, move); - // The erase from beginning is expensive but hopefully rare. - container->elements.erase(container->elements.begin(), it); + container->elements.erase(result.elementsIterator(), container->elements.end()); } |