Perl: propagate errors.

When an error happens, the ctx->error bit is now set, and croak()
is called to terminate further processing.  The ctx->error bit is
checked in ngx_http_perl_call_handler() to cancel further processing,
and is also checked in various output functions - to make sure these won't
be called if croak() was handled by an eval{} in perl code.

In particular, this ensures that output chain won't be called after
errors, as filters might not expect this to happen.  This fixes some
segmentation faults under low memory conditions.  Also this stops
request processing after filter finalization or request body reading
errors.

For cases where an HTTP error status can be additionally returned (for
example, 416 (Requested Range Not Satisfiable) from the range filter),
the ctx->status field is also added.

diff --git a/src/http/modules/perl/nginx.xs b/src/http/modules/perl/nginx.xs
index 2e9808f2bd7463ac899f80fcfb7bc19aa15e2c2e..8e17f6d5714adf6f3c742177ad7621c249533e65 100644 (file)
--- a/src/http/modules/perl/nginx.xs
+++ b/src/http/modules/perl/nginx.xs
@@ -125,9 +125,14 @@ send_http_header(r, ...)
     ngx_http_request_t   *r;
     ngx_http_perl_ctx_t  *ctx;
     SV                   *sv;
+    ngx_int_t             rc;
 
     ngx_http_perl_set_request(r, ctx);
 
+    if (ctx->error) {
+        croak("send_http_header(): called after error");
+    }
+
     if (r->headers_out.status == 0) {
         r->headers_out.status = NGX_HTTP_OK;
     }
@@ -151,7 +156,13 @@ send_http_header(r, ...)
 
     r->disable_not_modified = 1;
 
-    (void) ngx_http_send_header(r);
+    rc = ngx_http_send_header(r);
+
+    if (rc == NGX_ERROR || rc > NGX_OK) {
+        ctx->error = 1;
+        ctx->status = rc;
+        croak("ngx_http_send_header() failed");
+    }
 
 
 void
@@ -381,6 +392,7 @@ has_request_body(r, next)
     dXSTARG;
     ngx_http_request_t   *r;
     ngx_http_perl_ctx_t  *ctx;
+    ngx_int_t             rc;
 
     ngx_http_perl_set_request(r, ctx);
 
@@ -398,7 +410,14 @@ has_request_body(r, next)
         r->request_body_file_log_level = 0;
     }
 
-    ngx_http_read_client_request_body(r, ngx_http_perl_handle_request);
+    rc = ngx_http_read_client_request_body(r, ngx_http_perl_handle_request);
+
+    if (rc >= NGX_HTTP_SPECIAL_RESPONSE) {
+        ctx->error = 1;
+        ctx->status = rc;
+        ctx->next = NULL;
+        croak("ngx_http_read_client_request_body() failed");
+    }
 
     sv_upgrade(TARG, SVt_IV);
     sv_setiv(TARG, 1);
@@ -494,10 +513,17 @@ discard_request_body(r)
 
     ngx_http_request_t   *r;
     ngx_http_perl_ctx_t  *ctx;
+    ngx_int_t             rc;
 
     ngx_http_perl_set_request(r, ctx);
 
-    ngx_http_discard_request_body(r);
+    rc = ngx_http_discard_request_body(r);
+
+    if (rc != NGX_OK) {
+        ctx->error = 1;
+        ctx->status = rc;
+        croak("ngx_http_discard_request_body() failed");
+    }
 
 
 void
@@ -512,6 +538,10 @@ header_out(r, key, value)
 
     ngx_http_perl_set_request(r, ctx);
 
+    if (ctx->error) {
+        croak("header_out(): called after error");
+    }
+
     key = ST(1);
     value = ST(2);
 
@@ -588,10 +618,15 @@ print(r, ...)
     u_char               *p;
     size_t                size;
     STRLEN                len;
+    ngx_int_t             rc;
     ngx_buf_t            *b;
 
     ngx_http_perl_set_request(r, ctx);
 
+    if (ctx->error) {
+        croak("print(): called after error");
+    }
+
     if (items == 2) {
 
         /*
@@ -671,7 +706,12 @@ print(r, ...)
 
     out:
 
-    (void) ngx_http_perl_output(r, ctx, b);
+    rc = ngx_http_perl_output(r, ctx, b);
+
+    if (rc == NGX_ERROR) {
+        ctx->error = 1;
+        croak("ngx_http_perl_output() failed");
+    }
 
 
 void
@@ -683,6 +723,7 @@ sendfile(r, filename, offset = -1, bytes = 0)
     char                      *filename;
     off_t                      offset;
     size_t                     bytes;
+    ngx_int_t                  rc;
     ngx_str_t                  path;
     ngx_buf_t                 *b;
     ngx_open_file_info_t       of;
@@ -690,6 +731,10 @@ sendfile(r, filename, offset = -1, bytes = 0)
 
     ngx_http_perl_set_request(r, ctx);
 
+    if (ctx->error) {
+        croak("sendfile(): called after error");
+    }
+
     filename = SvPV_nolen(ST(1));
 
     if (filename == NULL) {
@@ -762,7 +807,12 @@ sendfile(r, filename, offset = -1, bytes = 0)
     b->file->log = r->connection->log;
     b->file->directio = of.is_directio;
 
-    (void) ngx_http_perl_output(r, ctx, b);
+    rc = ngx_http_perl_output(r, ctx, b);
+
+    if (rc == NGX_ERROR) {
+        ctx->error = 1;
+        croak("ngx_http_perl_output() failed");
+    }
 
 
 void
@@ -771,10 +821,15 @@ flush(r)
 
     ngx_http_request_t   *r;
     ngx_http_perl_ctx_t  *ctx;
+    ngx_int_t             rc;
     ngx_buf_t            *b;
 
     ngx_http_perl_set_request(r, ctx);
 
+    if (ctx->error) {
+        croak("flush(): called after error");
+    }
+
     b = ngx_calloc_buf(r->pool);
     if (b == NULL) {
         XSRETURN_EMPTY;
@@ -784,7 +839,12 @@ flush(r)
 
     ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, "$r->flush");
 
-    (void) ngx_http_perl_output(r, ctx, b);
+    rc = ngx_http_perl_output(r, ctx, b);
+
+    if (rc == NGX_ERROR) {
+        ctx->error = 1;
+        croak("ngx_http_perl_output() failed");
+    }
 
     XSRETURN_EMPTY;
 

diff --git a/src/http/modules/perl/ngx_http_perl_module.c b/src/http/modules/perl/ngx_http_perl_module.c
index c2ef4704803adc52624e93c3d878693ad999e585..ac6a7a2a32216f9dccfae4577d3b198e0de6fed2 100644 (file)
--- a/src/http/modules/perl/ngx_http_perl_module.c
+++ b/src/http/modules/perl/ngx_http_perl_module.c
@@ -246,6 +246,11 @@ ngx_http_perl_handle_request(ngx_http_request_t *r)
     ctx->filename.data = NULL;
     ctx->redirect_uri.len = 0;
 
+    if (rc == NGX_ERROR) {
+        ngx_http_finalize_request(r, rc);
+        return;
+    }
+
     if (ctx->done || ctx->next) {
         ngx_http_finalize_request(r, NGX_DONE);
         return;
@@ -690,6 +695,9 @@ ngx_http_perl_call_handler(pTHX_ ngx_http_request_t *r,
 
     status = 0;
 
+    ctx->error = 0;
+    ctx->status = NGX_OK;
+
     ENTER;
     SAVETMPS;
 
@@ -739,6 +747,18 @@ ngx_http_perl_call_handler(pTHX_ ngx_http_request_t *r,
     FREETMPS;
     LEAVE;
 
+    if (ctx->error) {
+
+        ngx_log_debug1(NGX_LOG_DEBUG_HTTP, c->log, 0,
+                       "call_sv: error, %d", ctx->status);
+
+        if (ctx->status != NGX_OK) {
+            return ctx->status;
+        }
+
+        return NGX_ERROR;
+    }
+
     /* check $@ */
 
     if (SvTRUE(ERRSV)) {

diff --git a/src/http/modules/perl/ngx_http_perl_module.h b/src/http/modules/perl/ngx_http_perl_module.h
index 4f1eaa3a26a948cd0b26a1f26803fc04698e0f3d..5c967dfb323529122c94b4d840e4a0154162e538 100644 (file)
--- a/src/http/modules/perl/ngx_http_perl_module.h
+++ b/src/http/modules/perl/ngx_http_perl_module.h
@@ -29,7 +29,10 @@ typedef struct {
 
     SV                       *next;
 
-    ngx_uint_t                done;       /* unsigned  done:1; */
+    ngx_int_t                 status;
+
+    unsigned                  done:1;
+    unsigned                  error:1;
 
     ngx_array_t              *variables;  /* array of ngx_http_perl_var_t */