[PATCH v3] http: Allow use of non-standard TCP/IP ports

2022-01-16 Thread Stephen Balousek
Allow the use of HTTP servers listening on ports other 80. This is done
with an extension to the http notation:

  (http[,server[,port]])

 - or -

  (http[,server[:port]])

Signed-off-by: Stephen Balousek 
---

Thanks for that, Daniel. Sorry to have so much trouble making sense of 
grub_strtoul()! The change you suggested works like a charm.
I also fixed my example IPv6 address to be a proper "documentation" address.

- Steve

 docs/grub.texi   | 33 +
 grub-core/net/http.c | 40 ++--
 2 files changed, 71 insertions(+), 2 deletions(-)

diff --git a/docs/grub.texi b/docs/grub.texi
index f8b4b3b21..2f43b73a1 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -3004,6 +3004,39 @@ environment variable @samp{net_default_server} is used.
 Before using the network drive, you must initialize the network.
 @xref{Network}, for more information.
 
+For the @samp{http} network protocol, @code{@var{server}} may specify a
+port number other than the default value of @samp{80}. The server name
+and port number are separated by either @samp{,} or @samp{:}.
+For IPv6 addresses, the server name and port number may only be separated
+by @samp{,}.
+
+@itemize @bullet
+@item
+@code{(http,@var{server},@var{port})}
+
+@item
+@code{(http,@var{server}:@var{port})}
+@end itemize
+
+These examples all reference an @samp{http} server at address
+@samp{192.168.0.100} listening on the non-standard port of @samp{3000}.
+In these examples, the DNS name @samp{grub.example.com} is resolved
+to @samp{192.168.0.100}.
+
+@example
+(http,grub.example.com,3000)
+(http,grub.example.com:3000)
+(http,192.168.0.100,3000)
+(http,192.168.0.100:3000)
+@end example
+
+Referencing an @samp{http} server over IPv6 on the non-standard
+port of @samp{3000} would look like this:
+
+@example
+(http,2001:db8::1,3000)
+@end example
+
 If you boot GRUB from a CD-ROM, @samp{(cd)} is available. @xref{Making
 a GRUB bootable CD-ROM}, for details.
 
diff --git a/grub-core/net/http.c b/grub-core/net/http.c
index b616cf40b..4274b8208 100644
--- a/grub-core/net/http.c
+++ b/grub-core/net/http.c
@@ -312,6 +312,10 @@ http_establish (struct grub_file *file, grub_off_t offset, 
int initial)
   int i;
   struct grub_net_buff *nb;
   grub_err_t err;
+  char *server_name;
+  char *port_string;
+  const char *port_string_end;
+  unsigned long port_number;
 
   nb = grub_netbuff_alloc (GRUB_NET_TCP_RESERVE_SIZE
   + sizeof ("GET ") - 1
@@ -390,10 +394,42 @@ http_establish (struct grub_file *file, grub_off_t 
offset, int initial)
   grub_netbuff_put (nb, 2);
   grub_memcpy (ptr, "\r\n", 2);
 
-  data->sock = grub_net_tcp_open (file->device->net->server,
- HTTP_PORT, http_receive,
+  port_string = grub_strrchr (file->device->net->server, ',');
+  if (port_string == NULL)
+{
+  /* If ',port' is not found in the http server string, look for ':port' */
+  port_string = grub_strrchr (file->device->net->server, ':');
+  /* For IPv6 addresses, the ':port' syntax is not supported and ',port' 
must be used. */
+  if (port_string != NULL && grub_strchr (file->device->net->server, ':') 
!= port_string)
+ port_string = NULL;
+}
+  if (port_string != NULL)
+{
+  port_number = grub_strtoul (port_string + 1, &port_string_end, 10);
+  if (*(port_string + 1) == '\0' || *port_string_end != '\0')
+ return grub_error (GRUB_ERR_BAD_NUMBER, N_("non-numeric or invalid 
port number `%s'"), port_string + 1);
+  if (port_number == 0 || port_number > 65535)
+ return grub_error (GRUB_ERR_OUT_OF_RANGE, N_("port number `%s' not in 
the range of 1 to 65535"), port_string + 1);
+
+  server_name = grub_strdup (file->device->net->server);
+  if (server_name == NULL)
+ return grub_errno;
+  server_name[port_string - file->device->net->server] = '\0';
+}
+  else
+{
+  port_number = HTTP_PORT;
+  server_name = file->device->net->server;
+}
+
+  data->sock = grub_net_tcp_open (server_name,
+ port_number, http_receive,
  http_err, NULL,
  file);
+
+  if (server_name != file->device->net->server)
+  grub_free (server_name);
+
   if (!data->sock)
 {
   grub_netbuff_free (nb);
-- 
2.34.1


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


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

2022-01-16 Thread 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.
---
v2:
compare header value ignoring lws
content-size value parsing should start after 'Content-Size:'
extract check header and its value in two functions

First of all, thank you for helping me how to contribute sending
patches through mail and with your suggestions.

I applied the suggestions you told about and I extracted that logic into two
new static functions in order to increase code readability.

I know that sizeof("inline string") would have better performance
if I have done it inline but if I try to apply it inside the extracted function
it will always return the size of the bigger const string passed to the
function. I think that kind of optimization here it doesn't worth VS code
readability, we are not going to deal with a large number of headers.

I still not very sure about the naming of "is_header" and
"is_header_value". And "is_header_value" is only valid when it is a header
without multiple values. As far as I understand if we had headers with multiple
values we should admit multi-line values starting with LWS, to have the header
name more than once, to parse elements by commas...

I think if we have to deal with that in the future the code could
be refactored instead of doing it now.

I have another doubt, I see that the project has some unit tests
but the http module is all static functions. I've been doing
these unit tests out of the project with the two new functions
I added trying the possible cases succesfully.

Should I adapt the code in order to be testable and include
the tests that confirms my patch works?

Thank you very much!

 grub-core/net/http.c | 41 +++--
 1 file changed, 35 insertions(+), 6 deletions(-)

diff --git a/grub-core/net/http.c b/grub-core/net/http.c
index b616cf40b..aed40f536 100644
--- a/grub-core/net/http.c
+++ b/grub-core/net/http.c
@@ -62,6 +62,37 @@ have_ahead (struct grub_file *file)
   return ret;
 }
 
+static int
+is_header (char *ptr, const char* name)
+{
+  grub_size_t length = grub_strlen (name);
+  return grub_strncasecmp (name, ptr, length) == 0 && ptr[length] == ':';
+}
+
+static int
+is_header_value (char *ptr, const char* value)
+{
+  char *ptr_start = ptr;
+  char *ptr_end = ptr + strlen (ptr);
+  grub_size_t value_length = strlen (value);
+
+  while(ptr_start && *ptr_start != ':')
+ptr_start++;
+
+  if (*ptr_start == ':')
+ptr_start++;
+
+  while (grub_isspace (*ptr_start))
+ptr_start++;
+  while (grub_isspace (ptr_end[-1]))
+ptr_end--;
+
+  if (value_length != (grub_size_t)(ptr_end - ptr_start))
+return 0;
+
+  return strncasecmp (value, ptr_start, value_length) == 0;
+}
+
 static grub_err_t
 parse_line (grub_file_t file, http_data_t data, char *ptr, grub_size_t len)
 {
@@ -130,18 +161,16 @@ 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)
-  == 0 && !data->size_recv)
+  if (is_header (ptr, "Content-Length") && !data->size_recv)
 {
-  ptr += sizeof ("Content-Length: ") - 1;
+  ptr += sizeof ("Content-Length:") - 1;
   file->size = grub_strtoull (ptr, (const char **)&ptr, 10);
   data->size_recv = 1;
   return GRUB_ERR_NONE;
 }
-  if (grub_memcmp (ptr, "Transfer-Encoding: chunked",
-  sizeof ("Transfer-Encoding: chunked") - 1) == 0)
+  if (is_header (ptr, "Transfer-Encoding"))
 {
-  data->chunked = 1;
+  data->chunked = is_header_value (ptr, "chunked");
   return GRUB_ERR_NONE;
 }
 
-- 
2.32.0


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel