Fix crash in String.arg()

This commit ensures that the String.arg() function works correctly,
by registering an anonymous function with the String Prototype object
which calls the StringArg function, ensuring that the "this" object
is valid (and passing the string as an argument to StringArg instead).

Change-Id: I0a8cbaa12b39beb03a237c3ab62c6e21fafdedbf
Reviewed-on: http://codereview.qt-project.org/4385
Reviewed-by: Qt Sanity Bot <qt_sanity_bot@ovi.com>
Reviewed-by: Aaron Kennedy <aaron.kennedy@nokia.com>

4 files changed, 92 insertions, 12 deletions

diff --git a/src/declarative/qml/v8/qdeclarativebuiltinfunctions.cpp b/src/declarative/qml/v8/qdeclarativebuiltinfunctions.cpp
index 8dfc11fed8..96c51da0ef 100644
--- a/src/declarative/qml/v8/qdeclarativebuiltinfunctions.cpp
+++ b/src/declarative/qml/v8/qdeclarativebuiltinfunctions.cpp
@@ -98,17 +98,19 @@ v8::Handle<v8::Value> stringArg(const v8::Arguments &args)
     if (args.Length() != 1)
         V8THROW_ERROR("String.arg(): Invalid arguments");
 
-    if (args[0]->IsUint32())
-        return V8ENGINE()->toString(value.arg(args[0]->Uint32Value()));
-    else if (args[0]->IsInt32())
-        return V8ENGINE()->toString(value.arg(args[0]->Int32Value()));
-    else if (args[0]->IsNumber())
-        return V8ENGINE()->toString(value.arg(args[0]->NumberValue()));
-    else if (args[0]->IsBoolean())
-        return V8ENGINE()->toString(value.arg(args[0]->BooleanValue()));
+    v8::Handle<v8::Value> arg = args[0];
+    if (arg->IsUint32())
+        return V8ENGINE()->toString(value.arg(arg->Uint32Value()));
+    else if (arg->IsInt32())
+        return V8ENGINE()->toString(value.arg(arg->Int32Value()));
+    else if (arg->IsNumber())
+        return V8ENGINE()->toString(value.arg(arg->NumberValue()));
+    else if (arg->IsBoolean())
+        return V8ENGINE()->toString(value.arg(arg->BooleanValue()));
 
-    return V8ENGINE()->toString(value.arg(V8ENGINE()->toString(args[0])));
+    return V8ENGINE()->toString(value.arg(V8ENGINE()->toString(arg)));
 }
+
 /*!
 \qmlmethod bool Qt::isQtObject(object)
 Returns true if \c object is a valid reference to a Qt or QML object, otherwise false.
diff --git a/src/declarative/qml/v8/qv8engine.cpp b/src/declarative/qml/v8/qv8engine.cpp
index 7be74f8711..4bbea93c00 100644
--- a/src/declarative/qml/v8/qv8engine.cpp
+++ b/src/declarative/qml/v8/qv8engine.cpp
@@ -546,9 +546,21 @@ void QV8Engine::initializeGlobal(v8::Handle<v8::Object> global)
     global->Set(v8::String::New("Qt"), qt);
     global->Set(v8::String::New("gc"), V8FUNCTION(QDeclarativeBuiltinFunctions::gc, this));
 
-    v8::Local<v8::Object> string = v8::Local<v8::Object>::Cast(global->Get(v8::String::New("String")));
-    v8::Local<v8::Object> stringPrototype = v8::Local<v8::Object>::Cast(string->Get(v8::String::New("prototype")));
-    stringPrototype->Set(v8::String::New("arg"), V8FUNCTION(stringArg, this));
+    {
+#define STRING_ARG "(function(stringArg) { "\
+                   "    String.prototype.arg = (function() {"\
+                   "        return stringArg.apply(this, arguments);"\
+                   "    })"\
+                   "})"
+
+        v8::Local<v8::Script> registerArg = v8::Script::New(v8::String::New(STRING_ARG), 0, 0, v8::Handle<v8::String>(), v8::Script::NativeMode);
+        v8::Local<v8::Value> result = registerArg->Run();
+        Q_ASSERT(result->IsFunction());
+        v8::Local<v8::Function> registerArgFunc = v8::Local<v8::Function>::Cast(result);
+        v8::Handle<v8::Value> args = V8FUNCTION(stringArg, this);
+        registerArgFunc->Call(v8::Local<v8::Object>::Cast(registerArgFunc), 1, &args);
+#undef STRING_ARG
+    }
 
     qt_add_domexceptions(this);
     m_xmlHttpRequestData = qt_add_qmlxmlhttprequest(this);
diff --git a/tests/auto/declarative/qdeclarativeecmascript/data/stringArg.qml b/tests/auto/declarative/qdeclarativeecmascript/data/stringArg.qml
new file mode 100644
index 0000000000..7019af9da5
--- /dev/null
+++ b/tests/auto/declarative/qdeclarativeecmascript/data/stringArg.qml
@@ -0,0 +1,49 @@
+import QtQuick 2.0
+
+Item {
+    id: root
+    property bool returnValue: false
+
+    property string first
+    property string second
+    property string third
+    property string fourth
+    property string fifth
+    property string sixth
+    property string seventh
+    property string eighth
+    property string ninth
+
+    function success() {
+        var a = "Value is %1";
+        for (var ii = 0; ii < 10; ++ii) {
+            first = a.arg("string");
+            second = a.arg(1);
+            third = a.arg(true);
+            fourth = a.arg(3.345);
+            fifth = a.arg(undefined);
+            sixth = a.arg(null);
+            seventh = a.arg({"test":5});
+            eighth = a.arg({"test":5, "again":6});
+        }
+
+        if (first != "Value is string") returnValue = false;
+        if (second != "Value is 1") returnValue = false;
+        if (third != "Value is true") returnValue = false;
+        if (fourth != "Value is 3.345") returnValue = false;
+        if (fifth != "Value is undefined") returnValue = false;
+        if (sixth != "Value is null") returnValue = false;
+        if (seventh != "Value is [Object object]") returnValue = false;
+        if (eighth != "Value is [Object object]") returnValue = false;
+        returnValue = true;
+    }
+
+    function failure() {
+        returnValue = true;
+        var a = "Value is %1";
+        for (var ii = 0; ii < 10; ++ii) {
+            ninth = a.arg(1,2,3,4);
+        }
+        returnValue = false;
+    }
+}
diff --git a/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp b/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp
index 941765d50c..0d88449692 100644
--- a/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp
+++ b/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp
@@ -159,6 +159,7 @@ private slots:
     void objectPassThroughSignals();
     void booleanConversion();
     void handleReferenceManagement();
+    void stringArg();
 
     void bug1();
     void bug2();
@@ -3554,6 +3555,22 @@ void tst_qdeclarativeecmascript::handleReferenceManagement()
     }
 }
 
+void tst_qdeclarativeecmascript::stringArg()
+{
+    QDeclarativeComponent component(&engine, TEST_FILE("stringArg.qml"));
+    QObject *object = component.create();
+    QVERIFY(object != 0);
+    QMetaObject::invokeMethod(object, "success");
+    QVERIFY(object->property("returnValue").toBool());
+
+    QString w1 = TEST_FILE("stringArg.qml").toString() + QLatin1String(":45: Error: String.arg(): Invalid arguments");
+    QTest::ignoreMessage(QtWarningMsg, w1.toAscii().constData());
+    QMetaObject::invokeMethod(object, "failure");
+    QVERIFY(object->property("returnValue").toBool());
+
+    delete object;
+}
+
 // Test that assigning a null object works 
 // Regressed with: df1788b4dbbb2826ae63f26bdf166342595343f4
 void tst_qdeclarativeecmascript::nullObjectBinding()