* [PATCH] net: Allow use of non-standard TCP/IP ports for HTTP protocol.
@ 2021-09-05 22:57 Stephen Balousek
2021-09-28 14:27 ` Daniel Kiper
0 siblings, 1 reply; 8+ messages in thread
From: Stephen Balousek @ 2021-09-05 22:57 UTC (permalink / raw)
To: grub-devel; +Cc: Stephen Balousek
Allow the use of HTTP servers listening on ports other 80. This is done
with an extension to the http notation of either:
(http[,server[,port]])
- or -
(http[,server[:port]])
Signed-off-by: Stephen Balousek <sbalousek@wickedloop.com>
---
grub-core/net/http.c | 43 +++++++++++++++++++++++++++++++++++++++----
1 file changed, 39 insertions(+), 4 deletions(-)
diff --git a/grub-core/net/http.c b/grub-core/net/http.c
index b616cf40b..87e496e7f 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;
+ long port_number;
nb = grub_netbuff_alloc (GRUB_NET_TCP_RESERVE_SIZE
+ sizeof ("GET ") - 1
@@ -390,10 +393,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,
- http_err, NULL,
- file);
+ port_string = grub_strrchr(file->device->net->server, ',');
+ if (!port_string)
+ {
+ port_string = grub_strrchr(file->device->net->server, ':');
+ if (grub_strchr(port_string + 1, ']'))
+ {
+ port_string = 0;
+ }
+ }
+ if (port_string)
+ {
+ port_number = grub_strtol(port_string + 1, 0, 10);
+ if (port_number <= 0 || port_number > 65535)
+ {
+ return grub_error (GRUB_ERR_FILE_NOT_FOUND, N_("invalid port number `%s'"), port_string + 1);
+ }
+
+ server_name = grub_strdup(file->device->net->server);
+ 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.33.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] net: Allow use of non-standard TCP/IP ports for HTTP protocol.
2021-09-05 22:57 [PATCH] net: Allow use of non-standard TCP/IP ports for HTTP protocol Stephen Balousek
@ 2021-09-28 14:27 ` Daniel Kiper
2021-10-10 22:15 ` Stephen Balousek
0 siblings, 1 reply; 8+ messages in thread
From: Daniel Kiper @ 2021-09-28 14:27 UTC (permalink / raw)
To: Stephen Balousek; +Cc: grub-devel
On Sun, Sep 05, 2021 at 03:57:42PM -0700, Stephen Balousek wrote:
> Allow the use of HTTP servers listening on ports other 80. This is done
> with an extension to the http notation of either:
>
> (http[,server[,port]])
>
> - or -
>
> (http[,server[:port]])
>
> Signed-off-by: Stephen Balousek <sbalousek@wickedloop.com>
> ---
> grub-core/net/http.c | 43 +++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 39 insertions(+), 4 deletions(-)
>
> diff --git a/grub-core/net/http.c b/grub-core/net/http.c
> index b616cf40b..87e496e7f 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;
> + long port_number;
s/long/unsigned long/g
> nb = grub_netbuff_alloc (GRUB_NET_TCP_RESERVE_SIZE
> + sizeof ("GET ") - 1
> @@ -390,10 +393,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,
> - http_err, NULL,
> - file);
> + port_string = grub_strrchr(file->device->net->server, ',');
Missing space between grub_strrchr and "(". Same thing should be fixed
in the functions calls below.
> + if (!port_string)
if (port_string == NULL)
> + {
> + port_string = grub_strrchr(file->device->net->server, ':');
> + if (grub_strchr(port_string + 1, ']'))
You blindly assume port_string is not NULL here.
> + {
> + port_string = 0;
port_string = NULL;
> + }
You do not need these curly brackets.
> + }
> + if (port_string)
if (port_string != NULL)
> + {
> + port_number = grub_strtol(port_string + 1, 0, 10);
Please use grub_strtoul() instead of grub_strtol() and handle all errors
properly. The strtoul man page is your friend.
> + if (port_number <= 0 || port_number > 65535)
> + {
> + return grub_error (GRUB_ERR_FILE_NOT_FOUND, N_("invalid port number `%s'"), port_string + 1);
> + }
Please drop these curly brackets.
> + server_name = grub_strdup(file->device->net->server);
grub_strdup() may return NULL...
> + server_name[port_string - file->device->net->server] = 0;
s/0/'\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);
> + }
Please drop these curly brackets.
Daniel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] net: Allow use of non-standard TCP/IP ports for HTTP protocol.
2021-09-28 14:27 ` Daniel Kiper
@ 2021-10-10 22:15 ` Stephen Balousek
2021-10-26 15:52 ` Daniel Kiper
0 siblings, 1 reply; 8+ messages in thread
From: Stephen Balousek @ 2021-10-10 22:15 UTC (permalink / raw)
To: Daniel Kiper; +Cc: grub-devel
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.
I reworked the patch with your suggested changes, and I also included an
attempt at some changes for the documentation.
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.
Thanks again,
- Steve
On 9/28/21 07:27, Daniel Kiper wrote:
> On Sun, Sep 05, 2021 at 03:57:42PM -0700, Stephen Balousek wrote:
>> Allow the use of HTTP servers listening on ports other 80. This is done
>> with an extension to the http notation of either:
>>
>> (http[,server[,port]])
>>
>> - or -
>>
>> (http[,server[:port]])
>>
>> Signed-off-by: Stephen Balousek <sbalousek@wickedloop.com>
>> ---
>> grub-core/net/http.c | 43 +++++++++++++++++++++++++++++++++++++++----
>> 1 file changed, 39 insertions(+), 4 deletions(-)
>>
>> diff --git a/grub-core/net/http.c b/grub-core/net/http.c
>> index b616cf40b..87e496e7f 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;
>> + long port_number;
> s/long/unsigned long/g
>
>> nb = grub_netbuff_alloc (GRUB_NET_TCP_RESERVE_SIZE
>> + sizeof ("GET ") - 1
>> @@ -390,10 +393,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,
>> - http_err, NULL,
>> - file);
>> + port_string = grub_strrchr(file->device->net->server, ',');
> Missing space between grub_strrchr and "(". Same thing should be fixed
> in the functions calls below.
>
>> + if (!port_string)
> if (port_string == NULL)
>
>> + {
>> + port_string = grub_strrchr(file->device->net->server, ':');
>> + if (grub_strchr(port_string + 1, ']'))
> You blindly assume port_string is not NULL here.
>
>> + {
>> + port_string = 0;
> port_string = NULL;
>
>> + }
> You do not need these curly brackets.
>
>> + }
>> + if (port_string)
> if (port_string != NULL)
>
>> + {
>> + port_number = grub_strtol(port_string + 1, 0, 10);
> Please use grub_strtoul() instead of grub_strtol() and handle all errors
> properly. The strtoul man page is your friend.
>
>> + if (port_number <= 0 || port_number > 65535)
>> + {
>> + return grub_error (GRUB_ERR_FILE_NOT_FOUND, N_("invalid port number `%s'"), port_string + 1);
>> + }
> Please drop these curly brackets.
>
>> + server_name = grub_strdup(file->device->net->server);
> grub_strdup() may return NULL...
>
>> + server_name[port_string - file->device->net->server] = 0;
> s/0/'\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);
>> + }
> Please drop these curly brackets.
>
> Daniel
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, ']') !=
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)
+ 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.33.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] net: Allow use of non-standard TCP/IP ports for HTTP protocol.
2021-10-10 22:15 ` Stephen Balousek
@ 2021-10-26 15:52 ` Daniel Kiper
2021-10-26 23:14 ` Stephen Balousek
0 siblings, 1 reply; 8+ messages in thread
From: Daniel Kiper @ 2021-10-26 15:52 UTC (permalink / raw)
To: Stephen Balousek; +Cc: grub-devel
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] net: Allow use of non-standard TCP/IP ports for HTTP protocol.
2021-10-26 15:52 ` Daniel Kiper
@ 2021-10-26 23:14 ` Stephen Balousek
2021-11-03 15:28 ` Daniel Kiper
0 siblings, 1 reply; 8+ messages in thread
From: Stephen Balousek @ 2021-10-26 23:14 UTC (permalink / raw)
To: Daniel Kiper; +Cc: grub-devel
Hi Daniel,
Thank you for another detailed response. Please let me know how that
section 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.
>
>> 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.
Right. I don't know what I was thinking 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.
Thank you.
>
> Thanks,
>
> Daniel
--
- Steve
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] net: Allow use of non-standard TCP/IP ports for HTTP protocol.
2021-10-26 23:14 ` Stephen Balousek
@ 2021-11-03 15:28 ` Daniel Kiper
2022-01-09 1:22 ` [PATCH v2] http: Allow use of non-standard TCP/IP ports Stephen Balousek
0 siblings, 1 reply; 8+ messages in thread
From: Daniel Kiper @ 2021-11-03 15:28 UTC (permalink / raw)
To: Stephen Balousek; +Cc: grub-devel
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2] http: Allow use of non-standard TCP/IP ports
2021-11-03 15:28 ` Daniel Kiper
@ 2022-01-09 1:22 ` Stephen Balousek
2022-01-14 14:55 ` Daniel Kiper
0 siblings, 1 reply; 8+ messages in thread
From: Stephen Balousek @ 2022-01-09 1:22 UTC (permalink / raw)
To: grub-devel; +Cc: Daniel Kiper, 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 <sbalousek@wickedloop.com>
---
Hi Daniel,
Happy New Year!
I apologize for the long delay. I really wanted to test the changes
for an HTTP server over IPv6, but not knowing much about setting up an
IPv6 network made for a pretty steep learning curve! Anyway, please
find attached a new version of the patch for allowing the use of
non-standard HTTP server ports.
I added some comments to the code for parsing the port number from an
IPv6 address, and added relevant examples to the INFO document.
Also, you mentioned:
I think it should be "port_number == GRUB_ULONG_MAX" instead of
"port_number == 0" here.
I took a closer look at grub_strtoul() and that function actually
returns zero (0) when a non-numeric value is encounteded in the
string. So, functionally, I think these changes are correct.
Thank you for your patience and all the help!
- Steve
docs/grub.texi | 33 +++++++++++++++++++++++++++++++++
grub-core/net/http.c | 39 +++++++++++++++++++++++++++++++++++++--
2 files changed, 70 insertions(+), 2 deletions(-)
diff --git a/docs/grub.texi b/docs/grub.texi
index f8b4b3b21..a95d86f95 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:dead:beef::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..f457cd010 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,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, 0, 10);
+ if (port_number == 0 && grub_errno == GRUB_ERR_BAD_NUMBER)
+ 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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2] http: Allow use of non-standard TCP/IP ports
2022-01-09 1:22 ` [PATCH v2] http: Allow use of non-standard TCP/IP ports Stephen Balousek
@ 2022-01-14 14:55 ` Daniel Kiper
0 siblings, 0 replies; 8+ messages in thread
From: Daniel Kiper @ 2022-01-14 14:55 UTC (permalink / raw)
To: Stephen Balousek; +Cc: grub-devel
On Sat, Jan 08, 2022 at 06:22:47PM -0700, Stephen Balousek wrote:
> 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>
> ---
>
> Hi Daniel,
Hi Stephen,
> Happy New Year!
Happy New Year for you too!
> I apologize for the long delay. I really wanted to test the changes
No worries. I know how it works...
> for an HTTP server over IPv6, but not knowing much about setting up an
> IPv6 network made for a pretty steep learning curve! Anyway, please
> find attached a new version of the patch for allowing the use of
> non-standard HTTP server ports.
>
> I added some comments to the code for parsing the port number from an
> IPv6 address, and added relevant examples to the INFO document.
>
> Also, you mentioned:
>
> I think it should be "port_number == GRUB_ULONG_MAX" instead of
> "port_number == 0" here.
>
> I took a closer look at grub_strtoul() and that function actually
> returns zero (0) when a non-numeric value is encounteded in the
> string. So, functionally, I think these changes are correct.
Yeah, only if there are no digits in the string... Please look below...
> Thank you for your patience and all the help!
> - Steve
>
>
> docs/grub.texi | 33 +++++++++++++++++++++++++++++++++
> grub-core/net/http.c | 39 +++++++++++++++++++++++++++++++++++++--
> 2 files changed, 70 insertions(+), 2 deletions(-)
>
> diff --git a/docs/grub.texi b/docs/grub.texi
> index f8b4b3b21..a95d86f95 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:dead:beef::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..f457cd010 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,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, 0, 10);
> + if (port_number == 0 && grub_errno == GRUB_ERR_BAD_NUMBER)
> + return grub_error (GRUB_ERR_BAD_NUMBER, N_("non-numeric or invalid port number `%s'"), port_string + 1);
This check is not reliable. If you want to rely on grub_errno you should
reset grub_errno to GRUB_ERR_NONE before grub_strtoul() call. However,
this still does not allow you to detect partially invalid strings. So,
I think you should do this:
port_number = grub_strtoul (port_string + 1, &endptr, 10);
if (*(port_string + 1) == '\0' || endptr != '\0')
return grub_error (GRUB_ERR_BAD_NUMBER, N_("non-numeric or invalid port number `%s'"), port_string + 1);
Otherwise the patch LGTM.
Daniel
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-01-14 14:55 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-05 22:57 [PATCH] net: Allow use of non-standard TCP/IP ports for HTTP protocol Stephen Balousek
2021-09-28 14:27 ` Daniel Kiper
2021-10-10 22:15 ` Stephen Balousek
2021-10-26 15:52 ` Daniel Kiper
2021-10-26 23:14 ` Stephen Balousek
2021-11-03 15:28 ` Daniel Kiper
2022-01-09 1:22 ` [PATCH v2] http: Allow use of non-standard TCP/IP ports Stephen Balousek
2022-01-14 14:55 ` Daniel Kiper
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.