Skip to content

mod_proxy_fcgi: Support Dynamic Buffering for Large FastCGI Headers #522

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from
Open

mod_proxy_fcgi: Support Dynamic Buffering for Large FastCGI Headers #522

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions include/util_varbuf.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@

#include "httpd.h"

#include "apr_pools.h"
#include "apr_errno.h"

#ifdef __cplusplus
extern "C" {
#endif
Expand Down Expand Up @@ -66,6 +69,8 @@ struct ap_varbuf {
struct ap_varbuf_info *info;
};

typedef struct ap_varbuf ap_varbuf;

/**
* Initialize a resizable buffer. It is safe to re-initialize a previously
* used ap_varbuf. The old buffer will be released when the corresponding
Expand Down Expand Up @@ -189,6 +194,35 @@ AP_DECLARE(apr_status_t) ap_varbuf_cfg_getline(struct ap_varbuf *vb,
ap_configfile_t *cfp,
apr_size_t max_len);

/**
* @brief Allocate and initialize a new dynamic variable buffer.
*
* This function creates a new ap_varbuf structure, allocates an initial buffer
* of the specified size from the given APR pool, and initializes its members.
*
* @param vb Pointer to the pointer that will be set to the new ap_varbuf.
* @param p The APR memory pool from which to allocate the buffer.
* @param initial_size The initial size in bytes for the buffer.
* @return APR_SUCCESS on success.
*/
AP_DECLARE(apr_status_t) ap_varbuf_make(ap_varbuf **vb, apr_pool_t *p, apr_size_t initial_size);

/**
* @brief Append a specified number of characters to a dynamic variable buffer.
*
* This function appends up to @c len characters from the string @c str to the
* dynamic buffer represented by @c vb. If the buffer is not large enough to hold
* the additional characters plus a null terminator, the function reallocates the
* buffer with a larger size. The buffer is always null-terminated after the operation.
*
* @param vb The dynamic variable buffer.
* @param str The string containing the characters to append.
* @param len The number of characters from @c str to append.
* @return APR_SUCCESS on success.
*/
AP_DECLARE(apr_status_t) ap_varbuf_strncat(ap_varbuf *vb, const char *str, apr_size_t len);


#ifdef __cplusplus
}
#endif
Expand Down
123 changes: 73 additions & 50 deletions modules/proxy/mod_proxy_fcgi.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "util_fcgi.h"
#include "util_script.h"
#include "ap_expr.h"
#include "util_varbuf.h"

module AP_MODULE_DECLARE_DATA proxy_fcgi_module;

Expand Down Expand Up @@ -593,6 +594,7 @@ static int handle_headers(request_rec *r, int *state,
return 0;
}


static apr_status_t dispatch(proxy_conn_rec *conn, proxy_dir_conf *conf,
request_rec *r, apr_pool_t *setaside_pool,
apr_uint16_t request_id, const char **err,
Expand All @@ -614,6 +616,9 @@ static apr_status_t dispatch(proxy_conn_rec *conn, proxy_dir_conf *conf,
char stack_iobuf[AP_IOBUFSIZE];
apr_size_t iobuf_size = AP_IOBUFSIZE;
char *iobuf = stack_iobuf;
/* Create our dynamic header buffer for large headers */
ap_varbuf *header_vb = NULL;
ap_varbuf_make(&header_vb, r->pool, 1024);

*err = NULL;
if (conn->worker->s->io_buffer_size_set) {
Expand Down Expand Up @@ -823,61 +828,78 @@ static apr_status_t dispatch(proxy_conn_rec *conn, proxy_dir_conf *conf,
APR_BRIGADE_INSERT_TAIL(ob, b);

if (! seen_end_of_headers) {
int st = handle_headers(r, &header_state,
iobuf, readbuflen);

if (st == 1) {
int status;
/* Try our dynamic header buffer approach first */
ap_varbuf_strncat(header_vb, iobuf, readbuflen);
if (strstr(header_vb->buf, "\r\n\r\n") != NULL) {
while (header_vb->strlen > 0 &&
isspace((unsigned char)header_vb->buf[0])) {
memmove(header_vb->buf, header_vb->buf + 1, header_vb->strlen - 1);
header_vb->strlen--;
header_vb->buf[header_vb->strlen] = '\0';
}
seen_end_of_headers = 1;

status = ap_scan_script_header_err_brigade_ex(r, ob,
NULL, APLOG_MODULE_INDEX);

/* FCGI has its own body framing mechanism which we don't
* match against any provided Content-Length, so let the
* core determine C-L vs T-E based on what's actually sent.
*/
if (!apr_table_get(r->subprocess_env, AP_TRUST_CGILIKE_CL_ENVVAR))
apr_table_unset(r->headers_out, "Content-Length");
apr_table_unset(r->headers_out, "Transfer-Encoding");

/* suck in all the rest */
if (status != OK) {
apr_bucket *tmp_b;
apr_brigade_cleanup(ob);
tmp_b = apr_bucket_eos_create(c->bucket_alloc);
APR_BRIGADE_INSERT_TAIL(ob, tmp_b);

*has_responded = 1;
r->status = status;
rv = ap_pass_brigade(r->output_filters, ob);
if (rv != APR_SUCCESS) {
*err = "passing headers brigade to output filters";
break;
{
int status_hdr;
apr_bucket_brigade *tmp_bb = apr_brigade_create(r->pool, c->bucket_alloc);
apr_bucket *hdr_bucket = apr_bucket_heap_create(header_vb->buf,
header_vb->strlen, NULL, c->bucket_alloc);
APR_BRIGADE_INSERT_TAIL(tmp_bb, hdr_bucket);
hdr_bucket = apr_bucket_eos_create(c->bucket_alloc);
APR_BRIGADE_INSERT_TAIL(tmp_bb, hdr_bucket);
status_hdr = ap_scan_script_header_err_brigade_ex(r, tmp_bb,
NULL, APLOG_MODULE_INDEX);
apr_brigade_cleanup(tmp_bb);
if (status_hdr != OK) {
*has_responded = 1;
return APR_EGENERAL;
}
else if (status == HTTP_NOT_MODIFIED
|| status == HTTP_PRECONDITION_FAILED) {
/* Special 'status' cases handled:
* 1) HTTP 304 response MUST NOT contain
* a message-body, ignore it.
* 2) HTTP 412 response.
* The break is not added since there might
* be more bytes to read from the FCGI
* connection. Even if the message-body is
* ignored (and the EOS bucket has already
* been sent) we want to avoid subsequent
* bogus reads. */
ignore_body = 1;
}
{
char *hdr_end = strstr(header_vb->buf, "\r\n\r\n") + 4;
apr_size_t hdr_total = header_vb->strlen;
apr_size_t remaining = hdr_total - (hdr_end - header_vb->buf);
if (remaining > 0) {
apr_bucket *rem_bucket = apr_bucket_heap_create(hdr_end,
remaining, NULL, c->bucket_alloc);
APR_BRIGADE_INSERT_TAIL(ob, rem_bucket);
}
else {
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(01070)
}
}
else {
int st = handle_headers(r, &header_state,
iobuf, readbuflen);
if (st == 1) {
int status;
seen_end_of_headers = 1;
status = ap_scan_script_header_err_brigade_ex(r, ob,
NULL, APLOG_MODULE_INDEX);
if (!apr_table_get(r->subprocess_env, AP_TRUST_CGILIKE_CL_ENVVAR))
apr_table_unset(r->headers_out, "Content-Length");
apr_table_unset(r->headers_out, "Transfer-Encoding");
if (status != OK) {
apr_bucket *tmp_b;
apr_brigade_cleanup(ob);
tmp_b = apr_bucket_eos_create(c->bucket_alloc);
APR_BRIGADE_INSERT_TAIL(ob, tmp_b);
*has_responded = 1;
r->status = status;
rv = ap_pass_brigade(r->output_filters, ob);
if (rv != APR_SUCCESS) {
*err = "passing headers brigade to output filters";
break;
}
else if (status == HTTP_NOT_MODIFIED
|| status == HTTP_PRECONDITION_FAILED) {
ignore_body = 1;
}
else {
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(01070)
"Error parsing script headers");
rv = APR_EINVAL;
break;
rv = APR_EINVAL;
break;
}
}
}

if (ap_proxy_should_override(conf, r->status) && ap_is_initial_req(r)) {
if (ap_proxy_should_override(conf, r->status) && ap_is_initial_req(r)) {
/*
* set script_error_status to discard
* everything after the headers
Expand Down Expand Up @@ -912,6 +934,7 @@ static apr_status_t dispatch(proxy_conn_rec *conn, proxy_dir_conf *conf,
* headers, so this part of the data will need
* to persist. */
apr_bucket_setaside(b, setaside_pool);
}
}
} else {
/* we've already passed along the headers, so now pass
Expand Down
27 changes: 27 additions & 0 deletions server/util.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3922,3 +3922,30 @@ AP_DECLARE(const char *)ap_dir_fnmatch(ap_dir_match_t *w, const char *path,

return NULL;
}

AP_DECLARE(apr_status_t) ap_varbuf_make(ap_varbuf **vb, apr_pool_t *p, apr_size_t initial_size)
{
*vb = apr_pcalloc(p, sizeof(ap_varbuf));
(*vb)->pool = p;
(*vb)->buf = apr_palloc(p, initial_size);
(*vb)->avail = initial_size;
(*vb)->strlen = 0;
(*vb)->buf[0] = '\0';
return APR_SUCCESS;
}

AP_DECLARE(apr_status_t) ap_varbuf_strncat(ap_varbuf *vb, const char *str, apr_size_t len)
{
if (vb->strlen + len + 1 > vb->avail) {
apr_size_t new_size = vb->strlen + len + 1;
char *newbuf = apr_palloc(vb->pool, new_size);
memcpy(newbuf, vb->buf, vb->strlen);
vb->buf = newbuf;
vb->avail = new_size;
}
memcpy(vb->buf + vb->strlen, str, len);
vb->strlen += len;
vb->buf[vb->strlen] = '\0';
return APR_SUCCESS;
}