On Fri, Oct 31, 2025 at 02:13:45PM +0100, Claudio Jeker wrote:
> On Fri, Oct 31, 2025 at 10:56:40AM +0000, Crystal Kolipe wrote:
> > On Thu, Oct 30, 2025 at 10:51:42PM +0100, Claudio Jeker wrote:
> > > On Wed, Oct 29, 2025 at 06:06:36PM +0100, Claudio Jeker wrote:
> > > > On Thu, Sep 25, 2025 at 01:31:07PM +0200, Claudio Jeker wrote:
> > > > > On Thu, Sep 25, 2025 at 01:14:16PM +0200, Florian Obser wrote:
> > > > > > Nobody stepped up to fix chunked encoding in fastcgi. I think we
> > > > > > should
> > > > > > disable it.
> > > > > > OK?
> > > > >
> > > > > No.
> > > >
> > > > Just to be clear, this turns off chunked encoding for output but the
> > > > problem is when data is sent with chunked encoding in a POST. I doubt
> > > > that
> > > > forcing clt->clt_fcgi.chunked = 0 will fix chunked encoding handling
> > > > during input processing. My fear is that httpd has no code to handle
> > > > chunked encoding in POST request (as in sent by the client) and all of
> > > > that needs to be written first.
> > >
> > > The problem is in server_read_httpchunks() which is the rev 1.1 copy of
> > > code from relayd and is not doing the right thing. When florian@ added
> > > POST support he only did it for server_read_httpcontent() but did not
> > > adjust server_read_httpchunks(). All of this was done 11 years ago so this
> > > bug has been in httpd since the beginning.
> > >
> > > I think the following diff below is what is needed. It seems to work with
> > > the provided test case. It is too late to do further tests, so I hope
> > > someone else will do that.
> >
> > Testing this diff, (as well as the second version you posted), the problem
> > of user-submitted body content being returned by the server seems to be
> > fixed.
> >
> > However, if a POST request is sent with transfer-encoding:chunked and a
> > non-zero length for the body, then the server returns nothing and the tcp
> > connection remains open.
> >
> > Is that the expected or desired behaviour?
> >
> > (Tested with both slowcgi and the custom fcgi handler that runs on
> > *.exoticsilicon.com, with the same results.)
>
> There is a lot more broken in that part of the code. This needs a bigger
> hammer. Now I have something that is better but at least php-fpm still
> does not give me the input stream :(
Better version where the chunked encoding decoder is not totally broken.
The problem is that fastcgi does not really specify how to handle chunked
encoding and it seems passing just the data makes at least php-fpm not
happy.
So I wonder if I need to pass all data and let the fastcgi handler do the
chunk decoding (again). This is just dumb. Now there is no documentation
on how this should be done (or at least I did not find it).
Another option is to take the hit in httpd and spool the file into memory
(and maybe onto disk) and then convert the Transfer-Encoding: chunked into
a Content-Lenght: XYZ header. It seems some http servers do that.
--
:wq Claudio
Index: httpd.h
===================================================================
RCS file: /cvs/src/usr.sbin/httpd/httpd.h,v
diff -u -p -r1.165 httpd.h
--- httpd.h 8 Oct 2024 05:28:11 -0000 1.165
+++ httpd.h 31 Oct 2025 12:54:33 -0000
@@ -108,7 +108,8 @@ enum httpchunk {
TOREAD_HTTP_CHUNK_LENGTH = -3,
TOREAD_HTTP_CHUNK_TRAILER = -4,
TOREAD_HTTP_NONE = -5,
- TOREAD_HTTP_RANGE = TOREAD_HTTP_CHUNK_LENGTH
+ TOREAD_HTTP_RANGE = TOREAD_HTTP_CHUNK_LENGTH,
+ TOREAD_HTTP_FINAL_CHUNK_TRAILER = -6,
};
#if DEBUG
@@ -706,7 +707,7 @@ void server_file_error(struct buffereve
/* server_fcgi.c */
int server_fcgi(struct httpd *, struct client *);
-int fcgi_add_stdin(struct client *, struct evbuffer *);
+int fcgi_add_stdin(struct client *, const char *, size_t);
/* httpd.c */
void event_again(struct event *, int, short,
Index: server_fcgi.c
===================================================================
RCS file: /cvs/src/usr.sbin/httpd/server_fcgi.c,v
diff -u -p -r1.97 server_fcgi.c
--- server_fcgi.c 8 Nov 2023 19:19:10 -0000 1.97
+++ server_fcgi.c 31 Oct 2025 18:52:32 -0000
@@ -386,7 +386,7 @@ server_fcgi(struct httpd *env, struct cl
bufferevent_enable(clt->clt_bev, EV_READ);
} else {
bufferevent_disable(clt->clt_bev, EV_READ);
- fcgi_add_stdin(clt, NULL);
+ fcgi_add_stdin(clt, NULL, 0);
}
if (strcmp(desc->http_version, "HTTP/1.1") == 0) {
@@ -412,7 +412,7 @@ server_fcgi(struct httpd *env, struct cl
}
int
-fcgi_add_stdin(struct client *clt, struct evbuffer *evbuf)
+fcgi_add_stdin(struct client *clt, const char *buf, size_t len)
{
struct fcgi_record_header h;
@@ -422,16 +422,16 @@ fcgi_add_stdin(struct client *clt, struc
h.id = htons(1);
h.padding_len = 0;
- if (evbuf == NULL) {
+ if (len == 0) {
h.content_len = 0;
return bufferevent_write(clt->clt_srvbev, &h,
sizeof(struct fcgi_record_header));
} else {
- h.content_len = htons(EVBUFFER_LENGTH(evbuf));
+ h.content_len = htons(len);
if (bufferevent_write(clt->clt_srvbev, &h,
sizeof(struct fcgi_record_header)) == -1)
return -1;
- return bufferevent_write_buffer(clt->clt_srvbev, evbuf);
+ return bufferevent_write(clt->clt_srvbev, buf, len);
}
return (0);
}
Index: server_http.c
===================================================================
RCS file: /cvs/src/usr.sbin/httpd/server_http.c,v
diff -u -p -r1.155 server_http.c
--- server_http.c 22 Dec 2024 13:51:42 -0000 1.155
+++ server_http.c 31 Oct 2025 16:39:24 -0000
@@ -513,11 +513,11 @@ server_read_httpcontent(struct buffereve
{
struct client *clt = arg;
struct evbuffer *src = EVBUFFER_INPUT(bev);
- size_t size;
+ const char *buf = EVBUFFER_DATA(src);
+ size_t size = EVBUFFER_LENGTH(src);
getmonotime(&clt->clt_tv_last);
- size = EVBUFFER_LENGTH(src);
DPRINTF("%s: session %d: size %lu, to read %lld", __func__,
clt->clt_id, size, clt->clt_toread);
if (!size)
@@ -525,21 +525,18 @@ server_read_httpcontent(struct buffereve
if (clt->clt_toread > 0) {
/* Read content data */
- if ((off_t)size > clt->clt_toread) {
+ if ((off_t)size > clt->clt_toread)
size = clt->clt_toread;
- if (fcgi_add_stdin(clt, src) == -1)
- goto fail;
- clt->clt_toread = 0;
- } else {
- if (fcgi_add_stdin(clt, src) == -1)
- goto fail;
- clt->clt_toread -= size;
- }
+
+ if (fcgi_add_stdin(clt, buf, size) == -1)
+ goto fail;
+ evbuffer_drain(src, size);
+ clt->clt_toread -= size;
DPRINTF("%s: done, size %lu, to read %lld", __func__,
size, clt->clt_toread);
}
if (clt->clt_toread == 0) {
- fcgi_add_stdin(clt, NULL);
+ fcgi_add_stdin(clt, NULL, 0);
clt->clt_toread = TOREAD_HTTP_HEADER;
bufferevent_disable(bev, EV_READ);
bev->readcb = server_read_http;
@@ -562,13 +559,13 @@ server_read_httpchunks(struct buffereven
{
struct client *clt = arg;
struct evbuffer *src = EVBUFFER_INPUT(bev);
+ const char *buf = EVBUFFER_DATA(src);
+ size_t size = EVBUFFER_LENGTH(src);
char *line;
long long llval;
- size_t size;
getmonotime(&clt->clt_tv_last);
- size = EVBUFFER_LENGTH(src);
DPRINTF("%s: session %d: size %lu, to read %lld", __func__,
clt->clt_id, size, clt->clt_toread);
if (!size)
@@ -576,17 +573,15 @@ server_read_httpchunks(struct buffereven
if (clt->clt_toread > 0) {
/* Read chunk data */
- if ((off_t)size > clt->clt_toread) {
+ if ((off_t)size > clt->clt_toread)
size = clt->clt_toread;
- if (server_bufferevent_write_chunk(clt, src, size)
- == -1)
- goto fail;
- clt->clt_toread = 0;
- } else {
- if (server_bufferevent_write_buffer(clt, src) == -1)
- goto fail;
- clt->clt_toread -= size;
- }
+ if (fcgi_add_stdin(clt, buf, size) == -1)
+ goto fail;
+ clt->clt_toread -= size;
+ evbuffer_drain(src, size);
+
+ if (clt->clt_toread == 0)
+ clt->clt_toread = TOREAD_HTTP_CHUNK_TRAILER;
DPRINTF("%s: done, size %lu, to read %lld", __func__,
size, clt->clt_toread);
}
@@ -613,18 +608,15 @@ server_read_httpchunks(struct buffereven
return;
}
- if (server_bufferevent_print(clt, line) == -1 ||
- server_bufferevent_print(clt, "\r\n") == -1) {
- free(line);
- goto fail;
- }
free(line);
if ((clt->clt_toread = llval) == 0) {
DPRINTF("%s: last chunk", __func__);
- clt->clt_toread = TOREAD_HTTP_CHUNK_TRAILER;
+ fcgi_add_stdin(clt, NULL, 0);
+ clt->clt_toread = TOREAD_HTTP_FINAL_CHUNK_TRAILER;
}
break;
+ case TOREAD_HTTP_FINAL_CHUNK_TRAILER:
case TOREAD_HTTP_CHUNK_TRAILER:
/* Last chunk is 0 bytes followed by trailer and empty line */
line = evbuffer_readln(src, NULL, EVBUFFER_EOL_CRLF_STRICT);
@@ -633,15 +625,15 @@ server_read_httpchunks(struct buffereven
bufferevent_enable(bev, EV_READ);
return;
}
- if (server_bufferevent_print(clt, line) == -1 ||
- server_bufferevent_print(clt, "\r\n") == -1) {
- free(line);
- goto fail;
- }
if (strlen(line) == 0) {
/* Switch to HTTP header mode */
- clt->clt_toread = TOREAD_HTTP_HEADER;
- bev->readcb = server_read_http;
+ if (clt->clt_toread == TOREAD_HTTP_CHUNK_TRAILER) {
+ clt->clt_toread = TOREAD_HTTP_CHUNK_LENGTH;
+ } else {
+ clt->clt_toread = 0;
+ bufferevent_disable(bev, EV_READ);
+ bev->readcb = server_read_http;
+ }
}
free(line);
break;
@@ -649,8 +641,6 @@ server_read_httpchunks(struct buffereven
/* Chunk is terminated by an empty newline */
line = evbuffer_readln(src, NULL, EVBUFFER_EOL_CRLF_STRICT);
free(line);
- if (server_bufferevent_print(clt, "\r\n") == -1)
- goto fail;
clt->clt_toread = TOREAD_HTTP_CHUNK_LENGTH;
break;
}
