[vlc-devel] [PATCH 4/4] text/url: fix port handling in vlc_UrlParse

[Rémi Denis-Courmont]

Le perjantaina 28. lokakuuta 2016, 10.49.47 EEST Filip Roséen a écrit :
> Differences compared to the previous implementation:
> 
>  - accept URLs with empty port-specification (RFC3986, 3.2.3).
> 
>  - reject port-specification with leading sign (RFC3986 only
>    allows *DIGIT).
> 
>  - do not error on ULONG_MAX if strtoul did not detect an overflow
>    error (if ULONG_MAX == UINT_MAX on the target platform, this change
>    is important).
> 
> refs #17555
> ---
>  src/text/url.c | 19 ++++++++++---------
>  1 file changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/src/text/url.c b/src/text/url.c
> index 90a51cb..c183bf1 100644
> --- a/src/text/url.c
> +++ b/src/text/url.c
> @@ -525,18 +525,19 @@ int vlc_UrlParse(vlc_url_t *restrict url, const char
> *str) }
> 
>          /* Port number */
> -        if (next != NULL)
> +        if (next != NULL && *next)
>          {
> -            char *end;
> -            unsigned long u = strtoul(next, &end, 10);
> +            char* end;
> +            unsigned long port = strtoul(next, &end, 10);
> 
> -            url->i_port = u;
> -            if (end == next || *end != '\0' || u == ULONG_MAX)
> -                ret = -1;
> -#if (ULONG_MAX > UINT_MAX)
> -            if (u > UINT_MAX)
> +            if (strchr("0123456789", *next) == NULL || *end ||

(((unsigned char)*next) - '0' < 10) is probably much faster/smaller.

> +                port > UINT_MAX || (port == ULONG_MAX && errno == ERANGE))

I think that this will cause warnings on 32-bits architectures because
port > UINT_MAX is impossible.

And you cannot rely on errno because it could be the result of an earlier 
function call failing.

> +            {
> +                errno = EINVAL;
>                  ret = -1;
> -#endif
> +            }
> +
> +            url->i_port = port;
>          }
> 
>          if (url->psz_path != NULL)

-- 
Rémi Denis-Courmont
Nonsponsored VLC developer
http://www.remlab.net/CV.pdf