[Spice-devel] [PATCH] Validate port values

[Marc-André Lureau]

----- Mensaje original -----
> >> +    static int portToInt(const std::string &port)
> >> +    {
> >> +        if (port.empty())
> >> +            return -1;
> >> +
> >> +        char *end;
> >> +        long int conv = strtol(port.c_str(), &end, 10);
> >> +        if (*end != '\0')
> >> +            return -2;
> >> +        if (conv < 0 || conv > 65535)
> >> +            return -2;
> > 
> > This will return an error if the value given is -1.
> 
> It is meant to. If the string contains numeric values in the range of
> 0-65535, it returns them. If there is something else (eg. -1), it
> returns -2 as an error indicator.

My understanding was that -1 could be given to indicate not to use this port.

> >> +    if (port < -1)
> >> +        LOG_ERROR("invalid port: " << m_port);
> > 
> > I would return too in this case, we were given invalid values.
> 
> As I have understood, we can continue with one valid port.

Not if we were given invalid arguments, it's not going to help if it sort-of work, it should rather stop here. We didn't reach a conclusion in previous discussion, but mine is clear.

> > 
> >> +    if (sport < -1)
> >> +        LOG_ERROR("invalid secure port: " << m_secure_port);
> > 
> >> +    if (port < 0 && sport < 0)
> >> +        return;
> > 
> > And in this case, it's useful to LOG_ERROR too.
> 
> You mean, that there are both ports empty, right?

Yes, if both are disabled (== -1) port values, we should also log an error instead of silently not doing anything.