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

[Jerome Forissier]

On 11/18/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

Then I believe the function should be renamed. _with_dns is misleading.

> 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 <adria...@gmail.com>
> ---
> 
> 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;
>       /*
>        * 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, ":");

You should do something with the port (or reject URLs with a non-default port).

> +
> +     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

Thanks,
-- 
Jerome