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.
Better diff that stops the bufferevent read callback when switching back
to HTTP header mode.
--
:wq Claudio
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 30 Oct 2025 22:04:45 -0000
@@ -578,12 +578,11 @@ server_read_httpchunks(struct buffereven
/* Read chunk data */
if ((off_t)size > clt->clt_toread) {
size = clt->clt_toread;
- if (server_bufferevent_write_chunk(clt, src, size)
- == -1)
+ if (fcgi_add_stdin(clt, src) == -1)
goto fail;
clt->clt_toread = 0;
} else {
- if (server_bufferevent_write_buffer(clt, src) == -1)
+ if (fcgi_add_stdin(clt, src) == -1)
goto fail;
clt->clt_toread -= size;
}
@@ -613,11 +612,6 @@ 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) {
@@ -633,15 +627,14 @@ 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 */
+ fcgi_add_stdin(clt, NULL);
clt->clt_toread = TOREAD_HTTP_HEADER;
+ bufferevent_disable(bev, EV_READ);
bev->readcb = server_read_http;
+ free(line);
+ return;
}
free(line);
break;
@@ -649,8 +642,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;
}
