1
0
mirror of https://github.com/openbsd/src.git synced 2026-06-18 07:13:36 +02:00

Fix uploads using chunked transfer-encoding

The current code is broken so rework server_read_httpchunks() to properly
implement chunked transfer-encoding. Chunked uploads only matter for
fastcgi handlers, so adjust them to operate with chunked uploads.

The problem is that the CGI spec mandates that CONTENT_LENGTH is set
but for chunked transfers the content-length header is actually not
allowed. Both fastcgi and cgi don't really need CONTENT_LENGTH since the
data is passed via FCGI_STDIN messages or a pipe and in both cases EOF
can be signaled just fine. Still some cgi/fastcgi handlers depend on the
presence of CONTENT_LENGTH and so those fail to process such chunked
uploads. For this reason add a config option to opt-in for chunked
uploads but by default any upload with transfer-encoding chunked will
result in a HTTP 400 error.

OK kirill@ rsadowski@
This commit is contained in:
claudio
2026-06-01 09:28:42 +00:00
parent 6bc4de42aa
commit 23f2ecbee2
5 changed files with 68 additions and 52 deletions
+11 -2
View File
@@ -1,4 +1,4 @@
.\" $OpenBSD: httpd.conf.5,v 1.130 2026/05/17 10:56:41 kirill Exp $
.\" $OpenBSD: httpd.conf.5,v 1.131 2026/06/01 09:28:42 claudio Exp $
.\"
.\" Copyright (c) 2014, 2015 Reyk Floeter <reyk@openbsd.org>
.\"
@@ -14,7 +14,7 @@
.\" ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
.\" OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
.\"
.Dd $Mdocdate: May 17 2026 $
.Dd $Mdocdate: June 1 2026 $
.Dt HTTPD.CONF 5
.Os
.Sh NAME
@@ -404,6 +404,15 @@ This allows FastCGI server chroot to be a directory under httpd chroot.
.It Ic param Ar variable value
Sets a variable that will be sent to the FastCGI server.
Each statement defines one variable.
.It Ic pass Ic chunked
Allow chunked uploads to be passed to the FastCGI server.
By default data uploads with
.Ar Transfer-Encoding: chunked
are forbidden.
By setting
.Ic pass Ic chunked
uploads with chunked encoding are accepted but because the CONTENT_LENGTH
variable can not be set it violates the CGI speficfication.
.El
.Pp
The FastCGI handler will be given the following variables by default:
+5 -3
View File
@@ -1,4 +1,4 @@
/* $OpenBSD: httpd.h,v 1.172 2026/05/17 10:56:41 kirill Exp $ */
/* $OpenBSD: httpd.h,v 1.173 2026/06/01 09:28:42 claudio Exp $ */
/*
* Copyright (c) 2006 - 2015 Reyk Floeter <reyk@openbsd.org>
@@ -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
@@ -544,6 +545,7 @@ struct server_config {
struct server_fcgiparams fcgiparams;
int fcgistrip;
int fcgiallowchunked;
char errdocroot[HTTPD_ERRDOCROOT_MAX];
TAILQ_ENTRY(server_config) entry;
@@ -711,7 +713,7 @@ void server_file_error(struct bufferevent *, short, void *);
/* 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,
+11 -1
View File
@@ -1,4 +1,4 @@
/* $OpenBSD: parse.y,v 1.134 2026/05/17 10:56:41 kirill Exp $ */
/* $OpenBSD: parse.y,v 1.135 2026/06/01 09:28:42 claudio Exp $ */
/*
* Copyright (c) 2020 Matthias Pressfreund <mpfr@fn.de>
@@ -833,6 +833,16 @@ fcgiflags : SOCKET STRING {
}
srv_conf->fcgistrip = $2;
}
| PASS STRING {
if (strcmp($2, "chunked") != 0) {
yyerror("Invalid token '%s' expected 'chunked'",
$2);
free($2);
YYERROR;
}
srv_conf->fcgiallowchunked = 1;
free($2);
}
;
connection : CONNECTION '{' optnl conflags_l '}'
+6 -6
View File
@@ -1,4 +1,4 @@
/* $OpenBSD: server_fcgi.c,v 1.100 2026/03/02 19:24:58 rsadowski Exp $ */
/* $OpenBSD: server_fcgi.c,v 1.101 2026/06/01 09:28:42 claudio Exp $ */
/*
* Copyright (c) 2014 Florian Obser <florian@openbsd.org>
@@ -388,7 +388,7 @@ server_fcgi(struct httpd *env, struct client *clt)
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) {
@@ -414,7 +414,7 @@ server_fcgi(struct httpd *env, struct client *clt)
}
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;
@@ -424,16 +424,16 @@ fcgi_add_stdin(struct client *clt, struct evbuffer *evbuf)
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);
}
+35 -40
View File
@@ -1,4 +1,4 @@
/* $OpenBSD: server_http.c,v 1.162 2026/05/17 10:56:41 kirill Exp $ */
/* $OpenBSD: server_http.c,v 1.163 2026/06/01 09:28:42 claudio Exp $ */
/*
* Copyright (c) 2020 Matthias Pressfreund <mpfr@fn.de>
@@ -512,11 +512,11 @@ server_read_httpcontent(struct bufferevent *bev, void *arg)
{
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)
@@ -524,21 +524,18 @@ server_read_httpcontent(struct bufferevent *bev, void *arg)
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;
@@ -561,13 +558,13 @@ server_read_httpchunks(struct bufferevent *bev, void *arg)
{
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)
@@ -575,17 +572,15 @@ server_read_httpchunks(struct bufferevent *bev, void *arg)
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);
}
@@ -612,18 +607,15 @@ server_read_httpchunks(struct bufferevent *bev, void *arg)
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);
@@ -632,15 +624,15 @@ server_read_httpchunks(struct bufferevent *bev, void *arg)
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 = TOREAD_HTTP_HEADER;
bufferevent_disable(bev, EV_READ);
bev->readcb = server_read_http;
}
}
free(line);
break;
@@ -648,8 +640,6 @@ server_read_httpchunks(struct bufferevent *bev, void *arg)
/* 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;
}
@@ -1465,6 +1455,11 @@ server_response(struct httpd *httpd, struct client *clt)
server_abort_http(clt, 413, "request body too large");
return (-1);
}
if (desc->http_chunked && !srv_conf->fcgiallowchunked) {
server_abort_http(clt, 400,
"Transfer-Encoding: chunked not allowed");
return (-1);
}
if (srv_conf->flags & SRVFLAG_BLOCK) {
server_abort_http(clt, srv_conf->return_code,