[webkit-reviews] review denied: [Bug 31617] WebSocket handshake doesn't check query component of URL : [Attachment 43421] WebSocket handshake check query component of URL.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Nov 20 14:17:01 PST 2009


Alexey Proskuryakov <ap at webkit.org> has denied Fumitoshi Ukai
<ukai at chromium.org>'s request for review:
Bug 31617: WebSocket handshake doesn't check query component of URL
https://bugs.webkit.org/show_bug.cgi?id=31617

Attachment 43421: WebSocket handshake check query component of URL.
https://bugs.webkit.org/attachment.cgi?id=43421&action=review

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
+    KURL location_url = m_url.copy();

As explained in a comment in KURL.h, copy() shouldn't be used here.

+    KURL location_url = m_url.copy();

In WebKit style, this would be named locationURL. I don't understand why the
function and variable use the word "location" - where did it come from?

+    location_url.setHost(location_url.host().lower());

Hmm, I think that KURL should do this internally. Probably worth a FIXME for
now.

We need more tests:
- what happens if the server echoes the string with query (my understanding is
that handshake should succeed):
- non-empty query;
- what happens if someone (client or server) sends an URL with fragment;
- what happens if the URL includes credentials (e.g.
ws://user:pass@server/path).

This patch changes behavior of clientLocation() in the latter case.


More information about the webkit-reviews mailing list