httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "William A. Rowe Jr." <>
Subject Re: r->connection->remote_ip and mod_remoteip
Date Sun, 07 Feb 2010 04:18:03 GMT
On 2/5/2010 7:05 PM, Graham Leggett wrote:
> Why does this not just use a simple pool cleanup on request end to
> restore the values, instead of all the song and dance to try and keep
> the previous value for restoration in the next request? Attaching it to
> a pool is fine, restoring it in the next request isn't clean.

That seems entirely sensible, but other than "it feels dirty" did you
actually have any observations of a flaw?

> I do notice mod_remoteip can only be set server wide, and not per
> directory.

Of course - and that won't change; the module interacts immediately before
reading the request; not before translating the name, or performing auth,
or any other request processing.  It is designed to have every aspect of
httpd request processing interact with the apparent address, not the
upstream physical connection.

Directories are associated in the map-to-storage phase.

> I'm sitting with one (non ASF) module that copies the header direct into
> the connection (!!!), I have a second (non ASF) module that seems to be
> a later incarnation of the first one that at least gets the pool
> lifetimes right and allocates the new IP out of the connection pool, but
> it remains a leak, particularly as the module is designed to accept
> connections from a load balancer with a connection pool where every
> request will have a different IP.

Writing modules is hard.  Writing modules well is harder.  Changing the
schema rarely solves _that_ underlying problem, but just bloats our req
and conn records even further.

> In trying to unravel this module mess I have inherited, I noticed that
> all these modules are allowing the request to poke around inside the
> connection, which didn't seem very clean to me.

> Instead, what I propose are fields inside the request itself, copied by
> reference from the connection, and modules can then fiddle with these
> request specific IP addresses to their heart's content, knowing that the
> scope of the information is accurate.

If we had a conn_rec::remote and request_rec::remote which pointed to all
of those request_* fields, where the r->remote defaults to the c->remote
value, the conn_rec and request_rec are made cleaner with minimal size
impact.  Users needing to fix c->remote_ip into r->remote->ip would be
more likely to choose correctly.  Given the state of modules today, any
user of mod_remoteip would likely be disappointed by the third party
module interaction when they adopted it.

> It also solves the problem that in cases where you want to log the
> original browser IP *and* the IP of the load balancer, you can do so.
> Right now, the real IP is obscured completely.

Not completely; the real IP chain (from the load balancer through all
of the proxy addresses) is present and available in notes.  Please do
take a closer look at the module before suggesting it needs redesign :)

View raw message