grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Javier Moragon
Subject: Re: [PATCH] http module is not checking correctly HTTP headers
Date: Thu, 13 Jan 2022 23:14:54 +0100

I'm sorry, it's my first time using a mailing list for publishing
patches so I sent you the message directly instead of to grub-devel
and my mail client wrapped the patch lines. I attached the patch
Instead of pasting the diff, I hope this time the lines don't get
wrapped.

El jue, 13 ene 2022 a las 5:08, Glenn Washburn
(<development@efficientek.com>) escribió:
>
> 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
>

Attachment: patch.patch
Description: Source code patch


reply via email to

[Prev in Thread] Current Thread [Next in Thread]