[webkit-reviews] review denied: [Bug 128657] Don't crash when SerializedScriptValue deserialization fails : [Attachment 223947] proposed fix

[bugzilla-daemon at webkit.org]

Oliver Hunt <oliver at apple.com> has denied Alexey Proskuryakov <ap at webkit.org>'s
request for review:
Bug 128657: Don't crash when SerializedScriptValue deserialization fails
https://bugs.webkit.org/show_bug.cgi?id=128657

Attachment 223947: proposed fix
https://bugs.webkit.org/attachment.cgi?id=223947&action=review

------- Additional Comments from Oliver Hunt <oliver at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=223947&action=review


r- due to the 0->jsNull change

> Source/WebCore/bindings/js/JSMessageEventCustom.cpp:67
> +	       // FIXME: Why does this suppress exceptions?

Check with stephanie - i can't remember whether the change responsible for this
code was to make some api thingy work, or to refactor it to this layout was
hers or mine

> Source/WebCore/bindings/js/SerializedScriptValue.cpp:2654
> -	   return 0;
> +	   return toRef(exec, jsNull());

Our api for deserialising is to return NULL on failure, not a jsvalue --
nullptr is distinguishable from jsNull.  This change makes it impossible to
distinguish desrialisation failure, and deserialing jsNull

> LayoutTests/crypto/subtle/postMessage-worker-expected.txt:6
> -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

wut?