You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by ro...@apache.org on 2006/01/07 22:37:42 UTC
svn commit: r366926 - in /httpd/httpd/branches/fcgi-proxy-dev/modules/proxy:
fcgi_protocol.h mod_proxy_fcgi.c
Author: rooneg
Date: Sat Jan 7 13:37:40 2006
New Revision: 366926
URL: http://svn.apache.org/viewcvs?rev=366926&view=rev
Log:
Make the handling of FastCGI request headers consistent. Previously, we
used a struct to hold the data when writing it, but read it into an array
when reading it. This meant that the knowledge of the header layout was
in two places. This change moves both sides to using an array, and adds
a set of #defines for the offsets into the array, so neither side can get
out of sync.
This also moves the logic for setting up the content length bytes into one
place, where before we had it in several places.
* modules/proxy/fcgi_protocol.h
(fcgi_header): Removed.
(FCGI_HDR_VERSION_OFFSET,
FCGI_HDR_TYPE_OFFSET,
FCGI_HDR_REQUEST_ID_B1_OFFSET,
FCGI_HDR_REQUEST_ID_B0_OFFSET,
FCGI_HDR_CONTENT_LEN_B1_OFFSET,
FCGI_HDR_CONTENT_LEN_B0_OFFSET,
FCGI_HDR_PADDING_LEN_OFFSET,
FCGI_HDR_RESERVED_OFFSET): New constants.
* modules/proxy/mod_proxy_fcgi.c
(fill_in_header): Take an array, not a struct pointer, and handle all
the contents of the header, not just the type and request id.
(send_begin_request, send_environment, dispatch): Update for new way
to fill in headers.
Modified:
httpd/httpd/branches/fcgi-proxy-dev/modules/proxy/fcgi_protocol.h
httpd/httpd/branches/fcgi-proxy-dev/modules/proxy/mod_proxy_fcgi.c
Modified: httpd/httpd/branches/fcgi-proxy-dev/modules/proxy/fcgi_protocol.h
URL: http://svn.apache.org/viewcvs/httpd/httpd/branches/fcgi-proxy-dev/modules/proxy/fcgi_protocol.h?rev=366926&r1=366925&r2=366926&view=diff
==============================================================================
--- httpd/httpd/branches/fcgi-proxy-dev/modules/proxy/fcgi_protocol.h (original)
+++ httpd/httpd/branches/fcgi-proxy-dev/modules/proxy/fcgi_protocol.h Sat Jan 7 13:37:40 2006
@@ -41,16 +41,31 @@
#define FCGI_UNKNOWN_TYPE 11
#define FCGI_MAXTYPE (FCGI_UNKNOWN_TYPE)
-typedef struct {
- unsigned char version;
- unsigned char type;
- unsigned char requestIdB1;
- unsigned char requestIdB0;
- unsigned char contentLengthB1;
- unsigned char contentLengthB0;
- unsigned char paddingLength;
- unsigned char reserved;
-} fcgi_header;
+/*
+ * In a padding free world, a FastCGI Header would look like this:
+ *
+ * typedef struct {
+ * unsigned char version;
+ * unsigned char type;
+ * unsigned char requestIdB1;
+ * unsigned char requestIdB0;
+ * unsigned char contentLengthB1;
+ * unsigned char contentLengthB0;
+ * unsigned char paddingLength;
+ * unsigned char reserved;
+ * } fcgi_header;
+ *
+ * But since we can't be sure the compiler won't introduce arbitrary padding,
+ * we use an unsigned char array, and the following offsets to build headers.
+ */
+#define FCGI_HDR_VERSION_OFFSET 0
+#define FCGI_HDR_TYPE_OFFSET 1
+#define FCGI_HDR_REQUEST_ID_B1_OFFSET 2
+#define FCGI_HDR_REQUEST_ID_B0_OFFSET 3
+#define FCGI_HDR_CONTENT_LEN_B1_OFFSET 4
+#define FCGI_HDR_CONTENT_LEN_B0_OFFSET 5
+#define FCGI_HDR_PADDING_LEN_OFFSET 6
+#define FCGI_HDR_RESERVED_OFFSET 7
/*
* Number of bytes in a fcgi_header. Future versions of the protocol
Modified: httpd/httpd/branches/fcgi-proxy-dev/modules/proxy/mod_proxy_fcgi.c
URL: http://svn.apache.org/viewcvs/httpd/httpd/branches/fcgi-proxy-dev/modules/proxy/mod_proxy_fcgi.c?rev=366926&r1=366925&r2=366926&view=diff
==============================================================================
--- httpd/httpd/branches/fcgi-proxy-dev/modules/proxy/mod_proxy_fcgi.c (original)
+++ httpd/httpd/branches/fcgi-proxy-dev/modules/proxy/mod_proxy_fcgi.c Sat Jan 7 13:37:40 2006
@@ -82,42 +82,46 @@
}
/*
- * Initialize a fastcgi_header H of type TYPE with id ID.
+ * Fill in a fastcgi request header with the following type, request id,
+ * content length, and padding length.
*
- * Sets the content length and padding length to 0, the caller should
- * reset them to more appropriate values later if needed.
+ * The header array must be at least FCGI_HEADER_LEN bytes long.
*/
-static void fill_in_header(fcgi_header *h, int type, int id)
+static void fill_in_header(unsigned char header[],
+ unsigned char type,
+ apr_uint16_t request_id,
+ apr_uint16_t content_len,
+ unsigned char padding_len)
{
- h->version = FCGI_VERSION;
+ header[FCGI_HDR_VERSION_OFFSET] = 1;
- h->type = type;
+ header[FCGI_HDR_TYPE_OFFSET] = type;
- h->requestIdB1 = ((id >> 8) & 0xff);
- h->requestIdB0 = ((id) & 0xff);
+ header[FCGI_HDR_REQUEST_ID_B1_OFFSET] = ((request_id >> 8) & 0xff);
+ header[FCGI_HDR_REQUEST_ID_B0_OFFSET] = ((request_id) & 0xff);
- h->contentLengthB1 = 0;
- h->contentLengthB0 = 0;
- h->paddingLength = 0;
+ header[FCGI_HDR_CONTENT_LEN_B1_OFFSET] = ((content_len >> 8) & 0xff);
+ header[FCGI_HDR_CONTENT_LEN_B0_OFFSET] = ((content_len) & 0xff);
+
+ header[FCGI_HDR_PADDING_LEN_OFFSET] = padding_len;
+
+ header[FCGI_HDR_RESERVED_OFFSET] = 0;
}
static apr_status_t send_begin_request(proxy_conn_rec *conn, int request_id)
{
struct iovec vec[2];
- fcgi_header header;
+ unsigned char header[FCGI_HEADER_LEN];
fcgi_begin_request_body brb;
apr_size_t len;
- fill_in_header(&header, FCGI_BEGIN_REQUEST, request_id);
-
- header.contentLengthB1 = ((sizeof(brb) >> 8) & 0xff);
- header.contentLengthB0 = ((sizeof(brb)) & 0xff);
+ fill_in_header(header, FCGI_BEGIN_REQUEST, request_id, sizeof(brb), 0);
brb.roleB1 = ((FCGI_RESPONDER >> 8) & 0xff);
brb.roleB0 = ((FCGI_RESPONDER) & 0xff);
brb.flags = FCGI_KEEP_CONN;
- vec[0].iov_base = &header;
+ vec[0].iov_base = header;
vec[0].iov_len = sizeof(header);
vec[1].iov_base = &brb;
vec[1].iov_len = sizeof(brb);
@@ -131,15 +135,13 @@
const apr_array_header_t *envarr;
const apr_table_entry_t *elts;
struct iovec vec[2];
- fcgi_header header;
+ unsigned char header[FCGI_HEADER_LEN];
apr_size_t bodylen;
char *body, *itr;
apr_status_t rv;
apr_size_t len;
int i;
- fill_in_header(&header, FCGI_PARAMS, request_id);
-
ap_add_common_vars(r);
ap_add_cgi_vars(r);
@@ -234,12 +236,9 @@
itr += vallen;
}
- /* First we send the actual env... */
- header.contentLengthB1 = ((bodylen >> 8) & 0xff);
- header.contentLengthB0 = ((bodylen) & 0xff);
- header.paddingLength = 0;
+ fill_in_header(header, FCGI_PARAMS, request_id, bodylen, 0);
- vec[0].iov_base = &header;
+ vec[0].iov_base = header;
vec[0].iov_len = sizeof(header);
vec[1].iov_base = body;
vec[1].iov_len = bodylen;
@@ -249,12 +248,9 @@
return rv;
}
- /* Then, an empty record to signify the end of the stream. */
- header.contentLengthB1 = 0;
- header.contentLengthB0 = 0;
- header.paddingLength = 0;
+ fill_in_header(header, FCGI_PARAMS, request_id, 0, 0);
- vec[0].iov_base = &header;
+ vec[0].iov_base = header;
vec[0].iov_len = sizeof(header);
return apr_socket_sendv(conn->sock, vec, 1, &len);
@@ -380,11 +376,9 @@
apr_status_t rv = APR_SUCCESS;
conn_rec *c = r->connection;
struct iovec vec[2];
- fcgi_header header;
+ unsigned char header[FCGI_HEADER_LEN];
apr_pollfd_t pfd;
- fill_in_header(&header, FCGI_STDIN, request_id);
-
pfd.desc_type = APR_POLL_SOCKET;
pfd.desc.s = conn->sock;
pfd.p = r->pool;
@@ -428,10 +422,10 @@
break;
}
- header.contentLengthB1 = ((writebuflen >> 8) & 0xff);
- header.contentLengthB0 = ((writebuflen) & 0xff);
+ fill_in_header(header, FCGI_STDIN, request_id,
+ (apr_uint16_t) writebuflen, 0);
- vec[0].iov_base = &header;
+ vec[0].iov_base = header;
vec[0].iov_len = sizeof(header);
vec[1].iov_base = writebuf;
vec[1].iov_len = writebuflen;
@@ -450,10 +444,9 @@
if (last_stdin) {
pfd.reqevents = APR_POLLIN; /* Done with input data */
- header.contentLengthB1 = 0;
- header.contentLengthB0 = 0;
+ fill_in_header(header, FCGI_STDIN, request_id, 0, 0);
- vec[0].iov_base = &header;
+ vec[0].iov_base = header;
vec[0].iov_len = sizeof(header);
rv = apr_socket_sendv(conn->sock, vec, 1, &len);
@@ -470,26 +463,18 @@
int rid, type;
apr_bucket *b;
char plen;
- /*
- * below mapped to fcgi_header layout. We
- * use a unsigned char array to ensure the
- * shifts are correct and avoid any potential
- * internal padding when using structs.
- */
- unsigned char fheader[FCGI_HEADER_LEN];
- memset(fheader, 0, FCGI_HEADER_LEN);
memset(readbuf, 0, sizeof(readbuf));
/* First, we grab the header... */
readbuflen = FCGI_HEADER_LEN;
- rv = apr_socket_recv(conn->sock, (char *) fheader, &readbuflen);
+ rv = apr_socket_recv(conn->sock, (char *) header, &readbuflen);
if (rv != APR_SUCCESS) {
break;
}
- dump_header_to_log(r, fheader, readbuflen);
+ dump_header_to_log(r, header, readbuflen);
if (readbuflen != FCGI_HEADER_LEN) {
ap_log_error(APLOG_MARK, APLOG_ERR, 0, r->server,
@@ -500,17 +485,18 @@
break;
}
- if (fheader[0] != FCGI_VERSION) {
+ if (header[FCGI_HDR_VERSION_OFFSET] != FCGI_VERSION) {
ap_log_error(APLOG_MARK, APLOG_ERR, 0, r->server,
"proxy: FCGI: Got bogus version %d",
- (int) fheader[0]);
+ (int) header[FCGI_HDR_VERSION_OFFSET]);
rv = APR_EINVAL;
break;
}
- type = fheader[1];
+ type = header[FCGI_HDR_TYPE_OFFSET];
- rid = (fheader[2] << 8) | fheader[3];
+ rid = header[FCGI_HDR_REQUEST_ID_B1_OFFSET] << 8;
+ rid |= header[FCGI_HDR_REQUEST_ID_B0_OFFSET];
if (rid != request_id) {
ap_log_error(APLOG_MARK, APLOG_ERR, 0, r->server,
@@ -522,9 +508,10 @@
#endif
}
- clen = (fheader[4] << 8) | fheader[5];
+ clen = header[FCGI_HDR_CONTENT_LEN_B1_OFFSET] << 8;
+ clen |= header[FCGI_HDR_CONTENT_LEN_B0_OFFSET];
- plen = fheader[6];
+ plen = header[FCGI_HDR_PADDING_LEN_OFFSET];
recv_again:
if (clen > sizeof(readbuf) - 1) {