[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] net: Allow use of non-standard TCP/IP ports for HTTP protoco
From: |
Daniel Kiper |
Subject: |
Re: [PATCH] net: Allow use of non-standard TCP/IP ports for HTTP protocol. |
Date: |
Tue, 26 Oct 2021 17:52:27 +0200 |
User-agent: |
NeoMutt/20170113 (1.7.2) |
Hi Stephen,
On Sun, Oct 10, 2021 at 03:15:34PM -0700, Stephen Balousek wrote:
> Hi Daniel,
>
> Thank you for the review. This is my first submission to the project, and I
> apologize for my lack of familiarity with the conventions and guidelines.
No worries!
> I reworked the patch with your suggested changes, and I also included an
> attempt at some changes for the documentation.
Thanks!
> I am not completely sure how to submit a revised patch to the mailing list
> and still maintain the email thread, so I am appending the new patch to this
> message.
Next time please create a new thread and mark the patch as v2, v3, etc.
respectively.
You can use "git format-patch" and "git send-email" to do it.
[...]
> From 3efc9a7f00a3e8f96609ee95262e87e61d42ce44 Mon Sep 17 00:00:00 2001
> From: Stephen Balousek <sbalousek@wickedloop.com>
> Date: Sun, 10 Oct 2021 15:12:20 -0700
> Subject: [PATCH] http: Allow use of non-standard TCP/IP ports
> To: grub-devel@gnu.org
>
> 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 <sbalousek@wickedloop.com>
> ---
> docs/grub.texi | 12 ++++++++++++
> grub-core/net/http.c | 37 +++++++++++++++++++++++++++++++++++--
> 2 files changed, 47 insertions(+), 2 deletions(-)
>
> diff --git a/docs/grub.texi b/docs/grub.texi
> index 99d0a0149..dbafe80d2 100644
> --- a/docs/grub.texi
> +++ b/docs/grub.texi
> @@ -3011,6 +3011,18 @@ 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{:}:
> +
> +@itemize @bullet
> +@item
> +@code{(http,@var{server},@var{port})}
> +
> +@item
> +@code{(http,@var{server}:@var{port})}
> +@end itemize
> +
> 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..eb07e333a 100644
> --- a/grub-core/net/http.c
> +++ b/grub-core/net/http.c
> @@ -312,6 +312,9 @@ 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;
> + unsigned long port_number;
>
> nb = grub_netbuff_alloc (GRUB_NET_TCP_RESERVE_SIZE
> + sizeof ("GET ") - 1
> @@ -390,10 +393,40 @@ 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)
> + {
> + port_string = grub_strrchr (file->device->net->server, ':');
> + if (port_string != NULL && grub_strchr (port_string + 1, ']') !=
I am not sure why you are looking for ']' here. Could you enlighten me?
> NULL)
> + port_string = NULL;
> + }
> + if (port_string != NULL)
> + {
> + port_number = grub_strtoul (port_string + 1, 0, 10);
> + if (port_number == 0 && grub_errno == GRUB_ERR_BAD_NUMBER)
I think it should be "port_number == GRUB_ULONG_MAX" instead of
"port_number == 0" here.
> + 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;
> + }
Otherwise patch looks much better.
Thanks,
Daniel