You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Rodent of Unusual Size <Ke...@Golux.Com> on 1998/07/11 18:41:23 UTC

[PATCH] PR#1644: mod_rewrite and Vary

I decided to tackle the less-impact case first.

The attached patch updates the Vary response header field if any of the
request header fields were involved in successful RewriteCond matches.

Ralf, I'd particularly appreciate your feedback on this.

#ken	P-)}

Ken Coar                    <http://Web.Golux.Com/coar/>
Apache Group member         <http://www.apache.org/>
"Apache Server for Dummies" <http://Web.Golux.Com/coar/ASFD/>

Index: modules/standard/mod_rewrite.c
===================================================================
RCS file: /export/home/cvs/apache-1.3/src/modules/standard/mod_rewrite.c,v
retrieving revision 1.123
diff -u -r1.123 mod_rewrite.c
--- mod_rewrite.c       1998/07/11 10:56:07     1.123
+++ mod_rewrite.c       1998/07/11 16:38:42
@@ -1728,6 +1728,7 @@
 {
     char *uri;
     char *output;
+    const char *vary;
     char newuri[MAX_STRING_LEN];
     char env[MAX_STRING_LEN];
     regex_t *regexp;
@@ -1841,6 +1842,7 @@
                 /*  One condition is false, but another can be
                  *  still true, so we have to continue...
                  */
+               ap_table_unset(r->notes, VARY_KEY);
                 continue;
             }
             else {
@@ -1869,10 +1871,20 @@
     }
     /*  if any condition fails the complete rule fails  */
     if (failed) {
+        ap_table_unset(r->notes, VARY_KEY);
         return 0;
     }
 
     /*
+     * Regardless of what we do next, we've found a match.  Check to see
+     * if any of the request header fields were involved, and add them
+     * to the Vary field of the response.
+     */
+    if ((vary = ap_table_get(r->notes, VARY_KEY)) != NULL) {
+        ap_table_merge(r->headers_out, "Vary", vary);
+    }
+
+    /*
      *  If this is a pure matching rule (`RewriteRule <pat> -')
      *  we stop processing and return immediately. The only thing
      *  we have not to forget are the environment variables
@@ -3716,6 +3728,7 @@
             continue;
         }
         if (strcasecmp(hdrs[i].key, name) == 0) {
+           ap_table_merge(r->notes, VARY_KEY, name);
             return hdrs[i].val;
         }
     }
Index: modules/standard/mod_rewrite.h
===================================================================
RCS file: /export/home/cvs/apache-1.3/src/modules/standard/mod_rewrite.h,v
retrieving revision 1.54
diff -u -r1.54 mod_rewrite.h
--- mod_rewrite.h       1998/07/11 10:56:08     1.54
+++ mod_rewrite.h       1998/07/11 16:38:42
@@ -113,6 +113,12 @@
 #include "http_log.h"
 #include "http_vhost.h"
 
+    /*
+     * The key in the r->notes table wherein we store our accumulated
+     * Vary values.
+     */
+#define VARY_KEY "rewrite-Vary"
+
     /* The NDBM support:
      * We support only NDBM files.
      * But we have to stat the file for the mtime,

Re: [PATCH] PR#1644: mod_rewrite and Vary

Posted by Rodent of Unusual Size <Ke...@Golux.Com>.
Rodent of Unusual Size wrote:
> 
> Aw, nuts.  This is broken; any condition failure will wipe out
> the record of any previous successes.  Take 2 coming up..

Here's take 2.

It's still only a partial fix.  We don't handle setting the Vary
field very well; for instance, Vary isn't sent back if the
rewrite resulted in a cachable redirect.  So I'm not sure this
is even worth it.

Opinions?

#ken	P-)}

Ken Coar                    <http://Web.Golux.Com/coar/>
Apache Group member         <http://www.apache.org/>
"Apache Server for Dummies" <http://Web.Golux.Com/coar/ASFD/>

Index: modules/standard/mod_rewrite.c
===================================================================
RCS file: /export/home/cvs/apache-1.3/src/modules/standard/mod_rewrite.c,v
retrieving revision 1.123
diff -u -r1.123 mod_rewrite.c
--- mod_rewrite.c       1998/07/11 10:56:07     1.123
+++ mod_rewrite.c       1998/07/11 19:09:25
@@ -1728,6 +1728,7 @@
 {
     char *uri;
     char *output;
+    const char *vary;
     char newuri[MAX_STRING_LEN];
     char env[MAX_STRING_LEN];
     regex_t *regexp;
@@ -1841,6 +1842,7 @@
                 /*  One condition is false, but another can be
                  *  still true, so we have to continue...
                  */
+               ap_table_unset(r->notes, VARY_KEY_THIS);
                 continue;
             }
             else {
@@ -1866,13 +1868,28 @@
                 break;
             }
         }
+       vary = ap_table_get(r->notes, VARY_KEY_THIS);
+       if (vary != NULL) {
+           ap_table_merge(r->notes, VARY_KEY, vary);
+           ap_table_unset(r->notes, VARY_KEY_THIS);
+       }
     }
     /*  if any condition fails the complete rule fails  */
     if (failed) {
+        ap_table_unset(r->notes, VARY_KEY);
         return 0;
     }
 
     /*
+     * Regardless of what we do next, we've found a match.  Check to see
+     * if any of the request header fields were involved, and add them
+     * to the Vary field of the response.
+     */
+    if ((vary = ap_table_get(r->notes, VARY_KEY)) != NULL) {
+        ap_table_merge(r->headers_out, "Vary", vary);
+    }
+
+    /*
      *  If this is a pure matching rule (`RewriteRule <pat> -')
      *  we stop processing and return immediately. The only thing
      *  we have not to forget are the environment variables
@@ -3716,6 +3733,7 @@
             continue;
         }
         if (strcasecmp(hdrs[i].key, name) == 0) {
+           ap_table_merge(r->notes, VARY_KEY_THIS, name);
             return hdrs[i].val;
         }
     }
Index: modules/standard/mod_rewrite.h
===================================================================
RCS file: /export/home/cvs/apache-1.3/src/modules/standard/mod_rewrite.h,v
retrieving revision 1.54
diff -u -r1.54 mod_rewrite.h
--- mod_rewrite.h       1998/07/11 10:56:08     1.54
+++ mod_rewrite.h       1998/07/11 19:09:26
@@ -113,6 +113,13 @@
 #include "http_log.h"
 #include "http_vhost.h"
 
+    /*
+     * The key in the r->notes table wherein we store our accumulated
+     * Vary values, and the one used for per-condition checks in a chain.
+     */
+#define VARY_KEY "rewrite-Vary"
+#define VARY_KEY_THIS "rewrite-Vary-this"
+
     /* The NDBM support:
      * We support only NDBM files.
      * But we have to stat the file for the mtime,

Re: [PATCH] PR#1644: mod_rewrite and Vary

Posted by Rodent of Unusual Size <Ke...@Golux.Com>.
Rodent of Unusual Size wrote:
> 
>                  /*  One condition is false, but another can be
>                   *  still true, so we have to continue...
>                   */
> +               ap_table_unset(r->notes, VARY_KEY);
>                  continue;

Aw, nuts.  This is broken; any condition failure will wipe out
the record of any previous successes.  Take 2 coming up..

#ken	P-)}

Ken Coar                    <http://Web.Golux.Com/coar/>
Apache Group member         <http://www.apache.org/>
"Apache Server for Dummies" <http://Web.Golux.Com/coar/ASFD/>