Fix warnings in sequence wrapper code

Previously, the sequence wrapper had unsigned int / signed int
comparisons (due to Qt container classes only allowing signed int
indexes (where negative indexes are invalid).

This commit ensures that unsigned indexes are bounds checked
appropriately, and also fixes a warning due to QString construction
from QByteArray.  Finally, it updates the documentation for sequences
to clarify the indexing semantics.

Change-Id: I4c6e133bef6e980a9ccb62ff15a70a5d41537ee3
Reviewed-by: Martin Jones <martin.jones@nokia.com>
Reviewed-by: Michael Brasser <michael.brasser@nokia.com>

3 files changed, 133 insertions, 0 deletions

diff --git a/tests/auto/declarative/qdeclarativeecmascript/data/sequenceConversion.indexes.qml b/tests/auto/declarative/qdeclarativeecmascript/data/sequenceConversion.indexes.qml
new file mode 100644
index 0000000000..23f1e90417
--- /dev/null
+++ b/tests/auto/declarative/qdeclarativeecmascript/data/sequenceConversion.indexes.qml
@@ -0,0 +1,89 @@
+import QtQuick 2.0
+import Qt.test 1.0
+
+Item {
+    id: root
+    objectName: "root"
+
+    MySequenceConversionObject {
+        id: msco
+        objectName: "msco"
+    }
+
+    property bool success: false
+
+    function verifyExpected(array, idx) {
+        for (var i = 0; i < idx; ++i) {
+            if (array[i] != i) {
+                return false;
+            }
+        }
+        return true;
+    }
+
+    function indexedAccess() {
+        success = true;
+
+        msco.intListProperty = [ 0, 1, 2, 3, 4 ];
+        var expectedLength = msco.intListProperty.length;
+        var maxIndex = msco.maxIndex;
+        var tooBigIndex = msco.tooBigIndex;
+        var negativeIndex = msco.negativeIndex;
+
+        // shouldn't be able to set the length > maxIndex.
+        msco.intListProperty.length = tooBigIndex;
+        if (msco.intListProperty.length != expectedLength)
+            success = false;
+        if (!verifyExpected(msco.intListProperty, 4))
+            success = false;
+
+        // shouldn't be able to set any index > maxIndex.
+        msco.intListProperty[tooBigIndex] = 12;
+        if (msco.intListProperty.length != expectedLength)
+            success = false;
+        if (!verifyExpected(msco.intListProperty, 4))
+            success = false;
+
+        // shouldn't be able to access any index > maxIndex.
+        var valueAtTBI = msco.intListProperty[tooBigIndex];
+        if (valueAtTBI != undefined)
+            success = false;
+        if (!verifyExpected(msco.intListProperty, 4))
+            success = false;
+
+        // shouldn't be able to set the length to < 0
+        msco.intListProperty.length = negativeIndex;
+        if (msco.intListProperty.length != expectedLength)
+            success = false; // shouldn't have changed.
+        if (!verifyExpected(msco.intListProperty, 4))
+            success = false;
+
+        // shouldn't be able to set any index < 0.
+        msco.intListProperty[negativeIndex] = 12;
+        if (msco.intListProperty.length != expectedLength)
+            success = false;
+        if (!verifyExpected(msco.intListProperty, 4))
+            success = false;
+
+        // shouldn't be able to access any index < 0.
+        var valueAtNI = msco.intListProperty[negativeIndex];
+        if (valueAtNI != undefined)
+            success = false;
+        if (!verifyExpected(msco.intListProperty, 4))
+            success = false;
+
+        // NOTE: while these two operations are technically
+        // fine, we expect std::bad_alloc exceptions here
+        // which we handle in the sequence wrapper.
+        msco.intListProperty.length = maxIndex;
+        if (msco.intListProperty.length != expectedLength)
+            success = false;
+        if (!verifyExpected(msco.intListProperty, 4))
+            success = false;
+        msco.intListProperty[maxIndex] = 15;
+        if (msco.intListProperty.length != expectedLength)
+            success = false;
+        if (!verifyExpected(msco.intListProperty, 4))
+            success = false;
+    }
+}
diff --git a/tests/auto/declarative/qdeclarativeecmascript/testtypes.h b/tests/auto/declarative/qdeclarativeecmascript/testtypes.h
index d413209062..a463d3f64a 100644
--- a/tests/auto/declarative/qdeclarativeecmascript/testtypes.h
+++ b/tests/auto/declarative/qdeclarativeecmascript/testtypes.h
@@ -1194,6 +1194,10 @@ class MySequenceConversionObject : public QObject
     Q_PROPERTY (QList<QPoint> pointListProperty READ pointListProperty WRITE setPointListProperty NOTIFY pointListPropertyChanged)
     Q_PROPERTY (QList<QVariant> variantListProperty READ variantListProperty WRITE setVariantListProperty NOTIFY variantListPropertyChanged)
 
+    Q_PROPERTY (qint32 maxIndex READ maxIndex CONSTANT)
+    Q_PROPERTY (quint32 tooBigIndex READ tooBigIndex CONSTANT)
+    Q_PROPERTY (qint32 negativeIndex READ negativeIndex CONSTANT)
+
 public:
     MySequenceConversionObject()
     {
@@ -1211,6 +1215,21 @@ public:
 
     ~MySequenceConversionObject() {}
 
+    qint32 maxIndex() const
+    {
+        return INT_MAX;
+    }
+    quint32 tooBigIndex() const
+    {
+        quint32 retn = 7;
+        retn += INT_MAX;
+        return retn;
+    }
+    qint32 negativeIndex() const
+    {
+        return -5;
+    }
+
     QList<int> intListProperty() const { return m_intList; }
     void setIntListProperty(const QList<int> &list) { m_intList = list; emit intListPropertyChanged(); }
     QList<int> intListProperty2() const { return m_intList2; }
diff --git a/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp b/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp
index 02f79d2dd7..354087da8a 100644
--- a/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp
+++ b/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp
@@ -170,6 +170,7 @@ private slots:
     void sequenceConversionRead();
     void sequenceConversionWrite();
     void sequenceConversionArray();
+    void sequenceConversionIndexes();
     void sequenceConversionThreads();
     void sequenceConversionBindings();
     void sequenceConversionCopy();
@@ -4608,6 +4609,30 @@ void tst_qdeclarativeecmascript::sequenceConversionArray()
     delete object;
 }
 
+
+void tst_qdeclarativeecmascript::sequenceConversionIndexes()
+{
+    // ensure that we gracefully fail if unsupported index values are specified.
+    // Qt container classes only support non-negative, signed integer index values.
+    QUrl qmlFile = testFileUrl("sequenceConversion.indexes.qml");
+    QDeclarativeComponent component(&engine, qmlFile);
+    QObject *object = component.create();
+    QVERIFY(object != 0);
+    QString w1 = qmlFile.toString() + QLatin1String(":34: Index out of range during length set");
+    QString w2 = qmlFile.toString() + QLatin1String(":41: Index out of range during indexed set");
+    QString w3 = qmlFile.toString() + QLatin1String(":48: Index out of range during indexed get");
+    QString w4 = qmlFile.toString() + QLatin1String(":78: std::bad_alloc during length set");
+    QString w5 = qmlFile.toString() + QLatin1String(":83: std::bad_alloc during indexed set");
+    QTest::ignoreMessage(QtWarningMsg, qPrintable(w1));
+    QTest::ignoreMessage(QtWarningMsg, qPrintable(w2));
+    QTest::ignoreMessage(QtWarningMsg, qPrintable(w3));
+    QTest::ignoreMessage(QtWarningMsg, qPrintable(w4));
+    QTest::ignoreMessage(QtWarningMsg, qPrintable(w5));
+    QMetaObject::invokeMethod(object, "indexedAccess");
+    QVERIFY(object->property("success").toBool());
+    delete object;
+}
+
 void tst_qdeclarativeecmascript::sequenceConversionThreads()
 {
     // ensure that sequence conversion operations work correctly in a worker thread