httpd-bugs mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
Subject DO NOT REPLY [Bug 44806] Set the IP address+port used for backend proxy requests.
Date Mon, 02 Jun 2008 21:37:57 GMT

--- Comment #10 from D. Stussy <>  2008-06-02 14:37:56
PST ---
RE: Comment #9 version:

1)  "Range" value:  Checking for (r < 1) needs to be done(*).  It's possible
that an admin could specify "ip:port+-range" and get a negative value.  Noting
that a range of zero is "not useful," perhaps we need to set a minimum value
for the range parameter?  In current parsing, not specifying a range at all
does default it to a single port.  The question is:  What is a suitable
default?  I'm thinking that it could be one of the following:
 - A fixed number, perhaps 8.
 - Some value derived from the "MaxClients" or "Min/MaxSpareServers" directive.
 - Some value derived from the Proxy Balancer code.
We need only check the lower value for "r" as the "port+r" code will check the
upper value.  For the "port+r" comparison, do we have a "MAX_PORT" definition
available (from an included file) so we don't have to hard-code the value?

* - Checking "r" against the minimum also will take care of cases where the
admin specified non-digit values for the range (which atoi() converts to zero).
 However, if the port remains as zero, a range value of zero (internally, 1) is
appropriate.  Patch snippet below.

2)  "Address" value:  In the latest version, we're storing the address(es)
returned by the apr_sockaddr_info_get() function.  By itself, I don't have a
problem with that and thought that something like that should be done. 
However, there are two issues with the step:

a)  We're fixing an address, but haven't checked to see if it was the ONLY
address that the hostname resolved to.  In my original suggestion, we would
need to make certain that DNS resolution produces a unique address (for a given
address family).  I'm willing to DROP THAT REQUIREMENT, but then we must note
in the documentation that should a hostname resolve to more than one address,
all addresses returned may be used.  Dropping the uniqueness requirement per AF
does make the parsing code simpler.  (If dropped, I think parsing might be

b)  Should there be multiple addresses because there's multiple address
families (e.g. IPv4 and IPv6), we need to try an address in the same family as
the destination.  It would be a problem if we store the IPv6 address (as first)
and try to reach an IPv4-only site when we ignore the IPv4 address available
for the hostname we're binding to because we're not looping through the
alternatives.  Apr_sockaddr_info_get() is returning ALL matching addresses, but
in proxy_util.c, it looks as if we're using only the first one and not checking
to see if we have a reachable address family combination of source and
destination, and when not, trying another address/AF combination.  Since at a
point before the patch, we've already set "backend_addr" (the destination) and
have "backend_addr->family" available, I think we need to compare these:

+                local_addr = conf->bind_addr;
>> if (backend_addr->family != local_addr->family) { /* connection won't work
      TRY ANOTHER ADDRESS from bind_addr, if available.
We may need another loop beyond the port+range loop.  Inner loop or outer is
your choice, but I think an inner loop is more efficient.

c)  Should we pass the port value to apr_sockaddr_info_get()?  I don't think it
makes a difference, but if I remember getaddrinfo() correctly (called 2
routines deep from here), it may - in that it allows us to filter out certain
returns of information.  (Cf. - bug reported for INN 2.4.3 that was returning
each IP address 3 times -
).  I note that the fix for the INN bug is already in the Apache code, but
maybe the port number is also needed to avoid the duplication issue?

PARSING code changes suggested (double "++" for changes):

++ #define MIN_RANGE 8         /* value subject to discussion */
++ #define MAX_PORTS 65535      /* may be defined from an include file */
        /* perhaps use "UINT16_MAX" from <stdint.h> ??? */
        /* perhaps use "USHRT_MAX"  from <limits.h> ??? */

+    if((apr_parse_addr_port(&host, &scope_id, &port, addr, parms->pool)
+                != APR_SUCCESS)
+            || scope_id               /* we dont know how to use scope_id */
+            || (!port && range)       /* only a combo [port+range] is valid */
++           || (port && (!range || (r < MIN_RANGE)))  /* or an invalid
portnumber (p+r) */
++           || ((port + r) > MAX_PORTS) /* or a range that wraps around zero
+      )
+        return "ProxyBindAddress:  Invalid address -"
+               " format is <addr>[:<port>+<range>]";
+    /* Preparse the address */
+    apr_sockaddr_info_get(&(psf->bind_addr), host, APR_UNSPEC, 0, 0,
+    psf->bind_port = port;
+    /* If there didn't exist a port then there was no range either. so we have
+     * starting value 0 for r when no port was specified.*/
++   psf->bind_range = min( port ? MIN_RANGE : 1, r + 1 );
+    psf->bind_idx = 0;
+    psf->bindopt_set = 1;
+    return NULL;

>From proxy_util.c:
+                    conf->bind_idx = i + 1;
+                    break;
+                } else {
+                    ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, "proxy: %s:
not bound to %s%u",
+                            proxy_function, conf->bind_addr, local_addr->port,
+                }

The ELSE isn't necessary, on account of the break.  It will be optimized out.

Configure bugmail:
------- You are receiving this mail because: -------
You are the assignee for the bug.

To unsubscribe, e-mail:
For additional commands, e-mail:

View raw message