* [PATCH] http: parse HTTP headers case-insensitive
@ 2022-01-14 21:20 Jamo
2022-01-14 22:35 ` Glenn Washburn
0 siblings, 1 reply; 4+ messages in thread
From: Jamo @ 2022-01-14 21:20 UTC (permalink / raw)
To: grub-devel; +Cc: 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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] http: parse HTTP headers case-insensitive
2022-01-14 21:20 [PATCH] http: parse HTTP headers case-insensitive Jamo
@ 2022-01-14 22:35 ` Glenn Washburn
0 siblings, 0 replies; 4+ messages in thread
From: Glenn Washburn @ 2022-01-14 22:35 UTC (permalink / raw)
To: Jamo; +Cc: The development of GNU GRUB
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] http: parse HTTP headers case-insensitive
2022-01-14 23:46 Jamo
@ 2022-01-15 2:13 ` Heinrich Schuchardt
0 siblings, 0 replies; 4+ messages in thread
From: Heinrich Schuchardt @ 2022-01-15 2:13 UTC (permalink / raw)
To: Jamo; +Cc: The development of GNU GRUB
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;
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] http: parse HTTP headers case-insensitive
@ 2022-01-14 23:46 Jamo
2022-01-15 2:13 ` Heinrich Schuchardt
0 siblings, 1 reply; 4+ messages in thread
From: Jamo @ 2022-01-14 23:46 UTC (permalink / raw)
To: grub-devel; +Cc: 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
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-01-15 2:13 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-14 21:20 [PATCH] http: parse HTTP headers case-insensitive Jamo
2022-01-14 22:35 ` Glenn Washburn
2022-01-14 23:46 Jamo
2022-01-15 2:13 ` Heinrich Schuchardt
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.