Websockets support for uwsgi protocol

Hi, the (tiny) attached patch enable support for new websockets handling
when the uwsgi protocol is used instead of HTTP.

I have tested it with various websocket libraries and with the api
available in uWSGI 1.9.

From 1.9 sources (with nginx pointing to uwsgi port 3031):

./uwsgi -s :3031 -w tests.websockets_echo --gevent 10

no additional configuration is needed for nginx

Hello!

(Cc’d [email protected] as this is better list for this discussion.)

On Wed, Feb 20, 2013 at 02:30:38PM +0100, Roberto De Ioris wrote:

no additional configuration is needed for nginx

Should we also do the same for SCGI?

I personally think it is more or less the same for all CGI-based
protocols, and the only problematic one is FastCGI, which probably
still needs wrapping within FCGI_STDIN/FCGI_STDOUT records instead
of real connection upgrade.


Maxim D.
http://nginx.com/support.html

available in uWSGI 1.9.
I personally think it is more or less the same for all CGI-based
protocols, and the only problematic one is FastCGI, which probably
still needs wrapping within FCGI_STDIN/FCGI_STDOUT records instead
of real connection upgrade.

AFAIK scgi body management works in the same way as the uwsgi one, so
the
patch should be usable there too


Roberto De Ioris
http://unbit.it

Should we also do the same for SCGI?

I personally think it is more or less the same for all CGI-based
protocols, and the only problematic one is FastCGI, which probably
still needs wrapping within FCGI_STDIN/FCGI_STDOUT records instead
of real connection upgrade.

AFAIK scgi body management works in the same way as the uwsgi one, so the
patch should be usable there too

Ok, i can confirm the same patch applied to the SCGI module in the same
position works (at least with uWSGI 1.9 in SCGI mode)

./uwsgi --scgi-nph-socket :3031 -w tests.websockets_echo --gevent 10

remember to add

scgi_param PATH_INFO $document_uri;

to the nginx config to make it work


Roberto De Ioris
http://unbit.it

Hello!

On Wed, Feb 20, 2013 at 03:07:53PM +0100, Roberto De Ioris wrote:

when the uwsgi protocol is used instead of HTTP.

patch should be usable there too

to the nginx config to make it work

Ok, so the next question is: any specific reason to exclude normal
CGI responses with “Status” as in your patch?

I in fact don’t like the idea of supporting http-like answers with
status like from CGI-like protocols, correct way is to use
“Status” header. Not sure why Manlio introduced it at all,
probably due to some compatibility concerns (and due to the fact
that SCGI specification explicitly refuses to specify response
format).

Something like this should be better, IMHO:

diff --git a/src/http/modules/ngx_http_scgi_module.c
b/src/http/modules/ngx_http_scgi_module.c
— a/src/http/modules/ngx_http_scgi_module.c
+++ b/src/http/modules/ngx_http_scgi_module.c
@@ -984,7 +984,7 @@ ngx_http_scgi_process_header(ngx_http_re
u = r->upstream;

         if (u->headers_in.status_n) {
  •            return NGX_OK;
    
  •            goto done;
           }
    
           if (u->headers_in.status) {
    

@@ -1015,6 +1015,14 @@ ngx_http_scgi_process_header(ngx_http_re
u->state->status = u->headers_in.status_n;
}

  •    done:
    
  •        if (u->headers_in.status_n == NGX_HTTP_SWITCHING_PROTOCOLS
    
  •            && r->headers_in.upgrade)
    
  •        {
    
  •            u->upgrade = 1;
    
  •        }
    
  •        return NGX_OK;
       }
    

diff --git a/src/http/modules/ngx_http_uwsgi_module.c
b/src/http/modules/ngx_http_uwsgi_module.c
— a/src/http/modules/ngx_http_uwsgi_module.c
+++ b/src/http/modules/ngx_http_uwsgi_module.c
@@ -1018,7 +1018,7 @@ ngx_http_uwsgi_process_header(ngx_http_r
u = r->upstream;

         if (u->headers_in.status_n) {
  •            return NGX_OK;
    
  •            goto done;
           }
    
           if (u->headers_in.status) {
    

@@ -1049,6 +1049,14 @@ ngx_http_uwsgi_process_header(ngx_http_r
u->state->status = u->headers_in.status_n;
}

  •    done:
    
  •        if (u->headers_in.status_n == NGX_HTTP_SWITCHING_PROTOCOLS
    
  •            && r->headers_in.upgrade)
    
  •        {
    
  •            u->upgrade = 1;
    
  •        }
    
  •        return NGX_OK;
       }
    


Maxim D.
http://nginx.com/support.html

Hello!

On Wed, Feb 20, 2013 at 04:20:29PM +0100, Roberto De Ioris wrote:

format).

Honestly i do not remember why Manlio added support for nph (but i have
added it to uWSGI SCGI parser too, so in my subconsciuous there should be
a good reason :P)

regarding your updated patch is better for sure

Committed, thnx.


Maxim D.
http://nginx.com/support.html

Ok, so the next question is: any specific reason to exclude normal
CGI responses with “Status” as in your patch?

I in fact don’t like the idea of supporting http-like answers with
status like from CGI-like protocols, correct way is to use
“Status” header. Not sure why Manlio introduced it at all,
probably due to some compatibility concerns (and due to the fact
that SCGI specification explicitly refuses to specify response
format).

Honestly i do not remember why Manlio added support for nph (but i have
added it to uWSGI SCGI parser too, so in my subconsciuous there should
be
a good reason :P)

regarding your updated patch is better for sure


Roberto De Ioris
http://unbit.it

This forum is not affiliated to the Ruby language, Ruby on Rails framework, nor any Ruby applications discussed here.

| Privacy Policy | Terms of Service | Remote Ruby Jobs