httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Luca Toscano <toscano.l...@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 09:02:05 GMT
Hi Yann,

2017-09-18 19:28 GMT+02:00 Yann Ylavic <ylavic.dev@gmail.com>:

> Hi Luca,
>
> On Mon, Sep 18, 2017 at 7:08 PM,  <elukey@apache.org> wrote:
> >
> > Modified: httpd/httpd/trunk/modules/mappers/mod_rewrite.c
> > URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/
> mappers/mod_rewrite.c?rev=1808746&r1=1808745&r2=1808746&view=diff
> > ============================================================
> ==================
> > --- httpd/httpd/trunk/modules/mappers/mod_rewrite.c (original)
> > +++ httpd/httpd/trunk/modules/mappers/mod_rewrite.c Mon Sep 18 17:08:54
> 2017
> > @@ -2035,7 +2035,10 @@ static char *lookup_variable(char *var,
> >
> >              case 'S':
> >                  if (!strcmp(var, "HTTP_HOST")) {
> > -                    result = lookup_header("Host", ctx);
> > +                    /* Skip the 'Vary: Host' header combination
> > +                     * as indicated in rfc7231 section-7.1.4
> > +                     */
> > +                    result = apr_table_get(ctx->r->headers_in, "Host");
>
> Why not address this in lookup_header directly?
> Looks like there several call sites...
>

I missed the HTTP:Host combination, I originally thought to make the strcmp
comparison only when explicitly needed rather than in all cases in which
lookup_header was called..


>
> Most seem to use a fixed name (other than "Host"), but the one around
> line 1879 is:
>
>             if (!strncasecmp(var, "HTTP", 4)) {
>                 result = lookup_header(var+5, ctx);
>             }
>
> so possibly also a candidate.
>
> This might be enough though, to avoid spurious cycles spent in
> lookup_header().
>

Something like the following? What do you think?

Index: modules/mappers/mod_rewrite.c
===================================================================
--- modules/mappers/mod_rewrite.c (revision 1808842)
+++ modules/mappers/mod_rewrite.c (working copy)
@@ -1877,7 +1877,14 @@
             const char *path;

             if (!strncasecmp(var, "HTTP", 4)) {
-                result = lookup_header(var+5, ctx);
+                /* Skip the 'Vary: Host' header combination
+                 * as indicated in rfc7231 section-7.1.4
+                 */
+                if (!strcmp(var+5, "Host")) {
+                    result = apr_table_get(ctx->r->headers_in, "Host");
+                } else {
+                    result = lookup_header(var+5, ctx);
+                }
             }
             else if (!strncasecmp(var, "LA-U", 4)) {
                 if (ctx->uri && subreq_ok(r)) {
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);
 }



Thanks for the review!

Luca

Mime
View raw message