Hello!

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

>
> >
> >> Hello!
> >>
> >> (Cc'd nginx-devel@ as this is better list for this discussion.)
> >>
> >> On Wed, Feb 20, 2013 at 02:30:38PM +0100, Roberto De Ioris wrote:
> >>
> >>> 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
> >>
> >> 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

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 Dounin
http://nginx.com/support.html

_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel