On 6/25/24 05:20, Jon Humphreys wrote:
> Jerome Forissier <jerome.foriss...@linaro.org> writes:
>
>> On 6/24/24 08:21, Jon Humphreys wrote:
>>> Jerome Forissier <jerome.foriss...@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.foriss...@linaro.org>
>>>> Co-developed-by: Maxim Uvarov <muva...@gmail.com>
>>>> Cc: Maxim Uvarov <muva...@gmail.com>
>>>> Signed-off-by: Jerome Forissier <jerome.foriss...@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
>>>>
<snip>
>>>> +/**
>>>> + * 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
>
> Jerome, here is a patch to allow a port specifier in wget's uri, which lwIP
> supports.
>
> thanks
> Jon
>
> Subject: [PATCH] net-lwip: lwIP wget supports user defined port in the uri, so
> allow it.
>
> Signed-off-by: Jonathan Humphreys <j-humphr...@ti.com>
> ---
> net-lwip/wget.c | 6 ------
> 1 file changed, 6 deletions(-)
>
> diff --git a/net-lwip/wget.c b/net-lwip/wget.c
> index 6b9c953ef51..d3fdd44a29e 100644
> --- a/net-lwip/wget.c
> +++ b/net-lwip/wget.c
> @@ -257,12 +257,6 @@ bool wget_validate_uri(char *uri)
> 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);
Patch added to v5, thanks!
--
Jerome
