Don't crash when SerializedScriptValue deserialization fails
authorap@apple.com <ap@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 13 Feb 2014 04:05:10 +0000 (04:05 +0000)
committerap@apple.com <ap@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 13 Feb 2014 04:05:10 +0000 (04:05 +0000)
https://bugs.webkit.org/show_bug.cgi?id=128657

Reviewed by Oliver Hunt.

Source/WebCore:

Test: crypto/subtle/postMessage-worker.html

* bindings/js/JSMessageEventCustom.cpp: (WebCore::JSMessageEvent::data): Added a FIXME.

* bindings/js/SerializedScriptValue.cpp:
(WebCore::CloneBase::fail): Don't assert on failure.
(WebCore::SerializedScriptValue::deserialize): Never return a null JSValue, these
are not allowed.

LayoutTests:

* crypto/subtle/postMessage-worker-expected.txt:
* crypto/subtle/resources/postMessage-worker.js:
* platform/mac/TestExpectations:
Unskip the test, and land (unimportant) failure results.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@164008 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/crypto/subtle/postMessage-worker-expected.txt
LayoutTests/crypto/subtle/resources/postMessage-worker.js
LayoutTests/platform/mac/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/bindings/js/JSMessageEventCustom.cpp
Source/WebCore/bindings/js/SerializedScriptValue.cpp

index fa90010..27235f2 100644 (file)
@@ -1,3 +1,15 @@
+2014-02-12  Alexey Proskuryakov  <ap@apple.com>
+
+        Don't crash when SerializedScriptValue deserialization fails
+        https://bugs.webkit.org/show_bug.cgi?id=128657
+
+        Reviewed by Oliver Hunt.
+
+        * crypto/subtle/postMessage-worker-expected.txt:
+        * crypto/subtle/resources/postMessage-worker.js:
+        * platform/mac/TestExpectations:
+        Unskip the test, and land (unimportant) failure results.
+
 2014-02-12  Brady Eidson  <beidson@apple.com>
 
         IDB: TestExpectations batch - "aborted-versionchange-closes.html to createObjectStore-null-name.html"
index a218fe5..60cd302 100644 (file)
@@ -3,12 +3,7 @@ Test sending crypto keys via postMessage to a worker.
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
 
-PASS All checks passed in worker
-PASS key.type is 'secret'
-PASS key.extractable is true
-PASS key.algorithm.name is 'HMAC'
-PASS key.algorithm.length is 16
-PASS key.usages is ["decrypt", "encrypt", "sign", "verify"]
+FAIL Check failed in worker: key is null
 PASS successfullyParsed is true
 
 TEST COMPLETE
index 7249061..ea0bec2 100644 (file)
@@ -1,6 +1,8 @@
 onmessage = function(evt)
 {
     var key = evt.data;
+    if (!key)
+        postMessage({ result:false, message:'key is ' + key });
     if (key.type != 'secret')
         postMessage({ result:false, message:'key.type should be "secret"' });
     else if (!key.extractable)
index 9c0ff53..931d50e 100644 (file)
@@ -1245,9 +1245,6 @@ media/track/video
 # SubtleCrypto is disabled on Mountain Lion
 webkit.org/b/124261 [ MountainLion ] crypto/subtle [ Skip ]
 
-# Deserialization of a CryptoKey in a worker fails with a crash.
-crypto/subtle/postMessage-worker.html [ Skip ]
-
 webkit.org/b/124311 compositing/regions/transform-transparent-positioned-video-inside-region.html [ Pass ImageOnlyFailure ]
 
 webkit.org/b/124321 [ Mavericks ] animations/resume-after-page-cache.html [ Pass Failure ]
index fba409d..ec742a5 100644 (file)
@@ -1,3 +1,19 @@
+2014-02-11  Alexey Proskuryakov  <ap@apple.com>
+
+        Don't crash when SerializedScriptValue deserialization fails
+        https://bugs.webkit.org/show_bug.cgi?id=128657
+
+        Reviewed by Oliver Hunt.
+
+        Test: crypto/subtle/postMessage-worker.html
+
+        * bindings/js/JSMessageEventCustom.cpp: (WebCore::JSMessageEvent::data): Added a FIXME.
+
+        * bindings/js/SerializedScriptValue.cpp:
+        (WebCore::CloneBase::fail): Don't assert on failure.
+        (WebCore::SerializedScriptValue::deserialize): Never return a null JSValue, these
+        are not allowed.
+
 2014-02-12  Bem Jones-Bey  <bjonesbe@adobe.com>
 
         [CSS Shapes] Rename shapeSize and others to make ShapeInfo and friends easier to understand
index 5615a08..ea9c45f 100644 (file)
@@ -64,9 +64,9 @@ JSValue JSMessageEvent::data(ExecState* exec) const
     case MessageEvent::DataTypeSerializedScriptValue:
         if (RefPtr<SerializedScriptValue> serializedValue = event.dataAsSerializedScriptValue()) {
             MessagePortArray ports = impl().ports();
+            // FIXME: Why does this suppress exceptions?
             result = serializedValue->deserialize(exec, globalObject(), &ports, NonThrowing);
-        }
-        else
+        } else
             result = jsNull();
         break;
 
index 0c5b0de..d1379f0 100644 (file)
@@ -381,10 +381,8 @@ protected:
         m_exec->vm().throwException(m_exec, createStackOverflowError(m_exec));
     }
 
-    NO_RETURN_DUE_TO_ASSERT
     void fail()
     {
-        ASSERT_NOT_REACHED();
         m_failed = true;
     }
 
@@ -2641,7 +2639,7 @@ JSValue SerializedScriptValue::deserialize(ExecState* exec, JSGlobalObject* glob
                                                                   m_arrayBufferContentsArray.get(), m_data);
     if (throwExceptions == Throwing)
         maybeThrowExceptionIfSerializationFailed(exec, result.second);
-    return result.first;
+    return result.first ? result.first : jsNull();
 }
 
 JSValueRef SerializedScriptValue::deserialize(JSContextRef destinationContext, JSValueRef* exception)
@@ -2653,7 +2651,7 @@ JSValueRef SerializedScriptValue::deserialize(JSContextRef destinationContext, J
         if (exception)
             *exception = toRef(exec, exec->exception());
         exec->clearException();
-        return 0;
+        return nullptr;
     }
     ASSERT(value);
     return toRef(exec, value);