Re: [libmicrohttpd] upgrading connection

[José Bollo]

Le mercredi 20 avril 2016 à 18:03 +0300, Evgeny Grin a écrit :
> 
> On 20.04.2016 15:28, José Bollo wrote:
> > I implement web-socket upgrading and it worked very well with firefox.
> > The bad happens when I use Chromium. The following little patch of the
> > function 'keepalive_possible' of 'connection.c' solves the issue:
> > 
> > --- a/connection.c
> > +++ b/connection.c
> > @@ -733,8 +733,7 @@
> >    {
> >      if (NULL == end)
> >        return MHD_YES;
> > -    if ( (MHD_str_equal_caseless_ (end, "close")) ||
> > -         (MHD_str_equal_caseless_ (end, "upgrade")) )
> > +    if ( (MHD_str_equal_caseless_ (end, "close")) )
> >        return MHD_NO;
> >     return MHD_YES;
> >    }
> > 
> > Explanation: I use MHD_suspend_connection and MHD_resume_connection to
> > handle the connection that MHD created (MHD listens and creates
> > connections). The connection is suspended in the callback
> > MHD_OPTION_NOTIFY_COMPLETED, after sending the HTTP reply 101 'switching
> > protocols'. I enjoy that MHD can send the reply.
> > 
> > But chromium emits the header "connection: upgrade\r\n" when firefox
> > emits the header "connection: keep-alive, upgrade\r\n". This has the
> > effect that the connection is closed unexpectedly for chromium but not
> > for firefox.
> > 
> > So here is my mind: (1) there is a bug in 'keepalive_possible' that
> > doesn't handle multiple keys for 'connection' (but maybe HTTP states
> > that only one key is possible, I haven't checked) (2) my patch is the
> > smallest one that solves my problem (3) what is the official path for
> > upgrading a connection? If none, do you agree that the use of
> > suspend/resume is a good idea? If yes, the patch should be upstreamed to
> > avoid the closing of the connection when "connection: upgrade\r\n"
> > appears.
> 
> Multiple "connection: " and multiple values for "connection: " header is
> not correctly supported now. It need improvement so code will handle all
> possible situation according to RFC. However in practice, browser sends
> only single value. "upgrade" is the exception, but "upgrade" is not
> supported currently. We are collecting ideas how to support "upgrade" in
> the best way, you can see some preliminary drafts in disabled part of
> microhttpd.h (look for "MHD_UpgradeAction").

Seen. Well... I don't fully understand the design at first look. In
particular, the comment of MHD_UpgradeHandler write things about
'data_in/out' but I don't understand what it refers to.

> Not sure that MHD_suspend_connection()/MHD_resume_connection() are
> designed to be used in such way. MHD keeps internal stage of connection
> processing and it's not updated after MHD_resume_connection().
> How did you obtain socket FD and when you are calling
> MHD_resume_connection()?

I use on suspend (not resume)
MHD_get_connection_info(connection,MHD_CONNECTION_INFO_CONNECTION_FD)->connect_fd

I configure MHD to use EPOLL

The effect of suspending is that the fd is removed from the file set
using  EPOLL_CTL_DEL. This works very well (with the patch) because the
buffers are clean. I guess that is will always be the case but...

Anyway the connection must be kept alive. No?

Best regards
José