[PATCH v4 01/14] net: wget: let wget_with_dns work with dns disabled

[Adriano Córdova]

El mar, 19 nov 2024 a las 6:27, Heinrich Schuchardt (<
heinrich.schuchardt at canonical.com>) escribió:

> On 18.11.24 22:08, Adriano Cordova wrote:
> > This was marked as TODO in the code:
> > -Enable use of wget_with_dns even if CMD_DNS is disabled if
> > the given uri has the ip address for the http server.
> > -Check for port in the uri when transforming to legacy wget
> > syntax inside wget_with_dns.
> > -Move the check for CMD_DNS inside wget_with_dns.
> >
> > Signed-off-by: Adriano Cordova <adrianox at gmail.com>
> > ---
>
> Changes in v4 are not described:
>
> * drop console output
> * avoid checking non-zero via '!= 0'.
>
> >
> > v3:
> >   Removed console output inside efi routine.
> >
> > (no changes since v2)
> >   net/wget.c | 36 +++++++++++++++++++++---------------
> >   1 file changed, 21 insertions(+), 15 deletions(-)
> >
> > diff --git a/net/wget.c b/net/wget.c
> > index 3bc2522cde..1166aa5aa6 100644
> > --- a/net/wget.c
> > +++ b/net/wget.c
> > @@ -530,12 +530,10 @@ void wget_start(void)
> >       wget_send(TCP_SYN, 0, 0, 0);
> >   }
> >
> > -#if (IS_ENABLED(CONFIG_CMD_DNS))
> >   int wget_with_dns(ulong dst_addr, char *uri)
> >   {
> >       int ret;
> > -     char *s, *host_name, *file_name, *str_copy;
> > -
> > +     char *s, *host_name, *file_name, *str_copy, *port;
>
> nits:
> Missing empty line.
>
> >       /*
> >        * Download file using wget.
> >        *
> > @@ -550,24 +548,33 @@ int wget_with_dns(ulong dst_addr, char *uri)
> >       s = str_copy + strlen("http://");
> >       host_name = strsep(&s, "/");
> >       if (!s) {
> > -             log_err("Error: invalied uri, no file path\n");
> >               ret = -EINVAL;
> >               goto out;
> >       }
> >       file_name = s;
> >
> > -     /* TODO: If the given uri has ip address for the http server, skip
> dns */
> > -     net_dns_resolve = host_name;
> > -     net_dns_env_var = "httpserverip";
> > -     if (net_loop(DNS) < 0) {
> > -             log_err("Error: dns lookup of %s failed, check setup\n",
> net_dns_resolve);
> > -             ret = -EINVAL;
> > -             goto out;
> > -     }
> > -     s = env_get("httpserverip");
> > -     if (!s) {
> > +     port = host_name;
> > +     host_name = strsep(&port, ":");
>
> RFC 1738 has:
>
> login          = [ user [ ":" password ] "@" ] hostport
> hostport       = host [ ":" port ]
>
> So here variable port may point to the password. And host_name to the
> user name. We might check for '@' and bail out with an error code if it
> occurs.
>
> You are determining the host port as a string, but I cannot see where
> you are ever using it.
>
> In wget_send_stored() I see this line:
>
> server_port = env_get_ulong("httpdstp", 10, SERVER_PORT) & 0xffff;
>
> Maybe make server_port a static variable and set it here if variable
> port does not contain a number.
>
> Best regards
>
> Heinrich
>

The current documentation and implementation of wget is not compliant with
RFC 1738 and only supports ip:path. I will erase the port and leave the
wget parsing
as it is documented (ip:path). I think reworking the legacy wget parsing to
be RFC 1738
compliant can be done in a separate patch.

>
> > +
> > +     if (string_to_ip(host_name).s_addr) {
> > +             s = host_name;
> > +     } else {
> > +#if IS_ENABLED(CONFIG_CMD_DNS)
> > +             net_dns_resolve = host_name;
> > +             net_dns_env_var = "httpserverip";
> > +             if (net_loop(DNS) < 0) {
> > +                     ret = -EINVAL;
> > +                     goto out;
> > +             }
> > +             s = env_get("httpserverip");
> > +             if (!s) {
> > +                     ret = -EINVAL;
> > +                     goto out;
> > +             }
> > +#else
> >               ret = -EINVAL;
> >               goto out;
> > +#endif
> >       }
> >
> >       strlcpy(net_boot_file_name, s, sizeof(net_boot_file_name));
> > @@ -581,7 +588,6 @@ out:
> >
> >       return ret < 0 ? ret : 0;
> >   }
> > -#endif
> >
> >   /**
> >    * wget_validate_uri() - validate the uri for wget
>
>