Re: svn commit: r1776575 - in /httpd/httpd/trunk: docs/log-message-tags/next-number docs/manual/mod/mod_remoteip.xml modules/metadata/mod_remoteip.c

2017-02-04 Thread Daniel Ruggeri
I'm not sure if my mail client mangled the message or my email provider did, but I couldn't read this except from lists.a.o so my reply may appear as a new thread in your email client if it's following thread IDs. Christophe JAILLET wrote: > First of all,

Re: AW: svn commit: r1776575 - in /httpd/httpd/trunk: docs/log-message-tags/next-number docs/manual/mod/mod_remoteip.xml modules/metadata/mod_remoteip.c

2017-02-04 Thread Daniel Ruggeri
On 1/30/2017 4:45 AM, Ruediger Pluem wrote: > Thinking of all the above it might be best if you read in mode > AP_MODE_SPECULATIVE on your own from upstream until > you have MIN_HDR_LEN data. If the PROXY header is present, read MIN_HDR_LEN > in AP_MODE_READBYTES to finally consume the > data

Re: AW: svn commit: r1776575 - in /httpd/httpd/trunk: docs/log-message-tags/next-number docs/manual/mod/mod_remoteip.xml modules/metadata/mod_remoteip.c

2017-01-30 Thread Ruediger Pluem
On 01/28/2017 07:14 PM, Daniel Ruggeri wrote: > > On 1/25/2017 9:02 PM, Daniel Ruggeri wrote: >> On 1/25/2017 6:53 PM, Daniel Ruggeri wrote: >>> I'd say that not returning until ctx->bb has enough information to >>> determine if the header is present or not would be sufficient. Isn't >>> this

Re: svn commit: r1776575 - in /httpd/httpd/trunk: docs/log-message-tags/next-number docs/manual/mod/mod_remoteip.xml modules/metadata/mod_remoteip.c

2017-01-30 Thread Ruediger Pluem
On 01/28/2017 06:59 PM, Daniel Ruggeri wrote: > Ruediger Pluem wrote: >> +/* try to read a header's worth of data */ >> +while (!ctx->done) { >> +if (APR_BRIGADE_EMPTY(ctx->bb)) { >> +ret = ap_get_brigade(f->next, ctx->bb, ctx->mode, block, >> +

Re: AW: svn commit: r1776575 - in /httpd/httpd/trunk: docs/log-message-tags/next-number docs/manual/mod/mod_remoteip.xml modules/metadata/mod_remoteip.c

2017-01-28 Thread Daniel Ruggeri
On 1/25/2017 9:02 PM, Daniel Ruggeri wrote: > On 1/25/2017 6:53 PM, Daniel Ruggeri wrote: >> I'd say that not returning until ctx->bb has enough information to >> determine if the header is present or not would be sufficient. Isn't >> this already done in the potentially repeated calls to

Re: svn commit: r1776575 - in /httpd/httpd/trunk: docs/log-message-tags/next-number docs/manual/mod/mod_remoteip.xml modules/metadata/mod_remoteip.c

2017-01-28 Thread Daniel Ruggeri
Ruediger Pluem wrote: > +/* try to read a header's worth of data */ > +while (!ctx->done) { > +if (APR_BRIGADE_EMPTY(ctx->bb)) { > +ret = ap_get_brigade(f->next, ctx->bb, ctx->mode, block, > + ctx->need - ctx->rcvd); > +if

Re: AW: svn commit: r1776575 - in /httpd/httpd/trunk: docs/log-message-tags/next-number docs/manual/mod/mod_remoteip.xml modules/metadata/mod_remoteip.c

2017-01-25 Thread Daniel Ruggeri
On 1/25/2017 6:53 PM, Daniel Ruggeri wrote: > I'd say that not returning until ctx->bb has enough information to > determine if the header is present or not would be sufficient. Isn't > this already done in the potentially repeated calls to ap_get_brigade on > line no 1056 inside the loop

Re: AW: svn commit: r1776575 - in /httpd/httpd/trunk: docs/log-message-tags/next-number docs/manual/mod/mod_remoteip.xml modules/metadata/mod_remoteip.c

2017-01-25 Thread Daniel Ruggeri
...@primary.net] >> Gesendet: Dienstag, 17. Januar 2017 00:57 >> An: dev@httpd.apache.org >> Betreff: Re: svn commit: r1776575 - in /httpd/httpd/trunk: docs/log- >> message-tags/next-number docs/manual/mod/mod_remoteip.xml >> modules/metadata/mod_remoteip.c >> >

Re: svn commit: r1776575 - in /httpd/httpd/trunk: docs/log-message-tags/next-number docs/manual/mod/mod_remoteip.xml modules/metadata/mod_remoteip.c

2017-01-25 Thread Daniel Ruggeri
To clarify, the issues with handling of the buckets and the fact that buckets are not being set aside came from the original code. The question about what to do regarding f->next versus passing the data along some other way is the only one that came from the code I added to make the header

Re: svn commit: r1776575 - in /httpd/httpd/trunk: docs/log-message-tags/next-number docs/manual/mod/mod_remoteip.xml modules/metadata/mod_remoteip.c

2017-01-24 Thread Jim Jagielski
++1. I know that Daniel is out of pocket for a little bit so I'l give it a coupla more days before I "restore" to the original filter code... > On Jan 24, 2017, at 3:46 AM, Ruediger Pluem wrote: > > > > On 01/17/2017 02:48 PM, Jim Jagielski wrote: >> >>> On Jan 16, 2017, at

Re: svn commit: r1776575 - in /httpd/httpd/trunk: docs/log-message-tags/next-number docs/manual/mod/mod_remoteip.xml modules/metadata/mod_remoteip.c

2017-01-24 Thread Ruediger Pluem
On 01/17/2017 02:48 PM, Jim Jagielski wrote: > >> On Jan 16, 2017, at 6:57 PM, Daniel Ruggeri wrote: >> >> For the most part, yes except the portions that make the header presence >> optional (the HDR_MISSING case). Those were added as it came into the >> code base to

Re: svn commit: r1776575 - in /httpd/httpd/trunk: docs/log-message-tags/next-number docs/manual/mod/mod_remoteip.xml modules/metadata/mod_remoteip.c

2017-01-20 Thread Christophe JAILLET
First of all, http://blog.haproxy.com/haproxy/proxy-protocol/ list another module implementation for Apache: https://github.com/ggrandes/apache24-modules/blob/master/mod_myfixip.c If anyone wants to give it a look. Anyway, a few minor comments below. CJ Le 30/12/2016 à 15:20,

Re: svn commit: r1776575 - in /httpd/httpd/trunk: docs/log-message-tags/next-number docs/manual/mod/mod_remoteip.xml modules/metadata/mod_remoteip.c

2017-01-17 Thread Jim Jagielski
> On Jan 16, 2017, at 6:57 PM, Daniel Ruggeri wrote: > > For the most part, yes except the portions that make the header presence > optional (the HDR_MISSING case). Those were added as it came into the > code base to handle a use case I was working on. I've added some >

AW: svn commit: r1776575 - in /httpd/httpd/trunk: docs/log-message-tags/next-number docs/manual/mod/mod_remoteip.xml modules/metadata/mod_remoteip.c

2017-01-17 Thread Plüm , Rüdiger , Vodafone Group
> -Ursprüngliche Nachricht- > Von: Daniel Ruggeri [mailto:drugg...@primary.net] > Gesendet: Dienstag, 17. Januar 2017 00:57 > An: dev@httpd.apache.org > Betreff: Re: svn commit: r1776575 - in /httpd/httpd/trunk: docs/log- > message-tags/next-number docs/manual/mo

Re: svn commit: r1776575 - in /httpd/httpd/trunk: docs/log-message-tags/next-number docs/manual/mod/mod_remoteip.xml modules/metadata/mod_remoteip.c

2017-01-16 Thread Daniel Ruggeri
For the most part, yes except the portions that make the header presence optional (the HDR_MISSING case). Those were added as it came into the code base to handle a use case I was working on. I've added some comments inline since I won't have time to poke around myself for a while yet. For

Re: svn commit: r1776575 - in /httpd/httpd/trunk: docs/log-message-tags/next-number docs/manual/mod/mod_remoteip.xml modules/metadata/mod_remoteip.c

2017-01-16 Thread Jim Jagielski
Let me take a look... afaict, this is a copy of what was donated, which has been working for numerous people. But that doesn't mean it can't have bugs ;) > On Jan 16, 2017, at 7:20 AM, Ruediger Pluem wrote: > > Anyone? > > Regards > > Rüdiger > > On 01/10/2017 12:39 PM,

Re: svn commit: r1776575 - in /httpd/httpd/trunk: docs/log-message-tags/next-number docs/manual/mod/mod_remoteip.xml modules/metadata/mod_remoteip.c

2017-01-16 Thread Ruediger Pluem
Anyone? Regards Rüdiger On 01/10/2017 12:39 PM, Ruediger Pluem wrote: > > > On 12/30/2016 03:20 PM, 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=rev >> Log: >> Merge new PROXY

Re: svn commit: r1776575 - in /httpd/httpd/trunk: docs/log-message-tags/next-number docs/manual/mod/mod_remoteip.xml modules/metadata/mod_remoteip.c

2017-01-10 Thread Ruediger Pluem
On 12/30/2016 03:20 PM, 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=rev > Log: > Merge new PROXY protocol code into mod_remoteip > > Modified: >

Re: svn commit: r1776575 - in /httpd/httpd/trunk: docs/log-message-tags/next-number docs/manual/mod/mod_remoteip.xml modules/metadata/mod_remoteip.c

2016-12-30 Thread Daniel Ruggeri
On 12/30/2016 8:37 PM, William A Rowe Jr wrote: > Simply address the issue that caused the -1... 'Code mostly > copyright...' needs to be 'Portions copyright'... A statement which is > unlikely to become entirely false. OK, got it. It wasn't entirely clear to me that was the hangup. On

Re: svn commit: r1776575 - in /httpd/httpd/trunk: docs/log-message-tags/next-number docs/manual/mod/mod_remoteip.xml modules/metadata/mod_remoteip.c

2016-12-30 Thread William A Rowe Jr
Simply address the issue that caused the -1... 'Code mostly copyright...' needs to be 'Portions copyright'... A statement which is unlikely to become entirely false. On Dec 30, 2016 18:28, "Daniel Ruggeri" wrote: > Aye - I suspected this would raise eyebrows so I did

Re: svn commit: r1776575 - in /httpd/httpd/trunk: docs/log-message-tags/next-number docs/manual/mod/mod_remoteip.xml modules/metadata/mod_remoteip.c

2016-12-30 Thread Eric Covener
On Fri, Dec 30, 2016 at 9:27 PM, Daniel Ruggeri wrote: > Aye - I suspected this would raise eyebrows so I did bring it up a few > times [1][2]. I'm sure we're in agreement that attribution is important > in the Open Source world so I'd like to be sure it's done

Re: svn commit: r1776575 - in /httpd/httpd/trunk: docs/log-message-tags/next-number docs/manual/mod/mod_remoteip.xml modules/metadata/mod_remoteip.c

2016-12-30 Thread Daniel Ruggeri
Aye - I suspected this would raise eyebrows so I did bring it up a few times [1][2]. I'm sure we're in agreement that attribution is important in the Open Source world so I'd like to be sure it's done appropriately. I'm happy to fix. Currently, though, I'm not sure how best to handle this veto...

Re: svn commit: r1776575 - in /httpd/httpd/trunk: docs/log-message-tags/next-number docs/manual/mod/mod_remoteip.xml modules/metadata/mod_remoteip.c

2016-12-30 Thread Eric Covener
On Fri, Dec 30, 2016 at 9:00 PM, William A Rowe Jr wrote: > On Dec 30, 2016 06:20, wrote: > > Author: druggeri > Date: Fri Dec 30 14:20:48 2016 > New Revision: 1776575 > > URL: http://svn.apache.org/viewvc?rev=1776575=rev > Log: > Merge new PROXY

Re: svn commit: r1776575 - in /httpd/httpd/trunk: docs/log-message-tags/next-number docs/manual/mod/mod_remoteip.xml modules/metadata/mod_remoteip.c

2016-12-30 Thread William A Rowe Jr
On Dec 30, 2016 06:20, wrote: Author: druggeri Date: Fri Dec 30 14:20:48 2016 New Revision: 1776575 URL: http://svn.apache.org/viewvc?rev=1776575=rev Log: Merge new PROXY protocol code into mod_remoteip Modified: httpd/httpd/trunk/docs/log-message-tags/next-number

Re: svn commit: r1776575 - in /httpd/httpd/trunk: docs/log-message-tags/next-number docs/manual/mod/mod_remoteip.xml modules/metadata/mod_remoteip.c

2016-12-30 Thread Eric Covener
On Fri, Dec 30, 2016 at 10:22 AM, Daniel Ruggeri wrote: > I do like the idea of being nice to end users... but this may be a dumb > followup question showing a lack of understanding. How would we trigger > behavior in two different modules for the same directive unless the >

Re: svn commit: r1776575 - in /httpd/httpd/trunk: docs/log-message-tags/next-number docs/manual/mod/mod_remoteip.xml modules/metadata/mod_remoteip.c

2016-12-30 Thread Daniel Ruggeri
On 12/30/2016 9:04 AM, Jim Jagielski wrote: > I was thinking something like > > ProxyProtocolEnable Incoming | Outgoing | Both | Optional | Off > > so that we have 1 command for all possible PROXY PROTOCOL usages. > This, I think, is clearer for end-users. I do like the idea of being nice to

Re: svn commit: r1776575 - in /httpd/httpd/trunk: docs/log-message-tags/next-number docs/manual/mod/mod_remoteip.xml modules/metadata/mod_remoteip.c

2016-12-30 Thread Jim Jagielski
Yeah, the "namespace" stuff makes sense, but really, when you think of it, the PROXY support is, theoretically at least, independent of RemoteIP, even though that module is the one that implements it. It makes it seem like it is "different" in some way to PROXY... I was thinking something like

Re: svn commit: r1776575 - in /httpd/httpd/trunk: docs/log-message-tags/next-number docs/manual/mod/mod_remoteip.xml modules/metadata/mod_remoteip.c

2016-12-30 Thread Daniel Ruggeri
On 12/30/2016 8:28 AM, Jim Jagielski wrote: > Kewl beans! Indeed - the best beans to have! > Any issue if we rename the directive to just ProxyProtocolEnable? > The "RemoteIP" prefix just seems weird :) I assume we try to keep a "namespace" for the more optional modules like this. It does seem

Re: svn commit: r1776575 - in /httpd/httpd/trunk: docs/log-message-tags/next-number docs/manual/mod/mod_remoteip.xml modules/metadata/mod_remoteip.c

2016-12-30 Thread Daniel Ruggeri
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

Re: svn commit: r1776575 - in /httpd/httpd/trunk: docs/log-message-tags/next-number docs/manual/mod/mod_remoteip.xml modules/metadata/mod_remoteip.c

2016-12-30 Thread Jim Jagielski
Kewl beans! Any issue if we rename the directive to just ProxyProtocolEnable? The "RemoteIP" prefix just seems weird :) Also, just as a head's up, I'm looking on adding PROXY support to the proxy module itself (that is, we *send* the PROXY line to backends) as a configurable option. So when that