Skip to content

Commit 0eec747

Browse files
author
Arturo Bernal
committed
mod_proxy_fcgi: support dynamic buffering for large FastCGI headers
This patch fixes Bug 64919 by replacing the fixed 8192‐byte header buffer with a dynamically resizable buffer that can handle arbitrarily long headers. It also adds a trimming step to remove any leading whitespace from the header data before parsing, ensuring that FastCGI responses are correctly interpreted.
1 parent fecd8da commit 0eec747

File tree

3 files changed

+134
-50
lines changed

3 files changed

+134
-50
lines changed

‎include/util_varbuf.h‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,9 @@
3636

3737
#include "httpd.h"
3838

39+
#include "apr_pools.h"
40+
#include "apr_errno.h"
41+
3942
#ifdef __cplusplus
4043
extern "C" {
4144
#endif
@@ -66,6 +69,8 @@ struct ap_varbuf {
6669
struct ap_varbuf_info *info;
6770
};
6871

72+
typedef struct ap_varbuf ap_varbuf;
73+
6974
/**
7075
* Initialize a resizable buffer. It is safe to re-initialize a previously
7176
* used ap_varbuf. The old buffer will be released when the corresponding
@@ -189,6 +194,35 @@ AP_DECLARE(apr_status_t) ap_varbuf_cfg_getline(struct ap_varbuf *vb,
189194
ap_configfile_t *cfp,
190195
apr_size_t max_len);
191196

197+
/**
198+
* @brief Allocate and initialize a new dynamic variable buffer.
199+
*
200+
* This function creates a new ap_varbuf structure, allocates an initial buffer
201+
* of the specified size from the given APR pool, and initializes its members.
202+
*
203+
* @param vb Pointer to the pointer that will be set to the new ap_varbuf.
204+
* @param p The APR memory pool from which to allocate the buffer.
205+
* @param initial_size The initial size in bytes for the buffer.
206+
* @return APR_SUCCESS on success.
207+
*/
208+
AP_DECLARE(apr_status_t) ap_varbuf_make(ap_varbuf **vb, apr_pool_t *p, apr_size_t initial_size);
209+
210+
/**
211+
* @brief Append a specified number of characters to a dynamic variable buffer.
212+
*
213+
* This function appends up to @c len characters from the string @c str to the
214+
* dynamic buffer represented by @c vb. If the buffer is not large enough to hold
215+
* the additional characters plus a null terminator, the function reallocates the
216+
* buffer with a larger size. The buffer is always null-terminated after the operation.
217+
*
218+
* @param vb The dynamic variable buffer.
219+
* @param str The string containing the characters to append.
220+
* @param len The number of characters from @c str to append.
221+
* @return APR_SUCCESS on success.
222+
*/
223+
AP_DECLARE(apr_status_t) ap_varbuf_strncat(ap_varbuf *vb, const char *str, apr_size_t len);
224+
225+
192226
#ifdef __cplusplus
193227
}
194228
#endif

‎modules/proxy/mod_proxy_fcgi.c‎

Lines changed: 73 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include "util_fcgi.h"
1919
#include "util_script.h"
2020
#include "ap_expr.h"
21+
#include "util_varbuf.h"
2122

2223
module AP_MODULE_DECLARE_DATA proxy_fcgi_module;
2324

@@ -593,6 +594,7 @@ static int handle_headers(request_rec *r, int *state,
593594
return 0;
594595
}
595596

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

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

825830
if (! seen_end_of_headers) {
826-
int st = handle_headers(r, &header_state,
827-
iobuf, readbuflen);
828-
829-
if (st == 1) {
830-
int status;
831+
/* Try our dynamic header buffer approach first */
832+
ap_varbuf_strncat(header_vb, iobuf, readbuflen);
833+
if (strstr(header_vb->buf, "\r\n\r\n") != NULL) {
834+
while (header_vb->strlen > 0 &&
835+
isspace((unsigned char)header_vb->buf[0])) {
836+
memmove(header_vb->buf, header_vb->buf + 1, header_vb->strlen - 1);
837+
header_vb->strlen--;
838+
header_vb->buf[header_vb->strlen] = '\0';
839+
}
831840
seen_end_of_headers = 1;
832-
833-
status = ap_scan_script_header_err_brigade_ex(r, ob,
834-
NULL, APLOG_MODULE_INDEX);
835-
836-
/* FCGI has its own body framing mechanism which we don't
837-
* match against any provided Content-Length, so let the
838-
* core determine C-L vs T-E based on what's actually sent.
839-
*/
840-
if (!apr_table_get(r->subprocess_env, AP_TRUST_CGILIKE_CL_ENVVAR))
841-
apr_table_unset(r->headers_out, "Content-Length");
842-
apr_table_unset(r->headers_out, "Transfer-Encoding");
843-
844-
/* suck in all the rest */
845-
if (status != OK) {
846-
apr_bucket *tmp_b;
847-
apr_brigade_cleanup(ob);
848-
tmp_b = apr_bucket_eos_create(c->bucket_alloc);
849-
APR_BRIGADE_INSERT_TAIL(ob, tmp_b);
850-
851-
*has_responded = 1;
852-
r->status = status;
853-
rv = ap_pass_brigade(r->output_filters, ob);
854-
if (rv != APR_SUCCESS) {
855-
*err = "passing headers brigade to output filters";
856-
break;
841+
{
842+
int status_hdr;
843+
apr_bucket_brigade *tmp_bb = apr_brigade_create(r->pool, c->bucket_alloc);
844+
apr_bucket *hdr_bucket = apr_bucket_heap_create(header_vb->buf,
845+
header_vb->strlen, NULL, c->bucket_alloc);
846+
APR_BRIGADE_INSERT_TAIL(tmp_bb, hdr_bucket);
847+
hdr_bucket = apr_bucket_eos_create(c->bucket_alloc);
848+
APR_BRIGADE_INSERT_TAIL(tmp_bb, hdr_bucket);
849+
status_hdr = ap_scan_script_header_err_brigade_ex(r, tmp_bb,
850+
NULL, APLOG_MODULE_INDEX);
851+
apr_brigade_cleanup(tmp_bb);
852+
if (status_hdr != OK) {
853+
*has_responded = 1;
854+
return APR_EGENERAL;
857855
}
858-
else if (status == HTTP_NOT_MODIFIED
859-
|| status == HTTP_PRECONDITION_FAILED) {
860-
/* Special 'status' cases handled:
861-
* 1) HTTP 304 response MUST NOT contain
862-
* a message-body, ignore it.
863-
* 2) HTTP 412 response.
864-
* The break is not added since there might
865-
* be more bytes to read from the FCGI
866-
* connection. Even if the message-body is
867-
* ignored (and the EOS bucket has already
868-
* been sent) we want to avoid subsequent
869-
* bogus reads. */
870-
ignore_body = 1;
856+
}
857+
{
858+
char *hdr_end = strstr(header_vb->buf, "\r\n\r\n") + 4;
859+
apr_size_t hdr_total = header_vb->strlen;
860+
apr_size_t remaining = hdr_total - (hdr_end - header_vb->buf);
861+
if (remaining > 0) {
862+
apr_bucket *rem_bucket = apr_bucket_heap_create(hdr_end,
863+
remaining, NULL, c->bucket_alloc);
864+
APR_BRIGADE_INSERT_TAIL(ob, rem_bucket);
871865
}
872-
else {
873-
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(01070)
866+
}
867+
}
868+
else {
869+
int st = handle_headers(r, &header_state,
870+
iobuf, readbuflen);
871+
if (st == 1) {
872+
int status;
873+
seen_end_of_headers = 1;
874+
status = ap_scan_script_header_err_brigade_ex(r, ob,
875+
NULL, APLOG_MODULE_INDEX);
876+
if (!apr_table_get(r->subprocess_env, AP_TRUST_CGILIKE_CL_ENVVAR))
877+
apr_table_unset(r->headers_out, "Content-Length");
878+
apr_table_unset(r->headers_out, "Transfer-Encoding");
879+
if (status != OK) {
880+
apr_bucket *tmp_b;
881+
apr_brigade_cleanup(ob);
882+
tmp_b = apr_bucket_eos_create(c->bucket_alloc);
883+
APR_BRIGADE_INSERT_TAIL(ob, tmp_b);
884+
*has_responded = 1;
885+
r->status = status;
886+
rv = ap_pass_brigade(r->output_filters, ob);
887+
if (rv != APR_SUCCESS) {
888+
*err = "passing headers brigade to output filters";
889+
break;
890+
}
891+
else if (status == HTTP_NOT_MODIFIED
892+
|| status == HTTP_PRECONDITION_FAILED) {
893+
ignore_body = 1;
894+
}
895+
else {
896+
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(01070)
874897
"Error parsing script headers");
875-
rv = APR_EINVAL;
876-
break;
898+
rv = APR_EINVAL;
899+
break;
900+
}
877901
}
878-
}
879-
880-
if (ap_proxy_should_override(conf, r->status) && ap_is_initial_req(r)) {
902+
if (ap_proxy_should_override(conf, r->status) && ap_is_initial_req(r)) {
881903
/*
882904
* set script_error_status to discard
883905
* everything after the headers
@@ -912,6 +934,7 @@ static apr_status_t dispatch(proxy_conn_rec *conn, proxy_dir_conf *conf,
912934
* headers, so this part of the data will need
913935
* to persist. */
914936
apr_bucket_setaside(b, setaside_pool);
937+
}
915938
}
916939
} else {
917940
/* we've already passed along the headers, so now pass

‎server/util.c‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3922,3 +3922,30 @@ AP_DECLARE(const char *)ap_dir_fnmatch(ap_dir_match_t *w, const char *path,
39223922

39233923
return NULL;
39243924
}
3925+
3926+
AP_DECLARE(apr_status_t) ap_varbuf_make(ap_varbuf **vb, apr_pool_t *p, apr_size_t initial_size)
3927+
{
3928+
*vb = apr_pcalloc(p, sizeof(ap_varbuf));
3929+
(*vb)->pool = p;
3930+
(*vb)->buf = apr_palloc(p, initial_size);
3931+
(*vb)->avail = initial_size;
3932+
(*vb)->strlen = 0;
3933+
(*vb)->buf[0] = '\0';
3934+
return APR_SUCCESS;
3935+
}
3936+
3937+
AP_DECLARE(apr_status_t) ap_varbuf_strncat(ap_varbuf *vb, const char *str, apr_size_t len)
3938+
{
3939+
if (vb->strlen + len + 1 > vb->avail) {
3940+
apr_size_t new_size = vb->strlen + len + 1;
3941+
char *newbuf = apr_palloc(vb->pool, new_size);
3942+
memcpy(newbuf, vb->buf, vb->strlen);
3943+
vb->buf = newbuf;
3944+
vb->avail = new_size;
3945+
}
3946+
memcpy(vb->buf + vb->strlen, str, len);
3947+
vb->strlen += len;
3948+
vb->buf[vb->strlen] = '\0';
3949+
return APR_SUCCESS;
3950+
}
3951+

0 commit comments

Comments
 (0)