Brad Nicholes wrote:
Also, this exposes a bug, I think, in 2.0/2.1. parsed_uri.port is valid iff parsed_uri.port_str != NULL. Currently, we are testing just to see if parsed_uri.port itself isn't 0.
What you are saying then is that without testing parsed_uri.port_str, there is no way of knowing if port 0 could actually be a valid port or if parsed_uri.port contains garbage that just happens to look like a port. The former depends on whether port 0 can actually be a valid port and the latter depends on how the parsed_uri structure is initialized.
From what I can see (prelim look into the URI parsing) it ispossible for port to be garbage if port_str == NULL. Exactly what kind of garbage is undefined... it could be 0 for some and *real* garbage for others... The check for port!=0 doesn't suffice to ensure that the value used for port is *valid*. Note that a garbage value of port that is non-zero would be used as valid in 2.x right now...
any hints on how to reproduce this problem?
is there some bad or unhelpful behavior in apr_uri_parse() that should be changed? (i.e., don't let port be non-zero if port_str is NULL)
it looks to me that apr_uri_parse() can set port_str to "" in some cases where there is no explicit port specified and the port integer is set to the default port of the scheme:
uptr->port_str = apr_pstrmemdup(p, s, uri - s);
if (uri != s) {
port = strtol(uptr->port_str, &endstr, 10);
uptr->port = port;
if (*endstr == '\0') {
goto deal_with_path;
}
/* Invalid characters after ':' found */
return APR_EGENERAL;
}
uptr->port = apr_uri_port_of_scheme(uptr->scheme);
goto deal_with_path;in this case, I don't see that checking port_str is going to do the right thing...
Maybe I'm barking up the wrong tree, but I get the feeling that maybe the real problem is that apr_uri_parse() doesn't return with port and port_str set to consistent values, and that this must be fixed anyway, and once it is fixed then it doesn't matter whether or not other code checks port or port_str.
