[PATCH] http: parse HTTP headers case-insensitive

[PATCH] http: parse HTTP headers case-insensitive

[Jamo]

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 transmission and the call never
ends.
---
 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..d3ba8c799 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)
     {
       data->chunked = 1;
       return GRUB_ERR_NONE;
-- 
2.32.0

Re: [PATCH] http: parse HTTP headers case-insensitive

[Heinrich Schuchardt]

On 1/15/22 00:46, Jamo 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 transmission and the call never
> ends.

This seems to be v2 of the patch.

To make reviewing easier , please, consider the following for future 
submissions:

Put the version number into the subject line:

     [PATCH v2] http: parse HTTP headers case-insensitive

and add a description of the change between the patches after triple 
hyphens:

---
v2:
	typos fixed

No need to resend this time.

> ---
>   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..d3ba8c799 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)

This comparison might still be too restrictive: The RFC 2616 
specification says:

"The field value MAY be preceded by any amount of LWS (linear white 
space), though a single SP is preferred."

LWS  = [CRLF] 1*( SP | HT )

So the header might look like

"Content-length:\t  3495".

or like

"Content-length:3495".

So here you should not test for the space but only for the string 
"Content-Length:".

>         == 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)

Here you need two comparisons and a function to skip the optional 
whitespace between the tokens. You should check that "chunked" is 
followed by LWS.

"Transfer-Encoding: chunkedMeat" should not match.
"Transfer-Encoding:chunked\t" should match.

Best regards

Heinrich

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

[PATCH] http: parse HTTP headers case-insensitive

[Jamo]

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.
---
 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..d3ba8c799 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)
     {
       data->chunked = 1;
       return GRUB_ERR_NONE;
-- 
2.32.0

Re: [PATCH] http: parse HTTP headers case-insensitive

[Glenn Washburn]

On Fri, 14 Jan 2022 22:20:17 +0100
Jamo <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.

I'm not sure if Daniel likes these backticks, perhaps normal quotes
would be better. I'll let him chime in if so.

> 
> The problem with that is when a HTTP server responds with a
> `content-length` header in lowercase GRUB gets stuck because HTTP

There should be a ',' right after "lowercase". 

> module doesn't know the length of the transmision and the call never

Just noticed that "transmision" should be spelled "transmission".

> ends.
> ---
>  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..d3ba8c799 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)
>      {
>        data->chunked = 1;
>        return GRUB_ERR_NONE;

Aside from the minor stuff in the commit message, the patch looks good
to me.

Reviewed-by: Glenn Washburn <development@efficientek.com>

Glenn