changeset 10:2b95417a1715

Auth request: fix body handling again. Setting r->discard_body is wrong way to go as it causes lingering timer to be armed on subrequest finalization. Create fake body instead. This also allows to protect real body file from being closed in case it was already read. Though it doesn't matter now as we set r->header_only and relevant code in ngx_http_upstream_send_response() isn't reached.
author Maxim Dounin <mdounin@mdounin.ru>
date Wed, 24 Mar 2010 07:23:22 +0300
parents 4385a10a836f
children ee8ff54f9b66
files ngx_http_auth_request_module.c t/auth-request.t
diffstat 2 files changed, 55 insertions(+), 2 deletions(-) [+]
line wrap: on
line diff
--- a/ngx_http_auth_request_module.c
+++ b/ngx_http_auth_request_module.c
@@ -190,7 +190,16 @@ ngx_http_auth_request_handler(ngx_http_r
         return NGX_ERROR;
     }
 
-    sr->discard_body = 1;
+    /*
+     * allocate fake request body to avoid attempts to read it and to make
+     * sure real body file (if already read) won't be closed by upstream
+     */
+
+    sr->request_body = ngx_pcalloc(r->pool, sizeof(ngx_http_request_body_t));
+    if (sr->request_body == NULL) {
+        return NGX_ERROR;
+    }
+
     sr->header_only = 1;
 
     ctx->subrequest = sr;
--- a/t/auth-request.t
+++ b/t/auth-request.t
@@ -20,7 +20,7 @@ select STDERR; $| = 1;
 select STDOUT; $| = 1;
 
 my $t = Test::Nginx->new()->has(qw/http rewrite proxy fastcgi auth_basic/)
-	->plan(17);
+	->plan(18);
 
 $t->write_file_expand('nginx.conf', <<'EOF');
 
@@ -95,6 +95,22 @@ http {
             auth_basic_user_file %%TESTDIR%%/htpasswd;
         }
 
+        location = /proxy-double {
+            proxy_pass http://127.0.0.1:8080/auth-error;
+            proxy_intercept_errors on;
+            error_page 404 = /proxy-double-fallback;
+            client_body_buffer_size 4k;
+        }
+        location = /proxy-double-fallback {
+            auth_request /auth-proxy-double;
+            proxy_pass http://127.0.0.1:8080/auth-open;
+        }
+        location = /auth-proxy-double {
+            proxy_pass http://127.0.0.1:8080/auth-open;
+            proxy_pass_request_body off;
+            proxy_set_header Content-Length "";
+        }
+
         location /fastcgi {
             auth_request /auth-fastcgi;
         }
@@ -136,6 +152,22 @@ unlike(http_get_auth('/proxy'), qr/INVIS
 
 like(http_post('/proxy'), qr/ 401 /, 'proxy auth post');
 
+# Consider the following scenario:
+#
+# 1. proxy_pass reads request body, then goes to fallback via error_page
+# 2. auth request uses proxy_pass, and upstream module closes request body file
+#    in ngx_http_upstream_send_response()
+# 3. oops: fallback has no body
+#
+# To prevent this we always allocate fake request body for auth request.
+#
+# Note that this doesn't happen when using header_only as relevant code
+# in ngx_http_upstream_send_response() isn't reached.  It may be reached
+# with proxy_cache or proxy_store, but they will shutdown client connection
+# in case of header_only and hence do not work for us at all.
+
+like(http_post_big('/proxy-double'), qr/ 204 /, 'proxy auth with body read');
+
 SKIP: {
 	eval { require FCGI; };
 	skip 'FCGI not installed', 2 if $@;
@@ -171,6 +203,18 @@ sub http_post {
 	return http($p, %extra);
 }
 
+sub http_post_big {
+	my ($url, %extra) = @_;
+
+	my $p = "POST $url HTTP/1.0" . CRLF .
+		"Host: localhost" . CRLF .
+		"Content-Length: 10240" . CRLF .
+		CRLF .
+		("1234567890" x 1024); 
+
+	return http($p, %extra);
+}
+
 ###############################################################################
 
 sub fastcgi_daemon {