You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Ruediger Pluem <rp...@apache.org> on 2008/09/18 21:21:24 UTC

Re: svn commit: r696614 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy_ajp.c


On 09/18/2008 11:35 AM, mturk@apache.org wrote:
> Author: mturk
> Date: Thu Sep 18 02:35:30 2008
> New Revision: 696614
> 
> URL: http://svn.apache.org/viewvc?rev=696614&view=rev
> Log:
> Always send body (zero size at least) whenever C-L is present in the request
> 
> Modified:
>     httpd/httpd/trunk/CHANGES
>     httpd/httpd/trunk/modules/proxy/mod_proxy_ajp.c
> 
> Modified: httpd/httpd/trunk/CHANGES

> Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_ajp.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_ajp.c?rev=696614&r1=696613&r2=696614&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/proxy/mod_proxy_ajp.c (original)
> +++ httpd/httpd/trunk/modules/proxy/mod_proxy_ajp.c Thu Sep 18 02:35:30 2008
> @@ -116,6 +116,27 @@
>      }
>  }
>  
> +static apr_off_t get_content_length(request_rec * r)
> +{
> +    apr_off_t len = 0;
> +
> +    if (r->clength > 0) {
> +        return r->clength;
> +    }
> +    else if (r->main == NULL || r->main == r) {

There are subrequests that have themselves as parent?

> +        const char *clp = apr_table_get(r->headers_in, "Content-Length");
> +
> +        if (clp) {
> +            char *errp;
> +            if (apr_strtoff(&len, clp, &errp, 10) || *errp || len < 0) {
> +                len = 0; /* parse error */
> +            }
> +        }
> +    }
> +
> +    return len;
> +}
> +
>  /*
>   * XXX: AJP Auto Flushing
>   *

> @@ -277,6 +301,27 @@
>              conn->worker->s->transferred += bufsiz;
>              send_body = 1;
>          }
> +        else if (content_length > 0) {
> +            ap_log_error(APLOG_MARK, APLOG_ERR, status, r->server,
> +                         "proxy: read zero bytes, expecting"
> +                         " %" APR_OFF_T_FMT " bytes",
> +                         content_length);
> +            status = ajp_send_data_msg(conn->sock, msg, 0);
> +            if (status != APR_SUCCESS) {
> +                /* We had a failure: Close connection to backend */
> +                conn->close++;
> +                ap_log_error(APLOG_MARK, APLOG_ERR, status, r->server,
> +                            "proxy: send failed to %pI (%s)",
> +                            conn->worker->cp->addr,
> +                            conn->worker->hostname);
> +                return HTTP_INTERNAL_SERVER_ERROR;
> +            }
> +            else {
> +                /* Client send zero bytes with C-L > 0
> +                 */
> +                return HTTP_BAD_REQUEST;
> +            }

But this does mean that the container receives a request, but we doesn't read the answer.
Is the container capable of handling this situation?
Or does it simply error out without sending anything and keeping the connection open on
requests with C-L set and a null length data packet sent?

Regards

Rüdiger

Re: svn commit: r696614 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy_ajp.c

Posted by "Plüm, Rüdiger, VF-Group" <ru...@vodafone.com>.
 

> -----Ursprüngliche Nachricht-----
> Von: Mladen Turk 
> Gesendet: Freitag, 19. September 2008 11:45
> An: dev@httpd.apache.org
> Betreff: Re: svn commit: r696614 - in /httpd/httpd/trunk: 
> CHANGES modules/proxy/mod_proxy_ajp.c
> 
> Plüm, Rüdiger, VF-Group wrote:
> >  
> >> Without the patch the container side will presume the next packet
> >> (can be new request or ping) is first body packet.
> > 
> > So with the patch the connection to the container can be reused and
> > the state of the connection in the container is that it waits for a
> > new request on this connection?
> > 
> 
> Yes.
> 
> It's quite straightforward. Per AJP protocol spec,
> whenever there is C-L > 0 header present the container
> expects two packets:
> a) request itself and b) the first body packet.
> Additional body packets are then requested by container
> by GET_BODY_PACKET message. If the first body packet
> is zero-length or container decides it doesn't
> wish to read rest of the body it terminates the request
> (without closing the physical connection of course)


Thanks for clarification.

Regards

Rüdiger

Re: svn commit: r696614 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy_ajp.c

Posted by Mladen Turk <mt...@apache.org>.
Plüm, Rüdiger, VF-Group wrote:
>  
>> Without the patch the container side will presume the next packet
>> (can be new request or ping) is first body packet.
> 
> So with the patch the connection to the container can be reused and
> the state of the connection in the container is that it waits for a
> new request on this connection?
> 

Yes.

It's quite straightforward. Per AJP protocol spec,
whenever there is C-L > 0 header present the container
expects two packets:
a) request itself and b) the first body packet.
Additional body packets are then requested by container
by GET_BODY_PACKET message. If the first body packet
is zero-length or container decides it doesn't
wish to read rest of the body it terminates the request
(without closing the physical connection of course)


Regards
-- 
^(TM)

Re: svn commit: r696614 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy_ajp.c

Posted by "Plüm, Rüdiger, VF-Group" <ru...@vodafone.com>.
 

> -----Ursprüngliche Nachricht-----
> Von: Mladen Turk 
> Gesendet: Freitag, 19. September 2008 11:18
> An: dev@httpd.apache.org
> Betreff: Re: svn commit: r696614 - in /httpd/httpd/trunk: 
> CHANGES modules/proxy/mod_proxy_ajp.c
> 
> Ruediger Pluem wrote:
 
> > Or does it simply error out without sending anything and 
> keeping the 
> > connection open on
> > requests with C-L set and a null length data packet sent?
> > 
> 
> Without the patch the container side will presume the next packet
> (can be new request or ping) is first body packet.

So with the patch the connection to the container can be reused and
the state of the connection in the container is that it waits for a
new request on this connection?

Regards

Rüdiger

Re: svn commit: r696614 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy_ajp.c

Posted by Mladen Turk <mt...@apache.org>.
Ruediger Pluem wrote:
> 
>> +    if (r->clength > 0) {
>> +        return r->clength;
>> +    }
>> +    else if (r->main == NULL || r->main == r) {
> 
> There are subrequests that have themselves as parent?
>

It's code from mod_jk. Can be overengineering.

> 
> But this does mean that the container receives a request, but we doesn't 
> read the answer.
> Is the container capable of handling this situation?

Yes. Zero length body means disconnected client (per AJP protocol)

> Or does it simply error out without sending anything and keeping the 
> connection open on
> requests with C-L set and a null length data packet sent?
> 

Without the patch the container side will presume the next packet
(can be new request or ping) is first body packet.

Regards
-- 
^(TM)