You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by rp...@apache.org on 2005/10/21 15:54:46 UTC
svn commit: r327185 - in /httpd/httpd/trunk: CHANGES modules/proxy/ajp.h
modules/proxy/ajp_header.c modules/proxy/ajp_msg.c
modules/proxy/mod_proxy_ajp.c
Author: rpluem
Date: Fri Oct 21 06:54:38 2005
New Revision: 327185
URL: http://svn.apache.org/viewcvs?rev=327185&view=rev
Log:
* Fix PR37100 (SEGV in mod_proxy_ajp), by sending the data up the filter
chain immediately instead of spooling it completely before passing it
to the filter chain. It contains a bandaid to handle intentional
flushes from Tomcat side. Further explanation in code and report.
ajp.h: Add ajp_msg_reuse prototype
mod_proxy_ajp.c: Adjust logic of ap_proxy_ajp_request
ajp_msg.c: Add ajp_msg_reuse
ajp_header.c: Adjusting logic of ajp_read_header
Modified:
httpd/httpd/trunk/CHANGES
httpd/httpd/trunk/modules/proxy/ajp.h
httpd/httpd/trunk/modules/proxy/ajp_header.c
httpd/httpd/trunk/modules/proxy/ajp_msg.c
httpd/httpd/trunk/modules/proxy/mod_proxy_ajp.c
Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewcvs/httpd/httpd/trunk/CHANGES?rev=327185&r1=327184&r2=327185&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Fri Oct 21 06:54:38 2005
@@ -2,6 +2,9 @@
Changes with Apache 2.3.0
[Remove entries to the current 2.0 and 2.2 section below, when backported]
+ *) mod_proxy_ajp: Do not spool the entire response from AJP backend before
+ sending it up the filter chain. PR37100. [Ruediger Pluem]
+
*) core: AddOutputFilterByType is ignored for proxied requests. PR31226.
[Joe Orton, Ruediger Pluem]
Modified: httpd/httpd/trunk/modules/proxy/ajp.h
URL: http://svn.apache.org/viewcvs/httpd/httpd/trunk/modules/proxy/ajp.h?rev=327185&r1=327184&r2=327185&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/ajp.h (original)
+++ httpd/httpd/trunk/modules/proxy/ajp.h Fri Oct 21 06:54:38 2005
@@ -35,6 +35,7 @@
#include "apr_buckets.h"
#include "apr_md5.h"
#include "apr_network_io.h"
+#include "apr_poll.h"
#include "apr_pools.h"
#include "apr_strings.h"
#include "apr_uri.h"
@@ -185,6 +186,14 @@
* @return APR_SUCCESS or error
*/
apr_status_t ajp_msg_reset(ajp_msg_t *msg);
+
+/**
+ * Reuse an AJP Message
+ *
+ * @param msg AJP Message to reuse
+ * @return APR_SUCCESS or error
+ */
+apr_status_t ajp_msg_reuse(ajp_msg_t *msg);
/**
* Mark the end of an AJP Message
Modified: httpd/httpd/trunk/modules/proxy/ajp_header.c
URL: http://svn.apache.org/viewcvs/httpd/httpd/trunk/modules/proxy/ajp_header.c?rev=327185&r1=327184&r2=327185&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/ajp_header.c (original)
+++ httpd/httpd/trunk/modules/proxy/ajp_header.c Fri Oct 21 06:54:38 2005
@@ -615,12 +615,22 @@
{
apr_byte_t result;
apr_status_t rc;
-
- rc = ajp_msg_create(r->pool, msg);
- if (rc != APR_SUCCESS) {
- ap_log_error(APLOG_MARK, APLOG_ERR, 0, r->server,
- "ajp_read_header: ajp_msg_create failed");
- return rc;
+
+ if (*msg) {
+ rc = ajp_msg_reuse(*msg);
+ if (rc != APR_SUCCESS) {
+ ap_log_error(APLOG_MARK, APLOG_ERR, 0, r->server,
+ "ajp_read_header: ajp_msg_reuse failed");
+ return rc;
+ }
+ }
+ else {
+ rc = ajp_msg_create(r->pool, msg);
+ if (rc != APR_SUCCESS) {
+ ap_log_error(APLOG_MARK, APLOG_ERR, 0, r->server,
+ "ajp_read_header: ajp_msg_create failed");
+ return rc;
+ }
}
ajp_msg_reset(*msg);
rc = ajp_ilink_receive(sock, *msg);
Modified: httpd/httpd/trunk/modules/proxy/ajp_msg.c
URL: http://svn.apache.org/viewcvs/httpd/httpd/trunk/modules/proxy/ajp_msg.c?rev=327185&r1=327184&r2=327185&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/ajp_msg.c (original)
+++ httpd/httpd/trunk/modules/proxy/ajp_msg.c Fri Oct 21 06:54:38 2005
@@ -139,6 +139,24 @@
}
/**
+ * Reuse an AJP Message
+ *
+ * @param msg AJP Message to reuse
+ * @return APR_SUCCESS or error
+ */
+apr_status_t ajp_msg_reuse(ajp_msg_t *msg)
+{
+ apr_byte_t *buf;
+
+ buf = msg->buf;
+ memset(msg, 0, sizeof(ajp_msg_t));
+ msg->buf = buf;
+ msg->header_len = AJP_HEADER_LEN;
+ ajp_msg_reset(msg);
+ return APR_SUCCESS;
+}
+
+/**
* Mark the end of an AJP Message
*
* @param msg AJP Message to end
Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_ajp.c
URL: http://svn.apache.org/viewcvs/httpd/httpd/trunk/modules/proxy/mod_proxy_ajp.c?rev=327185&r1=327184&r2=327185&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/mod_proxy_ajp.c (original)
+++ httpd/httpd/trunk/modules/proxy/mod_proxy_ajp.c Fri Oct 21 06:54:38 2005
@@ -90,6 +90,32 @@
}
/*
+ * XXX: Flushing bandaid
+ *
+ * When processing CMD_AJP13_SEND_BODY_CHUNK AJP messages we will do a poll
+ * with FLUSH_WAIT miliseconds timeout to determine if more data is currently
+ * available at the backend. If there is no more data available, we flush
+ * the data to the client by adding a flush bucket to the brigade we pass
+ * up the filter chain.
+ * This is only a bandaid to fix the AJP/1.3 protocol shortcoming of not
+ * sending (actually not having defined) a flush message, when the data
+ * should be flushed to the client. As soon as this protocol shortcoming is
+ * fixed this code should be removed.
+ *
+ * For further discussion see PR37100.
+ * http://issues.apache.org/bugzilla/show_bug.cgi?id=37100
+ */
+#define FLUSHING_BANDAID 1
+
+#ifdef FLUSHING_BANDAID
+/*
+ * Wait 10000 microseconds to find out if more data is currently
+ * available at the backend. Just an arbitrary choose.
+ */
+#define FLUSH_WAIT 10000
+#endif
+
+/*
* process the request and write the response.
*/
static int ap_proxy_ajp_request(apr_pool_t *p, request_rec *r,
@@ -112,6 +138,10 @@
int havebody = 1;
int isok = 1;
apr_off_t bb_len;
+#ifdef FLUSHING_BANDAID
+ apr_int32_t conn_poll_fd;
+ apr_pollfd_t *conn_poll;
+#endif
/*
* Send the AJP request to the remote server
@@ -134,6 +164,8 @@
/* allocate an AJP message to store the data of the buckets */
status = ajp_alloc_data_msg(r->pool, &buff, &bufsiz, &msg);
if (status != APR_SUCCESS) {
+ /* We had a failure: Close connection to backend */
+ conn->close++;
ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,
"proxy: ajp_alloc_data_msg failed");
return HTTP_INTERNAL_SERVER_ERROR;
@@ -171,6 +203,8 @@
status = apr_brigade_flatten(input_brigade, buff, &bufsiz);
if (status != APR_SUCCESS) {
+ /* We had a failure: Close connection to backend */
+ conn->close++;
apr_brigade_destroy(input_brigade);
ap_log_error(APLOG_MARK, APLOG_ERR, status, r->server,
"proxy: apr_brigade_flatten");
@@ -183,6 +217,8 @@
if (bufsiz > 0) {
status = ajp_send_data_msg(conn->sock, msg, bufsiz);
if (status != APR_SUCCESS) {
+ /* We had a failure: Close connection to backend */
+ conn->close++;
apr_brigade_destroy(input_brigade);
ap_log_error(APLOG_MARK, APLOG_ERR, status, r->server,
"proxy: send failed to %pI (%s)",
@@ -195,9 +231,12 @@
}
/* read the response */
+ conn->data = NULL;
status = ajp_read_header(conn->sock, r,
(ajp_msg_t **)&(conn->data));
if (status != APR_SUCCESS) {
+ /* We had a failure: Close connection to backend */
+ conn->close++;
apr_brigade_destroy(input_brigade);
ap_log_error(APLOG_MARK, APLOG_ERR, status, r->server,
"proxy: read response failed from %pI (%s)",
@@ -209,6 +248,18 @@
result = ajp_parse_type(r, conn->data);
output_brigade = apr_brigade_create(p, r->connection->bucket_alloc);
+#ifdef FLUSHING_BANDAID
+ /*
+ * Prepare apr_pollfd_t struct for later check if there is currently
+ * data available from the backend (do not flush response to client)
+ * or not (flush response to client)
+ */
+ conn_poll = apr_pcalloc(p, sizeof(apr_pollfd_t));
+ conn_poll->reqevents = APR_POLLIN;
+ conn_poll->desc_type = APR_POLL_SOCKET;
+ conn_poll->desc.s = conn->sock;
+#endif
+
bufsiz = AJP13_MAX_SEND_BODY_SZ;
while (isok) {
switch (result) {
@@ -272,16 +323,42 @@
case CMD_AJP13_SEND_BODY_CHUNK:
/* AJP13_SEND_BODY_CHUNK: piece of data */
status = ajp_parse_data(r, conn->data, &size, &buff);
- e = apr_bucket_transient_create(buff, size,
- r->connection->bucket_alloc);
- APR_BRIGADE_INSERT_TAIL(output_brigade, e);
- if (status != APR_SUCCESS)
+ if (status == APR_SUCCESS) {
+ e = apr_bucket_transient_create(buff, size,
+ r->connection->bucket_alloc);
+ APR_BRIGADE_INSERT_TAIL(output_brigade, e);
+
+#ifdef FLUSHING_BANDAID
+ /*
+ * If there is no more data available from backend side
+ * currently, flush response to client.
+ */
+ if (apr_poll(conn_poll, 1, &conn_poll_fd, FLUSH_WAIT)
+ == APR_TIMEUP) {
+ e = apr_bucket_flush_create(r->connection->bucket_alloc);
+ APR_BRIGADE_INSERT_TAIL(output_brigade, e);
+ }
+#endif
+ apr_brigade_length(output_brigade, 0, &bb_len);
+ if (bb_len != -1)
+ conn->worker->s->read += bb_len;
+ if (ap_pass_brigade(r->output_filters,
+ output_brigade) != APR_SUCCESS) {
+ ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
+ "proxy: error processing body");
+ isok = 0;
+ }
+ apr_brigade_cleanup(output_brigade);
+ }
+ else {
isok = 0;
+ }
break;
case CMD_AJP13_END_RESPONSE:
e = apr_bucket_eos_create(r->connection->bucket_alloc);
APR_BRIGADE_INSERT_TAIL(output_brigade, e);
- if (ap_pass_brigade(r->output_filters, output_brigade) != APR_SUCCESS) {
+ if (ap_pass_brigade(r->output_filters,
+ output_brigade) != APR_SUCCESS) {
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
"proxy: error processing body");
isok = 0;
@@ -291,6 +368,19 @@
isok = 0;
break;
}
+
+ /*
+ * If connection has been aborted by client: Stop working.
+ * Nevertheless, we regard our operation so far as a success:
+ * So do not set isok to 0 and set result to CMD_AJP13_END_RESPONSE
+ * But: Close this connection to the backend.
+ */
+ if (r->connection->aborted) {
+ conn->close++;
+ result = CMD_AJP13_END_RESPONSE;
+ break;
+ }
+
if (!isok)
break;
@@ -310,14 +400,11 @@
}
apr_brigade_destroy(input_brigade);
- apr_brigade_length(output_brigade, 0, &bb_len);
- if (bb_len != -1)
- conn->worker->s->read += bb_len;
-
- if (!isok)
- apr_brigade_destroy(output_brigade);
+ apr_brigade_destroy(output_brigade);
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 body failed to %pI (%s)",
conn->worker->cp->addr,
@@ -340,6 +427,8 @@
conn->worker->cp->addr,
conn->worker->hostname);
+ /* We had a failure: Close connection to backend */
+ conn->close++;
return HTTP_SERVICE_UNAVAILABLE;
}