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

[Heinrich Schuchardt]

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 <adria...@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

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