[asterisk-dev] [Code Review] 3248: Fix for WebRTC over WSS not working

[Moises Silva]

> On Feb. 27, 2014, 10:02 p.m., Matt Jordan wrote:
> > /branches/11/res/res_http_websocket.c, lines 324-350
> > <https://reviewboard.asterisk.org/r/3248/diff/1/?file=54350#file54350line324>
> >
> >     So, I always get nervous every time I see a 'sanity' check polling loop :-)
> >     
> >     I know Thava took a similar approach on the patch on ASTERISK-23099 without the sanity check:
> >     
> >     +		   if (ast_wait_for_input(session->fd, 100) > 0) {
> >     +			while ((readlen = fread(&(buf[readnow]), 1, MAXIMUM_FRAME_SIZE, session->f)) < 1) {
> >     +			  int  ferr = ferror(session->f);
> >     +			  int feoffile = feof(session->f);
> >     +			  ast_debug(3,"ast_websocket_read() fread error  ferr=%d, feoffile=%d, returnval=%"PRIu32"\n", ferr,feoffile,readlen);
> >     +			}  
> >     +		  }  
> >     
> >     I think your approach of checking for EAGAIN is better - was that to work through the case that you mention in the comments, where the fd says it is ready to be read, but in reality no data is available?

Yes the EAGAIN check is exactly because of that AFAIR


- Moises


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3248/#review10990
-----------------------------------------------------------


On Feb. 22, 2014, 8:03 p.m., Moises Silva wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3248/
> -----------------------------------------------------------
> 
> (Updated Feb. 22, 2014, 8:03 p.m.)
> 
> 
> Review request for Asterisk Developers and rnewton.
> 
> 
> Bugs: ASTERISK-21930 and ASTERISK-23099
>     https://issues.asterisk.org/jira/browse/ASTERISK-21930
>     https://issues.asterisk.org/jira/browse/ASTERISK-23099
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> 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
> 
> 
> Diffs
> -----
> 
>   /branches/11/res/res_http_websocket.c 408854 
> 
> Diff: https://reviewboard.asterisk.org/r/3248/diff/
> 
> 
> Testing
> -------
> 
> 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)
> 
> 
> Thanks,
> 
> Moises Silva
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140303/afdbcbe7/attachment-0001.html>