httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Dirk-Willem van Gulik <di...@webweaving.org>
Subject Patch: Centralize some Cache header handling
Date Fri, 08 Feb 2008 22:23:31 GMT
On Feb 8, 2008, at 6:26 PM, Dirk-Willem van Gulik wrote:

> Right now we have a nice utility function:
>
> 	ap_cache_cacheable_hdrs_out()
> 		Make a copy of the headers, and remove from
> 		the copy any hop-by-hop headers, as defined in Section
> 		13.5.1 of RFC 2616
>
> (in cache_util.c, part of mod_cache.h public headers) which we can  
> apply to both the in and outbound headers.
>
> Now as far as I can see -every- caching module will have to take  
> care of the error headers and the content -- and filtering both in- 
> and-out that way.
>
> So how about changing this to:
>
> -	ap_cache_cacheable_hdrs_out(r);
> 		calls ap_cache_cacheable_hdrs
> 	AND
> 		Set Content-Type if not set
> 	AND
> 		overlays r->err_headers_out
>
> -	ap_cache_cacheable_hdrs_in(r);
> 		calles ap_cache_cacheable_hdrs
>
> -	ap_cache_cacheable_hdrs(pool, headers, server) (private)
> 		delete/cleanse as per RFC 2616
> 	and as per CacheIgnoreHeaders
>
> Or is there a fundamental reason as to why some caches will not want  
> to do the 'usual'* ?
>
> Thoughts ?

Love to hear caching experts their thoughts -- esp. to hear confirmed  
that it is correct that mod_cache.c	needs indeed just to touch  
headers_out (and not in) at that point and has no need to touch up  
content-type.

Thanks,

Dw


Index: mod_cache.h
===================================================================
--- mod_cache.h	(revision 618646)
+++ mod_cache.h	(working copy)
@@ -289,12 +289,23 @@
  CACHE_DECLARE(const char *)ap_cache_tokstr(apr_pool_t *p, const char  
*list, const char **str);

  /* Create a new table consisting of those elements from a  
request_rec's
- * headers_out that are allowed to be stored in a cache
+ * headers that are allowed to be stored in a cache
   */
-CACHE_DECLARE(apr_table_t *)ap_cache_cacheable_hdrs_out(apr_pool_t  
*pool,
+CACHE_DECLARE(apr_table_t *)ap_cache_cacheable_hdrs(apr_pool_t *pool,
                                                          apr_table_t  
*t,
                                                          server_rec  
*s);

+/* Create a new table consisting of those elements from an input
+ * headers table that are allowed to be stored in a cache.
+ */
+CACHE_DECLARE(apr_table_t *)ap_cache_cacheable_hdrs_in(request_rec *);
+
+/* Create a new table consisting of those elements from an output
+ * headers table that are allowed to be stored in a cache;
+ * ensure there is a content type and capture any errors.
+ */
+CACHE_DECLARE(apr_table_t *)ap_cache_cacheable_hdrs_out(request_rec *);
+
  /**
   * cache_storage.c
   */
Index: cache_util.c
===================================================================
--- cache_util.c	(revision 618646)
+++ cache_util.c	(working copy)
@@ -571,10 +573,7 @@
      return apr_pstrdup(p, hashfile);
  }

-/* Create a new table consisting of those elements from an input
- * headers table that are allowed to be stored in a cache.
- */
-CACHE_DECLARE(apr_table_t *)ap_cache_cacheable_hdrs_out(apr_pool_t  
*pool,
+CACHE_DECLARE(apr_table_t *)ap_cache_cacheable_hdrs(apr_pool_t *pool,
                                                          apr_table_t  
*t,
                                                          server_rec *s)
  {
@@ -608,3 +607,34 @@
      }
      return headers_out;
  }
+
+/* Create a new table consisting of those elements from an input
+ * headers table that are allowed to be stored in a cache.
+ */
+CACHE_DECLARE(apr_table_t *)ap_cache_cacheable_hdrs_in(request_rec * r)
+{
+	return ap_cache_cacheable_hdrs(r->pool, r->headers_in, r->server);
+}
+
+/* Create a new table consisting of those elements from an output
+ * headers table that are allowed to be stored in a cache;
+ * ensure there is a content type and capture any errors.
+ */
+CACHE_DECLARE(apr_table_t *)ap_cache_cacheable_hdrs_out(request_rec *  
r)
+{
+	apr_table_t *headers_out;
+
+	headers_out = ap_cache_cacheable_hdrs(r->pool, r->headers_out,
+                                                  r->server);
+
+        if (!apr_table_get(headers_out, "Content-Type")
+            && r->content_type) {
+            apr_table_setn(headers_out, "Content-Type",
+                           ap_make_content_type(r, r->content_type));
+        }
+
+        headers_out = apr_table_overlay(r->pool, headers_out,
+                                        r->err_headers_out);
+
+	return headers_out;
+}
Index: mod_cache.c
===================================================================
--- mod_cache.c	(revision 618646)
+++ mod_cache.c	(working copy)
@@ -752,10 +778,12 @@
           * However, before doing that, we need to first merge in
           * err_headers_out and we also need to strip any hop-by-hop
           * headers that might have snuck in.
+         *
+	 * XX check -- we're not patching up content-type.
           */
          r->headers_out = apr_table_overlay(r->pool, r->headers_out,
                                             r->err_headers_out);
-        r->headers_out = ap_cache_cacheable_hdrs_out(r->pool, r- 
 >headers_out,
+        r->headers_out = ap_cache_cacheable_hdrs(r->pool, r- 
 >headers_out,
                                                       r->server);
          apr_table_clear(r->err_headers_out);

Index: mod_disk_cache.c
===================================================================
--- mod_disk_cache.c	(revision 618646)
+++ mod_disk_cache.c	(working copy)
@@ -922,17 +922,8 @@
      if (r->headers_out) {
          apr_table_t *headers_out;

-        headers_out = ap_cache_cacheable_hdrs_out(r->pool, r- 
 >headers_out,
-                                                  r->server);
+        headers_out = ap_cache_cacheable_hdrs_out(r);

-        if (!apr_table_get(headers_out, "Content-Type")
-            && r->content_type) {
-            apr_table_setn(headers_out, "Content-Type",
-                           ap_make_content_type(r, r->content_type));
-        }
-
-        headers_out = apr_table_overlay(r->pool, headers_out,
-                                        r->err_headers_out);
          rv = store_table(dobj->hfd, headers_out);
          if (rv != APR_SUCCESS) {
              return rv;
@@ -944,8 +935,8 @@
      if (r->headers_in) {
          apr_table_t *headers_in;

-        headers_in = ap_cache_cacheable_hdrs_out(r->pool, r- 
 >headers_in,
-                                                 r->server);
+        headers_in = ap_cache_cacheable_hdrs_in(r);
+
          rv = store_table(dobj->hfd, headers_in);
          if (rv != APR_SUCCESS) {
              return rv;
Index: mod_mem_cache.c
===================================================================
--- mod_mem_cache.c	(revision 618646)
+++ mod_mem_cache.c	(working copy)
@@ -605,17 +605,8 @@
      mobj->req_hdrs = deep_table_copy(mobj->pool, r->headers_in);

      /* Precompute how much storage we need to hold the headers */
-    headers_out = ap_cache_cacheable_hdrs_out(r->pool, r->headers_out,
-                                              r->server);
+    headers_out = ap_cache_cacheable_hdrs_out(r);

-    /* If not set in headers_out, set Content-Type */
-    if (!apr_table_get(headers_out, "Content-Type")
-        && r->content_type) {
-        apr_table_setn(headers_out, "Content-Type",
-                       ap_make_content_type(r, r->content_type));
-    }
-
-    headers_out = apr_table_overlay(r->pool, headers_out, r- 
 >err_headers_out);
      mobj->header_out = deep_table_copy(mobj->pool, headers_out);

      /* Init the info struct */


Mime
View raw message