Re: [PATCH] net: Allow use of non-standard TCP/IP ports for HTTP protocol.

[Daniel Kiper]

On Tue, Oct 26, 2021 at 04:14:12PM -0700, Stephen Balousek wrote:
> Hi Daniel,
>
> Thank you for another detailed response. Please let me know how that section

You are welcome!

> related to IPv6 should be commented, and I will submit an updated patch.
>
> - Steve
>
> On 10/26/2021 8:52 AM, Daniel Kiper wrote:
> > 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?
>
> IPv6. The idea is to capture the port number '8080' from '[::1]:8080' but
> not '1' from '[::1]'. I can add a comment here, since I agree the intent is
> not clear.

Ah, I totally forgot about IPv6. Please add a comment here. Additionally,
I think it is worth adding information about IPv6 format to the docs/grub.texi,
e.g., as an example of IPv4, IPv6 and normal (DNS) name formatting.

Daniel