[PATCH] RFC: ngx_http_upstream_process_upgraded: Allocate buffers also for data from upstream

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] RFC: ngx_http_upstream_process_upgraded: Allocate buffers also for data from upstream

Thomas Glanzmann
While using the ugprade funcationality of nginx to tunnel propiertary
HTTP commands I noticed that data were only passing through from
upstream to downstream but not the other way around. The reason for that
was that no receive buffers for downstream were allocated. Normally the
receiver buffers for upstream are allocated in
ngx_http_upstream_process_header. In my case they were not because I
upgrade the connection before exchanging any data. Maybe you consider
this for upstream.
---
 src/http/ngx_http_upstream.c |   22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/src/http/ngx_http_upstream.c b/src/http/ngx_http_upstream.c
index cf9ca0d..5ff1f2b 100644
--- a/src/http/ngx_http_upstream.c
+++ b/src/http/ngx_http_upstream.c
@@ -2644,20 +2644,20 @@ ngx_http_upstream_process_upgraded(ngx_http_request_t *r,
             b->end = b->last;
             do_write = 1;
         }
+    }
 
+    if (b->start == NULL) {
+        b->start = ngx_palloc(r->pool, u->conf->buffer_size);
         if (b->start == NULL) {
-            b->start = ngx_palloc(r->pool, u->conf->buffer_size);
-            if (b->start == NULL) {
-                ngx_http_upstream_finalize_request(r, u, NGX_ERROR);
-                return;
-            }
-
-            b->pos = b->start;
-            b->last = b->start;
-            b->end = b->start + u->conf->buffer_size;
-            b->temporary = 1;
-            b->tag = u->output.tag;
+            ngx_http_upstream_finalize_request(r, u, NGX_ERROR);
+            return;
         }
+
+        b->pos = b->start;
+        b->last = b->start;
+        b->end = b->start + u->conf->buffer_size;
+        b->temporary = 1;
+        b->tag = u->output.tag;
     }
 
     for ( ;; ) {
--
1.7.10.4

_______________________________________________
nginx mailing list
[hidden email]
http://mailman.nginx.org/mailman/listinfo/nginx
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] RFC: ngx_http_upstream_process_upgraded: Allocate buffers also for data from upstream

Maxim Dounin
Hello!

On Sat, Mar 15, 2014 at 11:11:51PM +0100, Thomas Glanzmann wrote:

> While using the ugprade funcationality of nginx to tunnel propiertary
> HTTP commands I noticed that data were only passing through from
> upstream to downstream but not the other way around. The reason for that
> was that no receive buffers for downstream were allocated. Normally the
> receiver buffers for upstream are allocated in
> ngx_http_upstream_process_header. In my case they were not because I
> upgrade the connection before exchanging any data. Maybe you consider
> this for upstream.

The u->buffer is allocated by ngx_http_upstream_process_header(),
and ngx_http_upstream_upgrade() cannot be called bypassing
ngx_http_upstream_process_header().

That is, the change you suggest isn't needed in vanilla nginx
(even with custom modules).

--
Maxim Dounin
http://nginx.org/

_______________________________________________
nginx mailing list
[hidden email]
http://mailman.nginx.org/mailman/listinfo/nginx
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] RFC: ngx_http_upstream_process_upgraded: Allocate buffers also for data from upstream

Thomas Glanzmann
Hello Maxim,

> The u->buffer is allocated by ngx_http_upstream_process_header(),
> and ngx_http_upstream_upgrade() cannot be called bypassing
> ngx_http_upstream_process_header().

> That is, the change you suggest isn't needed in vanilla nginx
> (even with custom modules).

I agree. The reason for my modification was that I called
ngx_http_upstream_upgrade in ngx_http_upstream_connect because I wanted
to avoid that the original request was sent to upstream.

Cheers,
        Thomas

_______________________________________________
nginx mailing list
[hidden email]
http://mailman.nginx.org/mailman/listinfo/nginx