[webkit-reviews] review denied: [Bug 61862] EventSource should support CORS : [Attachment 111298] patch 4

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has denied Per-Erik Brodin
<per-erik.brodin at ericsson.com>'s request for review:
Bug 61862: EventSource should support CORS
https://bugs.webkit.org/show_bug.cgi?id=61862

Attachment 111298: patch 4
https://bugs.webkit.org/attachment.cgi?id=111298&action=review

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=111298&action=review


Looks like the spec changed substantially since the patch was posted.

> Source/WebCore/page/EventSource.cpp:119
>      options.allowCredentials = AllowStoredCredentials;

The spec says that credentials should not be sent cross-origin unless
constructor is called with a second argument telling it to allow credentials:

    4. Let CORS mode be Anonymous.

    5. If the second argument is present, and the withCredentials dictionary
member has the value true, then set CORS mode to Use Credentials and initialize
the new EventSource object's withCredentials attribute to true.

There should also be a readonly withCredentials attribute on EventSource
object.

The spec says to always use credentials when reconnecting, but that looks like
a mistake.

> Source/WebCore/page/EventSource.cpp:256
> +    bool failedResourceSharingCheck = error.domain() ==
errorDomainWebKitInternal;

That's terribly fragile. We need a better way to communicate this to
EventSource code.