Matthias Pressfreund([email protected]) on 2021.10.23 17:16:18 +0200:
> 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?
because
505 HTTP Version Not Supported
means
The server does not support the HTTP protocol version used in the request.
I think its appropiate, nginx responds the same.
> 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".
i might add that indeed, thx.
>
> 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 =
> >
>
--
