Re: [PATCH] http module is not checking correctly HTTP headers

[Glenn Washburn]

On Wed, 12 Jan 2022 23:54:58 +0100
Javier Moragon <jamofer@gmail.com> wrote:

> According to https://www.ietf.org/rfc/rfc2616.txt 4.2, header names
> shall be case insensitive and we are now forced to read headers like
> `Content-Length` capitalized.
> 
> The problem with that is when a HTTP server responds with a
> `content-length` header in lowercase GRUB gets stuck because HTTP
> module doesn't know the length of the transmision and the call never
> ends. I've been able to reproduce it and after ignoring the text case
> it worked perfectly.
> 
> Here is it my patch proposal:
> 
> diff --git a/grub-core/net/http.c b/grub-core/net/http.c
> index b616cf40b..570fa3934 100644
> --- a/grub-core/net/http.c
> +++ b/grub-core/net/http.c
> @@ -130,7 +130,7 @@ parse_line (grub_file_t file, http_data_t data,
> char *ptr, grub_size_t len)
>        data->first_line_recv = 1;
>        return GRUB_ERR_NONE;
>      }
> -  if (grub_memcmp (ptr, "Content-Length: ", sizeof ("Content-Length: ") - 1)
> +  if (grub_strncasecmp (ptr, "Content-Length: ", grub_strlen
> ("Content-Length: ") )

I don't think there should be a new line here. And why change to
grub_strlen? Now the length is calculated everytime at runtime instead
of once at compile time.

>        == 0 && !data->size_recv)
>      {
>        ptr += sizeof ("Content-Length: ") - 1;
> @@ -138,8 +138,8 @@ parse_line (grub_file_t file, http_data_t data,
> char *ptr, grub_size_t len)
>        data->size_recv = 1;
>        return GRUB_ERR_NONE;
>      }
> -  if (grub_memcmp (ptr, "Transfer-Encoding: chunked",
> -    sizeof ("Transfer-Encoding: chunked") - 1) == 0)
> +  if (grub_strncasecmp (ptr, "Transfer-Encoding: chunked",
> +    grub_strlen ("Transfer-Encoding: chunked") ) == 0)

Ditto on the grub_strlen. I also don't like the original indentation of
this line and think that it should align with "ptr".

>      {
>        data->chunked = 1;
>        return GRUB_ERR_NONE;

Otherwise, it seems like good patch.

Glenn