httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Daniel Ruggeri <DRugg...@primary.net>
Subject 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
Date Fri, 30 Dec 2016 14:33:08 GMT
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, druggeri@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



Mime
View raw message