I went ahead and ported this code to mod_remoteip. I also taught it how to be optional (process PROXY headers when available, but don't fail otherwise), checks for IPv6 in APR as well as log numbers. A small bit of refactoring was included. A few questions came up during initial code review and the porting... opinions would be appreciated.
On 12/30/2016 8:20 AM, drugg...@apache.org wrote: > Author: druggeri > Date: Fri Dec 30 14:20:48 2016 > New Revision: 1776575 > > URL: http://svn.apache.org/viewvc?rev=1776575&view=rev > Log: > Merge new PROXY protocol code into mod_remoteip > > Modified: > httpd/httpd/trunk/docs/log-message-tags/next-number > httpd/httpd/trunk/docs/manual/mod/mod_remoteip.xml > httpd/httpd/trunk/modules/metadata/mod_remoteip.c . . . > Modified: httpd/httpd/trunk/modules/metadata/mod_remoteip.c > URL: > http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/metadata/mod_remoteip.c?rev=1776575&r1=1776574&r2=1776575&view=diff > ============================================================================== > --- httpd/httpd/trunk/modules/metadata/mod_remoteip.c (original) > +++ httpd/httpd/trunk/modules/metadata/mod_remoteip.c Fri Dec 30 14:20:48 2016 > @@ -12,15 +12,20 @@ > * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > * See the License for the specific language governing permissions and > * limitations under the License. > + * > + * The majority of the input filter code for PROXY protocol support is > + * Copyright 2014 Cloudzilla Inc. > */ Is this sufficient attribution? > +/* XXX: Unsure if this is needed if v6 support is not available on > + this platform */ > +#ifndef INET6_ADDRSTRLEN > +#define INET6_ADDRSTRLEN 46 > +#endif I don't have a non-IPv6 capable platform so I'm not sure if this will be defined anyway. Is this needed? > + > + /* add our filter */ > + if (!ap_add_input_filter(remoteip_filter_name, NULL, NULL, c)) { > + /* XXX: Shouldn't this WARN in log? */ > + return DECLINED; > + } This came from the original code... but I assume that if the admin desired for this to be configured, but we failed to enable it... we should at least complain. Maybe more? -- Daniel Ruggeri
