[PATCH v4 10/14] net-lwip: add wget command

Jerome Forissier jerome.forissier at linaro.org
Mon Jun 24 10:49:18 CEST 2024



On 6/24/24 08:21, Jon Humphreys wrote:
> Jerome Forissier <jerome.forissier at linaro.org> writes:
> 
>> Add support for the wget command with NET_LWIP.
>>
>> Based on code initially developed by Maxim U.
>>
>> Signed-off-by: Jerome Forissier <jerome.forissier at linaro.org>
>> Co-developed-by: Maxim Uvarov <muvarov at gmail.com>
>> Cc: Maxim Uvarov <muvarov at gmail.com>
>> Signed-off-by: Jerome Forissier <jerome.forissier at linaro.org>
>> ---
>>  cmd/Kconfig        |   7 ++
>>  cmd/net-lwip.c     |   8 ++
>>  include/net-lwip.h |  18 +++
>>  net-lwip/Makefile  |   1 +
>>  net-lwip/wget.c    | 269 +++++++++++++++++++++++++++++++++++++++++++++
>>  5 files changed, 303 insertions(+)
>>  create mode 100644 net-lwip/wget.c
>>
>> diff --git a/cmd/Kconfig b/cmd/Kconfig
>> index 6ef0b52cd34..d9a86540be6 100644
>> --- a/cmd/Kconfig
>> +++ b/cmd/Kconfig
>> @@ -2117,6 +2117,13 @@ config CMD_TFTPBOOT
>>  	help
>>  	  tftpboot - load file via network using TFTP protocol
>>  
>> +config CMD_WGET
>> +	bool "wget"
>> +	select PROT_TCP_LWIP
>> +	help
>> +	  wget is a simple command to download kernel, or other files,
>> +	  from a http server over TCP.
>> +
>>  endif
>>  
>>  endif
>> diff --git a/cmd/net-lwip.c b/cmd/net-lwip.c
>> index c021da6a674..42f8bd6b259 100644
>> --- a/cmd/net-lwip.c
>> +++ b/cmd/net-lwip.c
>> @@ -35,3 +35,11 @@ U_BOOT_CMD(
>>  	"hostname [envvar]"
>>  );
>>  #endif
>> +
>> +#if defined(CONFIG_CMD_WGET)
>> +U_BOOT_CMD(
>> +	wget,   3,      1,      do_wget,
>> +	"boot image via network using HTTP protocol",
>> +	"[loadAddress] URL"
>> +);
>> +#endif
>> diff --git a/include/net-lwip.h b/include/net-lwip.h
>> index 4d41b0208a3..ddf25e61417 100644
>> --- a/include/net-lwip.h
>> +++ b/include/net-lwip.h
>> @@ -11,8 +11,26 @@ struct netif *net_lwip_new_netif_noip(void);
>>  void net_lwip_remove_netif(struct netif *netif);
>>  struct netif *net_lwip_get_netif(void);
>>  
>> +/**
>> + * wget_with_dns() - runs dns host IP address resulution before wget
>> + *
>> + * @dst_addr:	destination address to download the file
>> + * @uri:	uri string of target file of wget
>> + * Return:	downloaded file size, negative if failed
>> + */
>> +
>> +int wget_with_dns(ulong dst_addr, char *uri);
>> +/**
>> + * wget_validate_uri() - varidate the uri
>> + *
>> + * @uri:	uri string of target file of wget
>> + * Return:	true if uri is valid, false if uri is invalid
>> + */
>> +bool wget_validate_uri(char *uri);
>> +
>>  int do_dhcp(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]);
>>  int do_dns(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]);
>>  int do_ping(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]);
>> +int do_wget(struct cmd_tbl *cmdtp, int flag, int argc, char * const argv[]);
>>  
>>  #endif /* __NET_LWIP_H__ */
>> diff --git a/net-lwip/Makefile b/net-lwip/Makefile
>> index aa247859483..bc011bb0e32 100644
>> --- a/net-lwip/Makefile
>> +++ b/net-lwip/Makefile
>> @@ -10,6 +10,7 @@ obj-$(CONFIG_CMD_DHCP) += dhcp.o
>>  obj-$(CONFIG_CMD_DNS) += dns.o
>>  obj-$(CONFIG_CMD_PING) += ping.o
>>  obj-$(CONFIG_CMD_TFTPBOOT) += tftp.o
>> +obj-$(CONFIG_CMD_WGET) += wget.o
>>  
>>  # Disable this warning as it is triggered by:
>>  # sprintf(buf, index ? "foo%d" : "foo", index)
>> diff --git a/net-lwip/wget.c b/net-lwip/wget.c
>> new file mode 100644
>> index 00000000000..069299bd469
>> --- /dev/null
>> +++ b/net-lwip/wget.c
>> @@ -0,0 +1,269 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/* Copyright (C) 2024 Linaro Ltd. */
>> +
>> +#include <command.h>
>> +#include <console.h>
>> +#include <display_options.h>
>> +#include <image.h>
>> +#include <lwip/apps/http_client.h>
>> +#include <lwip/timeouts.h>
>> +#include <net.h>
>> +#include <time.h>
>> +
>> +#define SERVER_NAME_SIZE 200
>> +#define HTTP_PORT_DEFAULT 80
>> +#define PROGRESS_PRINT_STEP_BYTES (100 * 1024)
>> +
>> +enum done_state {
>> +        NOT_DONE = 0,
>> +        SUCCESS = 1,
>> +        FAILURE = 2
>> +};
>> +
>> +struct wget_ctx {
>> +	ulong daddr;
>> +	ulong saved_daddr;
>> +	ulong size;
>> +	ulong prevsize;
>> +	ulong start_time;
>> +	enum done_state done;
>> +};
>> +
>> +static int parse_url(char *url, char *host, u16 *port, char **path)
>> +{
>> +	char *p, *pp;
>> +	long lport;
>> +
>> +	p = strstr(url, "http://");
>> +	if (!p)
>> +		return -EINVAL;
>> +
>> +	p += strlen("http://");
>> +
>> +	/* Parse hostname */
>> +	pp = strchr(p, ':');
>> +	if (!pp)
>> +		pp = strchr(p, '/');
>> +	if (!pp)
>> +		return -EINVAL;
>> +
>> +	if (p + SERVER_NAME_SIZE <= pp)
>> +		return -EINVAL;
>> +
>> +	memcpy(host, p, pp - p);
>> +	host[pp - p + 1] = '\0';
>> +
>> +	if (*pp == ':') {
>> +		/* Parse port number */
>> +		p = pp + 1;
>> +		lport = simple_strtol(p, &pp, 10);
>> +		if (pp && *pp != '/')
>> +			return -EINVAL;
>> +		if (lport > 65535)
>> +			return -EINVAL;
>> +		*port = (u16)lport;
>> +	} else {
>> +		*port = HTTP_PORT_DEFAULT;
>> +	}
>> +	if (*pp != '/')
>> +		return -EINVAL;
>> +	*path = pp;
>> +
>> +	return 0;
>> +}
>> +
>> +static err_t httpc_recv_cb(void *arg, struct altcp_pcb *pcb, struct pbuf *pbuf,
>> +			   err_t err)
>> +{
>> +	struct wget_ctx *ctx = arg;
>> +	struct pbuf *buf;
>> +
>> +	if (!pbuf)
>> +		return ERR_BUF;
>> +
>> +	for (buf = pbuf; buf; buf = buf->next) {
>> +		memcpy((void *)ctx->daddr, buf->payload, buf->len);
>> +		ctx->daddr += buf->len;
>> +		ctx->size += buf->len;
>> +		if (ctx->size - ctx->prevsize > PROGRESS_PRINT_STEP_BYTES) {
>> +			printf("#");
>> +			ctx->prevsize = ctx->size;
>> +		}
>> +	}
>> +
>> +	altcp_recved(pcb, pbuf->tot_len);
>> +	pbuf_free(pbuf);
>> +	return ERR_OK;
>> +}
>> +
>> +static void httpc_result_cb(void *arg, httpc_result_t httpc_result,
>> +			    u32_t rx_content_len, u32_t srv_res, err_t err)
>> +{
>> +	struct wget_ctx *ctx = arg;
>> +	ulong elapsed;
>> +
>> +	if (httpc_result != HTTPC_RESULT_OK) {
>> +		log_err("\nHTTP client error %d\n", httpc_result);
>> +		ctx->done = FAILURE;
>> +		return;
>> +	}
>> +
>> +	elapsed = get_timer(ctx->start_time);
>> +	if (rx_content_len > PROGRESS_PRINT_STEP_BYTES)
>> +		printf("\n");
>> +        printf("%u bytes transferred in %lu ms (", rx_content_len,
>> +	       get_timer(ctx->start_time));
>> +        print_size(rx_content_len / elapsed * 1000, "/s)\n");
>> +
>> +	if (env_set_hex("filesize", rx_content_len) ||
>> +	    env_set_hex("fileaddr", ctx->saved_daddr)) {
>> +		log_err("Could not set filesize or fileaddr\n");
>> +		ctx->done = FAILURE;
>> +		return;
>> +	}
>> +
>> +	ctx->done = SUCCESS;
>> +}
>> +
>> +int wget_with_dns(ulong dst_addr, char *uri)
>> +{
>> +	char server_name[SERVER_NAME_SIZE];
>> +	httpc_connection_t conn;
>> +	httpc_state_t *state;
>> +	struct netif *netif;
>> +	struct wget_ctx ctx;
>> +	char *path;
>> +	u16 port;
>> +
>> +	ctx.daddr = dst_addr;
>> +	ctx.saved_daddr = dst_addr;
>> +	ctx.done = NOT_DONE;
>> +	ctx.size = 0;
>> +	ctx.prevsize = 0;
>> +
>> +	if (parse_url(uri, server_name, &port, &path))
>> +		return CMD_RET_USAGE;
>> +
>> +	netif = net_lwip_new_netif();
>> +	if (!netif)
>> +		return -1;
>> +
>> +	memset(&conn, 0, sizeof(conn));
>> +	conn.result_fn = httpc_result_cb;
>> +	ctx.start_time = get_timer(0);
>> +	if (httpc_get_file_dns(server_name, port, path, &conn, httpc_recv_cb,
>> +			       &ctx, &state)) {
>> +		net_lwip_remove_netif(netif);
>> +		return CMD_RET_FAILURE;
>> +	}
>> +
>> +	while (!ctx.done) {
>> +		eth_rx();
>> +		sys_check_timeouts();
>> +		if (ctrlc())
>> +			break;
>> +	}
>> +
>> +	net_lwip_remove_netif(netif);
>> +
>> +	if (ctx.done == SUCCESS)
>> +		return 0;
>> +
>> +	return -1;
>> +}
>> +
>> +int do_wget(struct cmd_tbl *cmdtp, int flag, int argc, char * const argv[])
>> +{
>> +	char *end;
>> +	char *url;
>> +	ulong dst_addr;
>> +
>> +	if (argc < 2 || argc > 3)
>> +		return CMD_RET_USAGE;
>> +
>> +	dst_addr = hextoul(argv[1], &end);
>> +        if (end == (argv[1] + strlen(argv[1]))) {
>> +		if (argc < 3)
>> +			return CMD_RET_USAGE;
>> +		url = argv[2];
>> +	} else {
>> +		dst_addr = image_load_addr;
>> +		url = argv[1];
>> +	}
>> +
>> +	if (wget_with_dns(dst_addr, url))
>> +		return CMD_RET_FAILURE;
>> +
>> +	return CMD_RET_SUCCESS;
>> +}
>> +
>> +/**
>> + * wget_validate_uri() - validate the uri for wget
>> + *
>> + * @uri:	uri string
>> + *
>> + * This function follows the current U-Boot wget implementation.
>> + * scheme: only "http:" is supported
>> + * authority:
>> + *   - user information: not supported
>> + *   - host: supported
>> + *   - port: not supported(always use the default port)
>> + *
>> + * Uri is expected to be correctly percent encoded.
>> + * This is the minimum check, control codes(0x1-0x19, 0x7F, except '\0')
>> + * and space character(0x20) are not allowed.
>> + *
>> + * TODO: stricter uri conformance check
>> + *
>> + * Return:	true on success, false on failure
>> + */
>> +bool wget_validate_uri(char *uri)
>> +{
>> +	char c;
>> +	bool ret = true;
>> +	char *str_copy, *s, *authority;
>> +
>> +	for (c = 0x1; c < 0x21; c++) {
>> +		if (strchr(uri, c)) {
>> +			log_err("invalid character is used\n");
>> +			return false;
>> +		}
>> +	}
>> +	if (strchr(uri, 0x7f)) {
>> +		log_err("invalid character is used\n");
>> +		return false;
>> +	}
>> +
>> +	if (strncmp(uri, "http://", 7)) {
>> +		log_err("only http:// is supported\n");
>> +		return false;
>> +	}
>> +	str_copy = strdup(uri);
>> +	if (!str_copy)
>> +		return false;
>> +
>> +	s = str_copy + strlen("http://");
>> +	authority = strsep(&s, "/");
>> +	if (!s) {
>> +		log_err("invalid uri, no file path\n");
>> +		ret = false;
>> +		goto out;
>> +	}
>> +	s = strchr(authority, '@');
>> +	if (s) {
>> +		log_err("user information is not supported\n");
>> +		ret = false;
>> +		goto out;
>> +	}
>> +	s = strchr(authority, ':');
>> +	if (s) {
>> +		log_err("user defined port is not supported\n");
>> +		ret = false;
>> +		goto out;
>> +	}
>> +
>> +out:
>> +	free(str_copy);
>> +
>> +	return ret;
>> +}
>> -- 
>> 2.40.1

Hi Jon,

> Hi Jerome, I am trying out the lwIP stack on TI boards.
> 
> For wget, I noticed a bug and a user experience improvement. The bug is an
> off-by-one array access when parsing the url.

Indeed, someone reported this bug to me off-list and it's already fixed in
my WIP branch.

> The improvement emits a
> message if the url isn't an http://, since that is all we are supporting,
> and without the message, the wget command silently fails, which is
> confusing to the user.

I agree.

> I will post the 2 patches as a reply to this email. LMK if you want to use
> another method to collaborate. I can already see that efi http boot support
> will not work. wget_validate_uri() looks to be copied vertabim from the
> current implementation, and IMO we should completely remove it.

It is indeed copied. It is needed to avoid an unresolved symbol but I
understand more work may be needed for EFI HTTP boot support (not tested by
me).

If you are willing to help with this then you are welcome to post patches as
replies and I will integrate them in the series.

Thanks,
-- 
Jerome


More information about the U-Boot mailing list