Fix QCborStreamReader not flushing QIODevices due to internal buffering

When successfully finishing a parse, it's reasonable to expect that the
QIODevice was advanced to the end of the input data.

[ChangeLog][QtCore][QCborStreamReader] Fixed a bug that caused the
QIODevice that the data was being read from not to show the entire CBOR
message as consumed. This allows the user to consume data that may
follow the CBOR payload.

Fixes: QTBUG-77076
Change-Id: I1024ee42da0c4323953afffd15b23f5d8fcc6f50
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
Reviewed-by: BogDan Vatra <bogdan@kdab.com>
This commit is contained in:
Thiago Macieira 2019-07-17 09:41:31 -07:00
parent 89e0c2854a
commit 65f9646583
3 changed files with 177 additions and 20 deletions

View File

@ -1956,7 +1956,15 @@ inline void QCborStreamReader::preparse()
if (lastError() == QCborError::NoError) {
type_ = cbor_value_get_type(&d->currentElement);
if (type_ != CborInvalidType) {
if (type_ == CborInvalidType) {
// We may have reached the end.
if (d->device && d->containerStack.isEmpty()) {
d->buffer.clear();
if (d->bufferStart)
d->device->skip(d->bufferStart);
d->bufferStart = 0;
}
} else {
d->lastError = {};
// Undo the type mapping that TinyCBOR does (we have an explicit type
// for negative integer and we don't have separate types for Boolean,

View File

@ -80,6 +80,11 @@ private Q_SLOTS:
void addData_singleElement();
void addData_complex_data() { arrays_data(); }
void addData_complex();
void duplicatedData_data() { arrays_data(); }
void duplicatedData();
void extraData_data() { arrays_data(); }
void extraData();
};
#define FOR_CBOR_TYPE(F) \
@ -480,6 +485,28 @@ static QString parseOne(QCborStreamReader &reader)
return result;
}
static QString parse(QCborStreamReader &reader, const QByteArray &data)
{
qint64 oldPos = 0;
if (QIODevice *dev = reader.device())
oldPos = dev->pos();
QString r = parseOne(reader);
if (r.isEmpty())
return r;
if (reader.currentOffset() - oldPos != data.size())
r = QString("Number of parsed bytes (%1) not expected (%2)")
.arg(reader.currentOffset()).arg(data.size());
if (QIODevice *dev = reader.device()) {
if (dev->pos() - oldPos != data.size())
r = QString("QIODevice not advanced (%1) as expected (%2)")
.arg(dev->pos()).arg(data.size());
}
return r;
}
bool parseNonRecursive(QString &result, bool &printingStringChunks, QCborStreamReader &reader)
{
while (reader.lastError() == QCborError::NoError) {
@ -612,13 +639,13 @@ void tst_QCborStreamReader::fixed()
}
QVERIFY(reader.isValid());
QCOMPARE(reader.lastError(), QCborError::NoError);
QCOMPARE(parseOne(reader), expected);
QCOMPARE(parse(reader, data), expected);
// verify that we can re-read
reader.reset();
QVERIFY(reader.isValid());
QCOMPARE(reader.lastError(), QCborError::NoError);
QCOMPARE(parseOne(reader), expected);
QCOMPARE(parse(reader, data), expected);
}
void tst_QCborStreamReader::strings_data()
@ -721,7 +748,7 @@ void tst_QCborStreamReader::emptyContainers()
QCOMPARE(reader.lastError(), QCborError::NoError);
if (reader.isLengthKnown())
QCOMPARE(reader.length(), 0U);
QCOMPARE(parseOne(reader), expected);
QCOMPARE(parse(reader, data), expected);
// verify that we can re-read
reader.reset();
@ -729,7 +756,7 @@ void tst_QCborStreamReader::emptyContainers()
QCOMPARE(reader.lastError(), QCborError::NoError);
if (reader.isLengthKnown())
QCOMPARE(reader.length(), 0U);
QCOMPARE(parseOne(reader), expected);
QCOMPARE(parse(reader, data), expected);
}
void tst_QCborStreamReader::arrays_data()
@ -758,7 +785,7 @@ static void checkContainer(int len, const QByteArray &data, const QString &expec
QVERIFY(reader.isLengthKnown());
QCOMPARE(reader.length(), uint(len));
}
QCOMPARE(parseOne(reader), expected);
QCOMPARE(parse(reader, data), expected);
// verify that we can re-read
reader.reset();
@ -768,7 +795,7 @@ static void checkContainer(int len, const QByteArray &data, const QString &expec
QVERIFY(reader.isLengthKnown());
QCOMPARE(reader.length(), uint(len));
}
QCOMPARE(parseOne(reader), expected);
QCOMPARE(parse(reader, data), expected);
}
void tst_QCborStreamReader::arrays()
@ -892,7 +919,7 @@ void tst_QCborStreamReader::validation()
buffer.open(QIODevice::ReadOnly);
reader.setDevice(&buffer);
}
parseOne(reader);
parse(reader, data);
QVERIFY(reader.lastError() != QCborError::NoError);
// next() should fail
@ -997,7 +1024,7 @@ void tst_QCborStreamReader::addData_singleElement()
reader.addData(data.constData() + i, 1);
}
parseOne(reader);
parse(reader, data);
QCOMPARE(reader.lastError(), QCborError::EndOfFile);
}
@ -1009,7 +1036,7 @@ void tst_QCborStreamReader::addData_singleElement()
reader.addData(data.right(1));
}
QCOMPARE(reader.lastError(), QCborError::NoError);
QCOMPARE(parseOne(reader), expected);
QCOMPARE(parse(reader, data), expected);
}
void tst_QCborStreamReader::addData_complex()
@ -1085,6 +1112,69 @@ void tst_QCborStreamReader::addData_complex()
"{1, [" + expected + ", " + expected + "]}");
}
void tst_QCborStreamReader::duplicatedData()
{
QFETCH_GLOBAL(bool, useDevice);
QFETCH(QByteArray, data);
QFETCH(QString, expected);
removeIndicators(expected);
// double the data up
QByteArray doubledata = data + data;
QBuffer buffer(&doubledata);
QCborStreamReader reader(doubledata);
if (useDevice) {
buffer.open(QIODevice::ReadOnly);
reader.setDevice(&buffer);
}
QVERIFY(reader.isValid());
QCOMPARE(reader.lastError(), QCborError::NoError);
QCOMPARE(parse(reader, data), expected); // yes, data
QVERIFY(reader.currentOffset() < doubledata.size());
if (useDevice) {
reader.setDevice(&buffer);
QVERIFY(reader.isValid());
QCOMPARE(reader.lastError(), QCborError::NoError);
QCOMPARE(parse(reader, data), expected);
QCOMPARE(buffer.pos(), doubledata.size());
} else {
// there's no reader.setData()
}
}
void tst_QCborStreamReader::extraData()
{
QFETCH_GLOBAL(bool, useDevice);
QFETCH(QByteArray, data);
QFETCH(QString, expected);
removeIndicators(expected);
QByteArray extension(9, '\0');
// stress test everything with extra bytes (just one byte changing;
// TinyCBOR used to have a bug where the next byte got sometimes read)
for (int c = '\0'; c < 0x100; ++c) {
extension[0] = c;
QByteArray extendeddata = data + extension;
QBuffer buffer(&extendeddata);
QCborStreamReader reader(extendeddata);
if (useDevice) {
buffer.open(QIODevice::ReadOnly);
reader.setDevice(&buffer);
}
QVERIFY(reader.isValid());
QCOMPARE(reader.lastError(), QCborError::NoError);
QCOMPARE(parse(reader, data), expected); // yes, data
// if we were a parser, we could parse the next payload
if (useDevice)
QCOMPARE(buffer.readAll(), extension);
}
}
QTEST_MAIN(tst_QCborStreamReader)

View File

@ -91,8 +91,14 @@ private slots:
void toCbor_data();
void toCbor();
void toCborStreamWriter_data() { toCbor_data(); }
void toCborStreamWriter();
void fromCbor_data();
void fromCbor();
void fromCborStreamReaderByteArray_data() { fromCbor_data(); }
void fromCborStreamReaderByteArray();
void fromCborStreamReaderIODevice_data() { fromCbor_data(); }
void fromCborStreamReaderIODevice();
void validation_data();
void validation();
void toDiagnosticNotation_data();
@ -1395,6 +1401,22 @@ void tst_QCborValue::toCbor()
"\xa1\x01\xd9\xd9\xf7" + result);
}
void tst_QCborValue::toCborStreamWriter()
{
QFETCH(QCborValue, v);
QFETCH(QByteArray, result);
QFETCH(QCborValue::EncodingOptions, options);
QByteArray output;
QBuffer buffer(&output);
buffer.open(QIODevice::WriteOnly);
QCborStreamWriter writer(&buffer);
v.toCbor(writer, options);
QCOMPARE(buffer.pos(), result.size());
QCOMPARE(output, result);
}
void tst_QCborValue::fromCbor_data()
{
addCommonCborData();
@ -1425,20 +1447,11 @@ void tst_QCborValue::fromCbor_data()
<< raw("\xd8\x25\x51" "\1\2\3\4""\4\3\2\0""\0\0\0\0""\0\0\0\1""\2");
}
void tst_QCborValue::fromCbor()
void fromCbor_common(void (*doCheck)(const QCborValue &, const QByteArray &))
{
QFETCH(QCborValue, v);
QFETCH(QByteArray, result);
auto doCheck = [](const QCborValue &v, const QByteArray &result) {
QCborParserError error;
QCborValue decoded = QCborValue::fromCbor(result, &error);
QVERIFY2(error.error == QCborError(), qPrintable(error.errorString()));
QCOMPARE(error.offset, result.size());
QVERIFY(decoded == v);
QVERIFY(v == decoded);
};
doCheck(v, result);
if (QTest::currentTestFailed())
return;
@ -1489,6 +1502,52 @@ void tst_QCborValue::fromCbor()
return;
}
void tst_QCborValue::fromCbor()
{
auto doCheck = [](const QCborValue &v, const QByteArray &result) {
QCborParserError error;
QCborValue decoded = QCborValue::fromCbor(result, &error);
QVERIFY2(error.error == QCborError(), qPrintable(error.errorString()));
QCOMPARE(error.offset, result.size());
QVERIFY(decoded == v);
QVERIFY(v == decoded);
};
fromCbor_common(doCheck);
}
void tst_QCborValue::fromCborStreamReaderByteArray()
{
auto doCheck = [](const QCborValue &expected, const QByteArray &data) {
QCborStreamReader reader(data);
QCborValue decoded = QCborValue::fromCbor(reader);
QCOMPARE(reader.lastError(), QCborError());
QCOMPARE(reader.currentOffset(), data.size());
QVERIFY(decoded == expected);
QVERIFY(expected == decoded);
};
fromCbor_common(doCheck);
}
void tst_QCborValue::fromCborStreamReaderIODevice()
{
auto doCheck = [](const QCborValue &expected, const QByteArray &data) {
QBuffer buffer;
buffer.setData(data);
buffer.open(QIODevice::ReadOnly);
QCborStreamReader reader(&buffer);
QCborValue decoded = QCborValue::fromCbor(reader);
QCOMPARE(reader.lastError(), QCborError());
QCOMPARE(reader.currentOffset(), data.size());
QVERIFY(decoded == expected);
QVERIFY(expected == decoded);
QCOMPARE(buffer.pos(), reader.currentOffset());
};
fromCbor_common(doCheck);
}
void tst_QCborValue::validation_data()
{
addValidationColumns();