Skip to content

Commit 6f2a011

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 6f2a011

File tree

1 file changed

+138
-50
lines changed

1 file changed

+138
-50
lines changed

‎modules/proxy/mod_proxy_fcgi.c

Lines changed: 138 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,
@@ -614,6 +681,9 @@ static apr_status_t dispatch(proxy_conn_rec *conn, proxy_dir_conf *conf,
614681
char stack_iobuf[AP_IOBUFSIZE];
615682
apr_size_t iobuf_size = AP_IOBUFSIZE;
616683
char *iobuf = stack_iobuf;
684+
/* Create our dynamic header buffer for large headers */
685+
ap_varbuf *header_vb = NULL;
686+
ap_varbuf_make(&header_vb, r->pool, 1024);
617687

618688
*err = NULL;
619689
if (conn->worker->s->io_buffer_size_set) {
@@ -823,61 +893,78 @@ static apr_status_t dispatch(proxy_conn_rec *conn, proxy_dir_conf *conf,
823893
APR_BRIGADE_INSERT_TAIL(ob, b);
824894

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

0 commit comments

Comments
 (0)