[webkit-reviews] review denied: [Bug 28037] SocketStreamHandle interface for WebSocket API : [Attachment 34545] Add SocketStreamHandle interface

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Aug 11 09:08:10 PDT 2009


Alexey Proskuryakov <ap at webkit.org> has denied Fumitoshi Ukai
<ukai at chromium.org>'s request for review:
Bug 28037: SocketStreamHandle interface for WebSocket API
https://bugs.webkit.org/show_bug.cgi?id=28037

Attachment 34545: Add SocketStreamHandle interface
https://bugs.webkit.org/attachment.cgi?id=34545&action=review

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
> +    if (num_written < size)
> +	   m_buffer.append(buf + num_written, size - num_written);

I think that we need a (large) upper limit on buffer size - it's too easy to
send data faster than network permits, exhausting system memory and making the
system unresponsive. Certainly, there are many other ways to exhaust memory via
JavaScript, so it would be just a safety net, not a real security measure.

> +    remaining_data.append(m_buffer.data() + num_written,
> +			      m_buffer.size() - num_written);

As David pointed out, this is an indentation mistake - but there's no need to
split the line anyway, it's quite short.

Looks good to me in general. I'm not sure if I'm not stepping on Dave's toes
here, but r- to address the comments.


More information about the webkit-reviews mailing list