Not handling case where complete payload is received but not terminating zero byte about nginx_ajp_module HOT 4 CLOSED

Since I don't see any way to attach a file, I'll paste the patch here:

--- ngx_http_ajp_handler.c.orig 2011-02-12 00:44:33.000000000 -0500
+++ ngx_http_ajp_handler.c  2011-04-01 16:29:49.366773997 -0400
@@ -746,6 +746,8 @@
                     if (rc != NGX_OK) {
                         return NGX_ERROR;
                     }
+                    // account for the extra byte at the end of the chunk
+                    a->length++;
 
                     buf->pos += offset;
                     a->state = ngx_http_ajp_st_response_body_data_sending;
@@ -777,12 +779,13 @@
 
         ngx_log_debug2(NGX_LOG_DEBUG_HTTP, p->log, 0,
                 "input filter packet, length:%z, buffer_size:%z",
-                       a->length, ngx_buf_size(buf));
+                       a->length - 1, ngx_buf_size(buf));
 
         /* Get a zero length packet */
-        if (a->length == 0) {
+        if (a->length == 1) {
             if (buf->pos < buf->last) {
                 buf->pos++;
+                a->length--;
             }
 
             continue;
@@ -828,8 +831,8 @@
 
         /* STUB */ b->num = buf->num;
 
-        if (buf->pos + a->length < buf->last) {
-            buf->pos += a->length;
+        if (buf->pos + a->length <= buf->last) {
+            buf->pos += (a->length - 1);
             b->last = buf->pos;
 
             /* The last byte of this message always seems to be

from nginx_ajp_module.

Thanks for this excellent bug report, Chris, you are such a good hacker.

I have added a patch for this module. But it's a little different from yours. I think the length is the meaning of ajp data packet. It should not add a one-extra byte in the length which may confuse its true meaning. So I add a new member of extra_zero_byte in the struct of ngx_http_ajp_ctx_t.

The commit is here: 1445606

I have not tested fully with this patch. Can you check out this experiment branch and test it. Thank you very much. The branch is here:
https://github.com/yaoweibin/nginx_ajp_module/commits/experiment

from nginx_ajp_module.

Hi yaoweibin,
I've looked at your commit, and I see one potential problem. The new member extra_zero_byte isn't initialized to zero anywhere. I think a line like:

a->extra_zero_byte = 0;

needs to be added in the function ngx_http_ajp_reinit_request().

Also, in this "if" statement:

        /* Get a zero length packet */
        if (a->length == 0 && a->extra_zero_byte && (buf->pos < buf->last)) {
            if (buf->pos < buf->last) {
                buf->pos++;
                a->extra_zero_byte = 0;
            }

            continue;
        }

the inner "if" is redundant.

from nginx_ajp_module.

Thanks for your opinion, Chris. I think you are right. I have fixed this problem in the experiment branch.

from nginx_ajp_module.