<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://reviewboard.asterisk.org/r/3248/">https://reviewboard.asterisk.org/r/3248/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On March 4th, 2014, 11:03 a.m. UTC, <b>wdoekes</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://reviewboard.asterisk.org/r/3248/diff/2/?file=55032#file55032line291" style="color: black; font-weight: bold; text-decoration: underline;">/branches/11/res/res_http_websocket.c</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">int AST_OPTIONAL_API_NAME(ast_websocket_set_nonblock)(struct ast_websocket *session)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">291</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cm"> * We have to keep in mind during this function that the fact that session->fd seems ready (via poll) does not necessarily mean we have application data ready, because in the case</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Please trim the comment lengths a bit.
72, 80 or 100 characters per line is more readable than the ~180 you have here.</pre>
</blockquote>
<p>On March 4th, 2014, 11:37 p.m. UTC, <b>Moises Silva</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Fixed ... but, doesn't everybody has huge screens now days? ... I agree could be handy if I am reading code on my phone though :)</pre>
</blockquote>
<p>On March 5th, 2014, 8:16 a.m. UTC, <b>wdoekes</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Thanks :)
Some people use those screens for multiple windows at once.
But even if they don't, humans tend to have less trouble reading
column layout than long lines. See also:
http://c2.com/cgi/wiki?ColumnModeSometimesBetterThanWidespan
Another example: the github commit window fits just under 120 characters,
no matter what size of screen you have.</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Cool thanks for the follow up with references, good arguments :)</pre>
<br />
<p>- Moises</p>
<br />
<p>On March 3rd, 2014, 1:19 a.m. UTC, Moises Silva wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers and rnewton.</div>
<div>By Moises Silva.</div>
<p style="color: grey;"><i>Updated March 3, 2014, 1:19 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://issues.asterisk.org/jira/browse/ASTERISK-21930">ASTERISK-21930</a>,
<a href="https://issues.asterisk.org/jira/browse/ASTERISK-23099">ASTERISK-23099</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
Asterisk
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Several fixes for the WebSockets implementation in res/res_http_websocket.c
* Flush the websocket session FILE* as fwrite() may not actually guarantee sending
the data to the network. If we do not flush, it seems that buffering on the SSL
socket for outbound messages causes issues
* Refactored ast_websocket_read to take into account that SSL file descriptors
may be ready to read via fread() but poll() will not actually say so because
the data was already read from the network buffers and is now in the libc buffers
This should fix an issue that I have experienced and other users may have reported [1][2][3], where
secure websockets wouldn't work, messages seem to not make it into Asterisk
[1] http://lists.digium.com/pipermail/asterisk-users/2013-August/280175.html
[2] https://issues.asterisk.org/jira/browse/ASTERISK-21930
[3] https://issues.asterisk.org/jira/browse/ASTERISK-23099</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">See ASTERISK-21930 for details on other users testing these changes. I did both WS and WSS calls, confirmed audio works with chrome. This patch is for Asterisk 11 as the issue is reported on Asterisk 11, but I tested a few months ago and same issue existed on 12 and trunk. I created my own team branches for those too (/team/moy/webrtc-11, /team/moy/webrtc-12, /team/moy/webrtc-trunk)
Confirmed working by Sean Bright on Jan 20, 2014 on Asterisk 11 (see ASTERISK-21930 comment)</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/branches/11/res/res_http_websocket.c <span style="color: grey">(409360)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/3248/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>