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: Glenn Washburn
Subject: Re: [PATCH] http module is not checking correctly HTTP headers
Date: Thu, 13 Jan 2022 21:26:57 -0600

On Fri, 14 Jan 2022 00:05:59 +0100
Jamo <jamofer@gmail.com> wrote:

> From: Javier Moragon <jamofer@gmail.com>
> 
> I applied the last suggestion in this patch and I've finally realized 
> how to use git send-email.
> 
> I'm sorry for the unnecesary mails and I hope for next contributions
> I won't make the same mistakes, Thank you!

No worries, we all had to learn at some point. There are still a couple
issues. One is that this space is for the commit message. For instance,
based on your first email it might be:

  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.

Another issue is that when you update a patch, you should update its
version number. See the "-v" option in git-format-patch. Also, the list
convention is to start a new thread when submitting an updated patch.
So no need to do have an "In-Reply-To" header on the next update.

> 
> ---

If you want to have some text that will not get recorded in the commit,
like the text after your email above, you can put that here.

>  grub-core/net/http.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/grub-core/net/http.c b/grub-core/net/http.c
> index b616cf40b..7a56ec7ef 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: ", sizeof ("Content-Length: ") 
> - 1)
>        == 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",
> +                        sizeof ("Transfer-Encoding: chunked") - 1) == 0)

Almost there. The code style guidelines for this project use a TAB
character for every 8 spaces. It looks like you used all spaces to do
the indent.

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

Glenn



reply via email to

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