httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Yann Ylavic <ylavic....@gmail.com>
Subject Re: svn commit: r1808746 - in /httpd/httpd/trunk: CHANGES modules/mappers/mod_rewrite.c server/util_expr_eval.c
Date Tue, 19 Sep 2017 10:13:06 GMT
Hi Luca,

On Tue, Sep 19, 2017 at 11:02 AM, Luca Toscano <toscano.luca@gmail.com> wrote:
>
> Something like the following? What do you think?

After a quick look, I think we should put the test in looker_header, like:

Index: modules/mappers/mod_rewrite.c
===================================================================
--- modules/mappers/mod_rewrite.c       (revision 1808823)
+++ modules/mappers/mod_rewrite.c       (working copy)
@@ -1796,7 +1796,7 @@ static const char *lookup_header(const char *name,
 {
     const char *val = apr_table_get(ctx->r->headers_in, name);

-    if (val) {
+    if (val && strcasecmp(name, "Host") != 0) {
         ctx->vary_this = ctx->vary_this
                          ? apr_pstrcat(ctx->r->pool, ctx->vary_this, ", ",
                                        name, NULL)
--

The call sites either have the name to be really checked (e.g. var+5),
or they use string literals (e.g. "Host", "Accept", ...) thus the
compiler is able to inline and optimize in or out the test by itself.


> Index: server/util_expr_eval.c
> ===================================================================
> --- server/util_expr_eval.c (revision 1808842)
> +++ server/util_expr_eval.c (working copy)
> @@ -1044,7 +1044,12 @@
>          t = ctx->r->headers_in;
>      else {                          /* req, http */
>          t = ctx->r->headers_in;
> -        add_vary(ctx, arg);
> +        /* Skip the 'Vary: Host' header combination
> +         * as indicated in rfc7231 section-7.1.4
> +         */
> +        if (strcmp(arg, "Host")){
> +            add_vary(ctx, arg);
> +        }
>      }
>      return apr_table_get(t, arg);
>  }

This one looks good.


Regards,
Yann.

Mime
View raw message