On 2021-10-21 16:38, Sebastian Benoit wrote:
>
> This diff makes httpd return "505 HTTP Version Not Supported"
> for < 0.9 and > 1.9 http versions. Anything from 1.1 to 1.9 is
> interpreted as 1.1. This is what nginx does too.
I don't understand why an invalid HTTP version sent by the client
should result in a server error while the problem actually is on
the other side, isn't it? Wouldn't a "400 Bad Request" response
instead of a "505 HTTP Version Not Supported" be more appropriate?
Second, the latest version correctly detects "HTTP/4.0" (for
example) as an unsupported version while "HTTP/123" wrongly is
accepted as "HTTP/1.1".
The suggestion below changes both of the above.
Index: usr.sbin/httpd/server_http.c
===================================================================
RCS file: /cvs/src/usr.sbin/httpd/server_http.c,v
retrieving revision 1.145
diff -u -p -r1.145 server_http.c
--- usr.sbin/httpd/server_http.c 22 Oct 2021 08:51:50 -0000 1.145
+++ usr.sbin/httpd/server_http.c 23 Oct 2021 14:39:50 -0000
@@ -206,8 +206,12 @@ http_version_num(char *version)
return (9);
if (strcmp(version, "HTTP/1.0") == 0)
return (10);
+ /* version strings other than 8 chars long are invalid */
+ if (strlen(version) != 8)
+ return (0);
/* any other version 1.x gets downgraded to 1.1 */
- if (strncmp(version, "HTTP/1", 6) == 0)
+ if (strncmp(version, "HTTP/1.", 7) == 0 &&
+ version[7] >= '1' && version[7] <= '9')
return (11);
return (0);
@@ -350,13 +354,13 @@ server_read_http(struct bufferevent *bev
* be changed independently by the filters later.
* Allow HTTP version 0.9 to 1.1.
* Downgrade http version > 1.1 <= 1.9 to version 1.1.
- * Return HTTP Version Not Supported for anything else.
+ * Anything else is a client error (malformed version).
*/
version = http_version_num(http_version);
if (version == 0) {
- server_abort_http(clt, 505, "bad http version");
+ server_abort_http(clt, 400, "malformed");
goto abort;
} else if (version == 11) {
if ((desc->http_version =
>
> ok?
>
> diff --git usr.sbin/httpd/server_http.c usr.sbin/httpd/server_http.c
> index 6a74f3e45c5..52aaf3711c2 100644
> --- usr.sbin/httpd/server_http.c
> +++ usr.sbin/httpd/server_http.c
> @@ -51,6 +51,7 @@ int server_http_authenticate(struct server_config
> *,
> struct client *);
> char *server_expand_http(struct client *, const char *,
> char *, size_t);
> +int http_version_num(char *);
>
> static struct http_method http_methods[] = HTTP_METHODS;
> static struct http_error http_errors[] = HTTP_ERRORS;
> @@ -198,6 +199,19 @@ done:
> return (ret);
> }
>
> +int http_version_num(char *version)
> +{
> + if (strcmp(version, "HTTP/0.9") == 0)
> + return (9);
> + if (strcmp(version, "HTTP/1.0") == 0)
> + return (10);
> + /* any other version 1.x gets downgraded to 1.1 */
> + if (strncmp(version, "HTTP/1", 6) == 0)
> + return (11);
> +
> + return (0);
> +}
> +
> void
> server_read_http(struct bufferevent *bev, void *arg)
> {
> @@ -207,6 +221,7 @@ server_read_http(struct bufferevent *bev, void *arg)
> char *line = NULL, *key, *value;
> const char *errstr;
> size_t size, linelen;
> + int version;
> struct kv *hdr = NULL;
>
> getmonotime(&clt->clt_tv_last);
> @@ -329,12 +344,29 @@ server_read_http(struct bufferevent *bev, void *arg)
> *desc->http_query++ = '\0';
>
> /*
> - * Have to allocate the strings because they could
> + * We have to allocate the strings because they could
> * be changed independently by the filters later.
> + * Allow HTTP version 0.9 to 1.1.
> + * Downgrade http version > 1.1 <= 1.9 to version 1.1.
> + * Return HTTP Version Not Supported for anything else.
> */
> - if ((desc->http_version =
> - strdup(desc->http_version)) == NULL)
> - goto fail;
> +
> + version = http_version_num(desc->http_version);
> + if (version == 11) {
> + if ((desc->http_version =
> + strdup("HTTP/1.1")) == NULL)
> + goto fail;
> + } else {
> + if ((desc->http_version =
> + strdup(desc->http_version)) == NULL)
> + goto fail;
> + }
> +
> + if (version == 0) {
> + server_abort_http(clt, 505, "bad http version");
> + goto abort;
> + }
> +
>
> if (desc->http_query != NULL &&
> (desc->http_query =
>
