On Thu, 13 Jan 2022 23:14:54 +0100 Javier Moragon <jamo...@gmail.com> wrote:
> 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. Its okay, I had a few hiccups my first several times sending patches to the list. Sending the patch as an attachment does fix the line wrapping issue, but it creates another one. Its makes it more difficult for reviewers to review it. This list is accustomed to having the patch sent inline (in the text of the email). The advantage of this is that reviewers get to comment inline on specific lines of the patch (like I did for your original patch). Most people use git-format-patch and git-send for creating and sending patches to the list. Also, I've noticed that you changed the indentation of the second change of this latest patch. Could you please create a new patch with the indention exactly as it was? Your editor is probably changing the indentation with out you realizing it. To be clear, the "sizeof" should be preceeded by a string of 2 tabs followed by 2 spaces. Glenn > > El jue, 13 ene 2022 a las 5:08, Glenn Washburn > (<developm...@efficientek.com>) escribió: > > > > On Wed, 12 Jan 2022 23:54:58 +0100 > > Javier Moragon <jamo...@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 > > _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel