Skip to content

Commit 707299d

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 707299d

File tree

1 file changed

+139
-50
lines changed

1 file changed

+139
-50
lines changed

‎modules/proxy/mod_proxy_fcgi.c‎

Lines changed: 139 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 <ctype.h>
2122

2223
module AP_MODULE_DECLARE_DATA proxy_fcgi_module;
2324

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

597+
/**
598+
* @struct ap_varbuf
599+
* @brief Structure representing a dynamically resizable buffer.
600+
*
601+
* This structure holds a character buffer along with its current length and the total
602+
* allocated size. The buffer is allocated from the provided APR pool.
603+
*/
604+
typedef struct {
605+
char *buf;
606+
apr_size_t cur_len;
607+
apr_size_t size;
608+
apr_pool_t *pool;
609+
} ap_varbuf;
610+
611+
/**
612+
* @brief Allocate and initialize a new dynamic variable buffer.
613+
*
614+
* This function creates a new ap_varbuf structure, allocates an initial buffer
615+
* of the specified size from the given APR pool, and initializes its members.
616+
*
617+
* @param vb A pointer to the pointer that will hold the allocated ap_varbuf.
618+
* @param p The APR memory pool from which to allocate the buffer.
619+
* @param initial_size The initial size in bytes for the buffer.
620+
* @return APR_SUCCESS on success.
621+
*/
622+
static apr_status_t ap_varbuf_make(ap_varbuf **vb, apr_pool_t *p, apr_size_t initial_size)
623+
{
624+
*vb = apr_pcalloc(p, sizeof(ap_varbuf));
625+
(*vb)->pool = p;
626+
(*vb)->buf = apr_palloc(p, initial_size);
627+
(*vb)->size = initial_size;
628+
(*vb)->cur_len = 0;
629+
(*vb)->buf[0] = '\0';
630+
return APR_SUCCESS;
631+
}
632+
633+
634+
/**
635+
* @brief Append a specified number of characters to a dynamic variable buffer.
636+
*
637+
* This function appends up to @c len characters from the string @c str to the
638+
* dynamic buffer represented by @c vb. If the buffer is not large enough to hold
639+
* the additional characters plus a null terminator, the function reallocates the
640+
* buffer with a larger size. The buffer is always null-terminated after the operation.
641+
*
642+
* @param vb The dynamic variable buffer to which the string will be appended.
643+
* @param str The string containing the characters to append.
644+
* @param len The number of characters from @c str to append.
645+
* @return APR_SUCCESS on success.
646+
*/
647+
static apr_status_t ap_varbuf_strncat(ap_varbuf *vb, const char *str, apr_size_t len)
648+
{
649+
if (vb->cur_len + len + 1 > vb->size) {
650+
apr_size_t new_size = vb->cur_len + len + 1;
651+
char *newbuf = apr_palloc(vb->pool, new_size);
652+
memcpy(newbuf, vb->buf, vb->cur_len);
653+
vb->buf = newbuf;
654+
vb->size = new_size;
655+
}
656+
memcpy(vb->buf + vb->cur_len, str, len);
657+
vb->cur_len += len;
658+
vb->buf[vb->cur_len] = '\0';
659+
return APR_SUCCESS;
660+
}
661+
662+
596663
static apr_status_t dispatch(proxy_conn_rec *conn, proxy_dir_conf *conf,
597664
request_rec *r, apr_pool_t *setaside_pool,
598665
apr_uint16_t request_id, const char **err,
@@ -640,6 +707,10 @@ static apr_status_t dispatch(proxy_conn_rec *conn, proxy_dir_conf *conf,
640707
ib = apr_brigade_create(r->pool, c->bucket_alloc);
641708
ob = apr_brigade_create(r->pool, c->bucket_alloc);
642709

710+
/* Create our dynamic header buffer for large headers */
711+
ap_varbuf *header_vb = NULL;
712+
ap_varbuf_make(&header_vb, r->pool, 1024);
713+
643714
while (! done) {
644715
apr_interval_time_t timeout;
645716
apr_size_t len;
@@ -823,61 +894,78 @@ static apr_status_t dispatch(proxy_conn_rec *conn, proxy_dir_conf *conf,
823894
APR_BRIGADE_INSERT_TAIL(ob, b);
824895

825896
if (! seen_end_of_headers) {
826-
int st = handle_headers(r, &header_state,
827-
iobuf, readbuflen);
828-
829-
if (st == 1) {
830-
int status;
897+
/* Try our dynamic header buffer approach first */
898+
ap_varbuf_strncat(header_vb, iobuf, readbuflen);
899+
if (strstr(header_vb->buf, "\r\n\r\n") != NULL) {
900+
while (header_vb->cur_len > 0 &&
901+
isspace((unsigned char)header_vb->buf[0])) {
902+
memmove(header_vb->buf, header_vb->buf + 1, header_vb->cur_len - 1);
903+
header_vb->cur_len--;
904+
header_vb->buf[header_vb->cur_len] = '\0';
905+
}
831906
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;
907+
{
908+
int status_hdr;
909+
apr_bucket_brigade *tmp_bb = apr_brigade_create(r->pool, c->bucket_alloc);
910+
apr_bucket *hdr_bucket = apr_bucket_heap_create(header_vb->buf,
911+
header_vb->cur_len, NULL, c->bucket_alloc);
912+
APR_BRIGADE_INSERT_TAIL(tmp_bb, hdr_bucket);
913+
hdr_bucket = apr_bucket_eos_create(c->bucket_alloc);
914+
APR_BRIGADE_INSERT_TAIL(tmp_bb, hdr_bucket);
915+
status_hdr = ap_scan_script_header_err_brigade_ex(r, tmp_bb,
916+
NULL, APLOG_MODULE_INDEX);
917+
apr_brigade_cleanup(tmp_bb);
918+
if (status_hdr != OK) {
919+
*has_responded = 1;
920+
return APR_EGENERAL;
857921
}
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;
922+
}
923+
{
924+
char *hdr_end = strstr(header_vb->buf, "\r\n\r\n") + 4;
925+
apr_size_t hdr_total = header_vb->cur_len;
926+
apr_size_t remaining = hdr_total - (hdr_end - header_vb->buf);
927+
if (remaining > 0) {
928+
apr_bucket *rem_bucket = apr_bucket_heap_create(hdr_end,
929+
remaining, NULL, c->bucket_alloc);
930+
APR_BRIGADE_INSERT_TAIL(ob, rem_bucket);
871931
}
872-
else {
873-
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(01070)
932+
}
933+
}
934+
else {
935+
int st = handle_headers(r, &header_state,
936+
iobuf, readbuflen);
937+
if (st == 1) {
938+
int status;
939+
seen_end_of_headers = 1;
940+
status = ap_scan_script_header_err_brigade_ex(r, ob,
941+
NULL, APLOG_MODULE_INDEX);
942+
if (!apr_table_get(r->subprocess_env, AP_TRUST_CGILIKE_CL_ENVVAR))
943+
apr_table_unset(r->headers_out, "Content-Length");
944+
apr_table_unset(r->headers_out, "Transfer-Encoding");
945+
if (status != OK) {
946+
apr_bucket *tmp_b;
947+
apr_brigade_cleanup(ob);
948+
tmp_b = apr_bucket_eos_create(c->bucket_alloc);
949+
APR_BRIGADE_INSERT_TAIL(ob, tmp_b);
950+
*has_responded = 1;
951+
r->status = status;
952+
rv = ap_pass_brigade(r->output_filters, ob);
953+
if (rv != APR_SUCCESS) {
954+
*err = "passing headers brigade to output filters";
955+
break;
956+
}
957+
else if (status == HTTP_NOT_MODIFIED
958+
|| status == HTTP_PRECONDITION_FAILED) {
959+
ignore_body = 1;
960+
}
961+
else {
962+
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(01070)
874963
"Error parsing script headers");
875-
rv = APR_EINVAL;
876-
break;
964+
rv = APR_EINVAL;
965+
break;
966+
}
877967
}
878-
}
879-
880-
if (ap_proxy_should_override(conf, r->status) && ap_is_initial_req(r)) {
968+
if (ap_proxy_should_override(conf, r->status) && ap_is_initial_req(r)) {
881969
/*
882970
* set script_error_status to discard
883971
* everything after the headers
@@ -912,6 +1000,7 @@ static apr_status_t dispatch(proxy_conn_rec *conn, proxy_dir_conf *conf,
9121000
* headers, so this part of the data will need
9131001
* to persist. */
9141002
apr_bucket_setaside(b, setaside_pool);
1003+
}
9151004
}
9161005
} else {
9171006
/* we've already passed along the headers, so now pass

0 commit comments

Comments
 (0)