[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