You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by mt...@apache.org on 2008/09/18 11:35:30 UTC
svn commit: r696614 - in /httpd/httpd/trunk: CHANGES
modules/proxy/mod_proxy_ajp.c
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
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=696614&r1=696613&r2=696614&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Thu Sep 18 02:35:30 2008
@@ -2,6 +2,14 @@
Changes with Apache 2.3.0
[ When backported to 2.2.x, remove entry from this file ]
+ *) mod_proxy_ajp: Fix wrongly formatted requests where client
+ sets Content-Length header, but doesn't provide a body.
+ Servlet container always expects that next packet is
+ body whenever C-L is present in the headers. This can lead
+ to wrong interpretation of the packets. In this case
+ send the empty body packet, so container can deal with
+ that. [Mladen Turk]
+
*) mod_authnz_ldap: don't return NULL-valued environment variables to
other modules. PR 39045 [Francois Pesce <francois.pesce gmail.com>]
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) {
+ 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
*
@@ -166,6 +187,7 @@
ap_get_module_config(r->server->module_config, &proxy_module);
apr_size_t maxsize = AJP_MSG_BUFFER_SZ;
int send_body = 0;
+ apr_off_t content_length = 0;
if (psf->io_buffer_size_set)
maxsize = psf->io_buffer_size;
@@ -221,6 +243,8 @@
ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,
"proxy: request is chunked");
} else {
+ /* Get client provided Content-Length header */
+ content_length = get_content_length(r);
status = ap_get_brigade(r->input_filters, input_brigade,
AP_MODE_READBYTES, APR_BLOCK_READ,
maxsize - AJP_HEADER_SZ);
@@ -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;
+ }
+ }
}
/* read the response */
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)
Re: svn commit: r696614 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy_ajp.c
Posted by Ruediger Pluem <rp...@apache.org>.
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