[webkit-reviews] review granted: [Bug 34289] WebSocket ignores HttpOnly cookies, but should use in Handshake. : [Attachment 48608] Patch

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has granted Fumitoshi Ukai
<ukai at chromium.org>'s request for review:
Bug 34289: WebSocket ignores HttpOnly cookies, but should use in Handshake.
https://bugs.webkit.org/show_bug.cgi?id=34289

Attachment 48608: Patch
https://bugs.webkit.org/attachment.cgi?id=48608&action=review

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
     String cookies(const Document*, const KURL&);
+    String cookieRequestHeaderFieldValue(const Document*, const KURL&);

Looking at this, I think that there should be a comment explaining that
cookies() omits HttpOnly cookies.

+	 "-x", "/websocket/tests/cookies",

Ideally, we should be able to set his to "/websocket/tests". That way, no one
will get surprised by trying to add a .pl test to another subdirectory. Of
course, pywebsocket would need to learn how to distinguish .html and .pl files.


>I think this is because these belong to different port.

Indeed, I keep forgetting about this!

> Do you think we should remove this warning?

It seems confusing, as we're passing a specific directory for CGIs.

This warning is not necessary for WebKit, since it's fairly clear that a
machine running Apache on LayoutTests/http/tests on an external interface is
vulnerable to attacks (by default, it only binds to 127.0.0.1 loopback).
Websocket tests do not seem to add much to this.

r=me