You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Dirk-Willem van Gulik <di...@webweaving.org> on 2008/04/17 13:04:56 UTC
Centralizing http-proxy knowledge in cache_util.c
Before I give the other modules a pass over -- does below strawman
address everyones
concerns w.r. to the API changes ?
To recap:
- ap_cache_cacheable_hdrs_out gets depricated. And the repliced
code is to be removed from mod_*_cache.
- a ap_cache_cacheable_hdrs_in() and ap_cache_cacheable_hdrs_out()
are added, which each rely on:
- ap_cache_cacheable_headers - which knows the HTTP rules.
The intention is to make sure that caching modules do no longer have to
understand the section in RFC 2616 deling with hop-by-hop.
Right now _out is just a wrapper for ap_cache_cacheable_headers - but
I expect that we'll have to populate it soon. And to avoid an API
version
bump I'd thus rather not do it as a #define for now.
Thanks,
Dw.
Index: CHANGES
===================================================================
--- CHANGES (revision 649044)
+++ CHANGES (working copy)
@@ -2,6 +2,14 @@
Changes with Apache 2.3.0
[ When backported to 2.2.x, remove entry from this file ]
+ *) cache: remove ap_cache_cacheable_hdrs_out() which was used
+ for both in- and out-put headers; and replace it by a single
+ ap_cache_cacheable_headers() wrapped in a in- and out-put
+ specific ap_cache_cacheable_hdrs_in()/out(). The latter
+ which will also merge error and ensure content-type. To keep
+ cache modules consistent with ease. This API change bumps
+ up the minor MM by one [Dirk-Willem van Gulik].
+
*) mod_rewrite: Allow Cookie option to set secure and HttpOnly
flags.
PR 44799 [Christian Wenz <christian wenz.org>]
Index: modules/cache/mod_cache.h
===================================================================
--- modules/cache/mod_cache.h (revision 649044)
+++ modules/cache/mod_cache.h (working copy)
@@ -286,9 +286,27 @@
const char *key, char **val);
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
+/* Create a new table consisting of those elements from an
+ * headers table that are allowed to be stored in a cache.
*/
+CACHE_DECLARE(apr_table_t *)ap_cache_cacheable_headers(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_headers_in(request_rec
* r);
+
+/* 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_headers_out(request_rec * r);
+
+/* Legacy call - functionally equivalent to ap_cache_cacheable_headers.
+ * @deprecated @see ap_cache_cacheable_headers
+ */
CACHE_DECLARE(apr_table_t *)ap_cache_cacheable_hdrs_out(apr_pool_t
*pool,
apr_table_t
*t,
server_rec
*s);
Index: modules/cache/cache_util.c
===================================================================
--- modules/cache/cache_util.c (revision 649044)
+++ modules/cache/cache_util.c (working copy)
@@ -577,10 +577,10 @@
return apr_pstrdup(p, hashfile);
}
-/* Create a new table consisting of those elements from an input
+/* Create a new table consisting of those elements from an
* 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_headers(apr_pool_t
*pool,
apr_table_t
*t,
server_rec *s)
{
@@ -588,12 +588,20 @@
char **header;
int i;
+ /* Short circuit the common case that there are not
+ * (yet) any headers populated.
+ */
+ if (t == NULL) {
+ return apr_table_make(pool, 10);
+ };
+
/* 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
*/
apr_table_t *headers_out;
headers_out = apr_table_copy(pool, t);
+
apr_table_unset(headers_out, "Connection");
apr_table_unset(headers_out, "Keep-Alive");
apr_table_unset(headers_out, "Proxy-Authenticate");
@@ -605,6 +613,7 @@
conf = (cache_server_conf *)ap_get_module_config(s->module_config,
&cache_module);
+
/* Remove the user defined headers set with CacheIgnoreHeaders.
* This may break RFC 2616 compliance on behalf of the
administrator.
*/
@@ -614,3 +623,44 @@
}
return headers_out;
}
+
+/* Legacy call - functionally equivalent to ap_cache_cacheable_headers.
+ * @deprecated @see ap_cache_cacheable_headers
+ */
+CACHE_DECLARE(apr_table_t *)ap_cache_cacheable_hdrs_out(apr_pool_t *p,
+ apr_table_t *t,
+ server_rec *s)
+{
+ return ap_cache_cacheable_headers(p,t,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_headers_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_headers_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: modules/cache/mod_cache.c
===================================================================
--- modules/cache/mod_cache.c (revision 649044)
+++ modules/cache/mod_cache.c (working copy)
@@ -756,10 +756,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_headers(r->pool, r-
>headers_out,
r->server);
apr_table_clear(r->err_headers_out);
Index: modules/cache/mod_mem_cache.c
===================================================================
--- modules/cache/mod_mem_cache.c (revision 649044)
+++ modules/cache/mod_mem_cache.c (working copy)
@@ -604,17 +604,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_headers_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 */
Index: modules/cache/mod_disk_cache.c
===================================================================
--- modules/cache/mod_disk_cache.c (revision 649044)
+++ modules/cache/mod_disk_cache.c (working copy)
@@ -937,17 +937,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_headers_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) {
ap_log_error(APLOG_MARK, APLOG_WARNING, rv, r->server,
@@ -962,8 +953,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_header_in(r);
+
rv = store_table(dobj->hfd, headers_in);
if (rv != APR_SUCCESS) {
ap_log_error(APLOG_MARK, APLOG_WARNING, rv, r->server,
Index: include/ap_mmn.h
===================================================================
--- include/ap_mmn.h (revision 649044)
+++ include/ap_mmn.h (working copy)
@@ -156,6 +156,8 @@
* 20080403.1 (2.3.0-dev) Add authn/z hook and provider
registration wrappers.
* 20080403.2 (2.3.0-dev) Add ap_escape_path_segment_buffer() and
ap_unescape_all().
* 20080407.0 (2.3.0-dev) Remove ap_graceful_stop_signalled.
+ * 20080407.1 Deprecate ap_cache_cacheable_hdrs_out and
add two
+ * generalized ap_cache_cacheable_headers_(in|
out).
*/
#define MODULE_MAGIC_COOKIE 0x41503234UL /* "AP24" */
@@ -163,7 +165,7 @@
#ifndef MODULE_MAGIC_NUMBER_MAJOR
#define MODULE_MAGIC_NUMBER_MAJOR 20080407
#endif
-#define MODULE_MAGIC_NUMBER_MINOR 0 /* 0...n */
+#define MODULE_MAGIC_NUMBER_MINOR 1 /* 0...n */
/**
* Determine if the server's current MODULE_MAGIC_NUMBER is at least a
Re: Centralizing http-proxy knowledge in cache_util.c
Posted by Dirk-Willem van Gulik <di...@webweaving.org>.
On Apr 17, 2008, at 2:49 PM, Plüm, Rüdiger, VF-Group wrote:
>
>
>> -----Ursprüngliche Nachricht-----
>> Von: Dirk-Willem van Gulik
>> Gesendet: Donnerstag, 17. April 2008 14:19
>> An: dev@httpd.apache.org
>> Betreff: Re: Centralizing http-proxy knowledge in cache_util.c
>>
>>
>> On Apr 17, 2008, at 1:50 PM, Plüm, Rüdiger, VF-Group wrote:
>>
>>> What is this function (ap_cache_cacheable_hdrs)? I am
>> trying to get
>>> confused
>>> by all these functions which sound very similar :-).
>>
>>
>> Basically I try to rationalize it to:
>>
>> 1) ap_cache_cacheable_hdrs
>>
>> Understands the general Hop by Hop rules - i.e. which headers
>> are cachable and which are not - regardless of 'direction'.
>>
>> Could be renamed to ap_cache_cacheable_headers if we break
>> the old naming convention a little.
>
> Which you already had done in your patch. Hence the confusion :-).
Ah Apologies - that _is_ confusing :)
Dw.
Re: Centralizing http-proxy knowledge in cache_util.c
Posted by Plüm,
Rüdiger,
VF-Group <ru...@vodafone.com>.
> -----Ursprüngliche Nachricht-----
> Von: Dirk-Willem van Gulik
> Gesendet: Donnerstag, 17. April 2008 14:19
> An: dev@httpd.apache.org
> Betreff: Re: Centralizing http-proxy knowledge in cache_util.c
>
>
> On Apr 17, 2008, at 1:50 PM, Plüm, Rüdiger, VF-Group wrote:
>
> > What is this function (ap_cache_cacheable_hdrs)? I am
> trying to get
> > confused
> > by all these functions which sound very similar :-).
>
>
> Basically I try to rationalize it to:
>
> 1) ap_cache_cacheable_hdrs
>
> Understands the general Hop by Hop rules - i.e. which headers
> are cachable and which are not - regardless of 'direction'.
>
> Could be renamed to ap_cache_cacheable_headers if we break
> the old naming convention a little.
Which you already had done in your patch. Hence the confusion :-).
Regards
Rüdiger
Re: Centralizing http-proxy knowledge in cache_util.c
Posted by Dirk-Willem van Gulik <di...@webweaving.org>.
On Apr 17, 2008, at 1:50 PM, Plüm, Rüdiger, VF-Group wrote:
> What is this function (ap_cache_cacheable_hdrs)? I am trying to get
> confused
> by all these functions which sound very similar :-).
Basically I try to rationalize it to:
1) ap_cache_cacheable_hdrs
Understands the general Hop by Hop rules - i.e. which headers
are cachable and which are not - regardless of 'direction'.
Could be renamed to ap_cache_cacheable_headers if we break
the old naming convention a little.
2) ap_cache_cacheable_headers_out
3) ap_cache_cacheable_headers_in
Understand the above -AND- the rules for outbound and
inbound headers.
Right now there are no extra outbound rules yet (it defers to
the general i/o case). But I expect thoseto be there the moment we
bring the mem-cache and memcached into the fold. And I also expect
the other one to get a bit more complex when we start doing
error pages proper (e.g. when there is a proxypass).
Thanks,
Dw.
Re: Centralizing http-proxy knowledge in cache_util.c
Posted by Plüm,
Rüdiger,
VF-Group <ru...@vodafone.com>.
> -----Ursprüngliche Nachricht-----
> Von: Dirk-Willem van Gulik
> Gesendet: Donnerstag, 17. April 2008 13:05
> An: dev@httpd.apache.org
> Betreff: Centralizing http-proxy knowledge in cache_util.c
>
> Before I give the other modules a pass over -- does below strawman
> address everyones
> concerns w.r. to the API changes ?
>
> To recap:
>
> - ap_cache_cacheable_hdrs_out gets depricated. And the repliced
> code is to be removed from mod_*_cache.
>
> - a ap_cache_cacheable_hdrs_in() and ap_cache_cacheable_hdrs_out()
> are added, which each rely on:
>
> - ap_cache_cacheable_headers - which knows the HTTP rules.
>
> The intention is to make sure that caching modules do no
> longer have to
> understand the section in RFC 2616 deling with hop-by-hop.
>
> Right now _out is just a wrapper for ap_cache_cacheable_headers - but
> I expect that we'll have to populate it soon. And to avoid an API
> version
> bump I'd thus rather not do it as a #define for now.
>
> Thanks,
>
> Dw.
>
> Index: modules/cache/mod_cache.h
> ===================================================================
> --- modules/cache/mod_cache.h (revision 649044)
> +++ modules/cache/mod_cache.h (working copy)
> @@ -614,3 +623,44 @@
> }
> return headers_out;
> }
> +
> +/* Legacy call - functionally equivalent to
> ap_cache_cacheable_headers.
> + * @deprecated @see ap_cache_cacheable_headers
> + */
> +CACHE_DECLARE(apr_table_t
> *)ap_cache_cacheable_hdrs_out(apr_pool_t *p,
> +
> apr_table_t *t,
> +
> server_rec *s)
> +{
> + return ap_cache_cacheable_headers(p,t,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_headers_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_headers_out(request_rec * r)
> +{
> + apr_table_t *headers_out;
> +
> + headers_out = ap_cache_cacheable_hdrs(r->pool, r->headers_out,
> + r->server);
What is this function (ap_cache_cacheable_hdrs)? I am trying to get confused
by all these functions which sound very similar :-).
> +
> + 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;
> +}
Regards
Rüdiger
Re: Centralizing http-proxy knowledge in cache_util.c (resend with fix)
Posted by Dirk-Willem van Gulik <di...@webweaving.org>.
(Resent - previous patch also contained the module changes).
Before I give the other modules a pass over -- does below strawman
address everyones
concerns w.r. to the API changes ?
To recap:
- ap_cache_cacheable_hdrs_out gets depricated. And the repliced
code is to be removed from mod_*_cache.
- a ap_cache_cacheable_hdrs_in() and ap_cache_cacheable_hdrs_out()
are added, which each rely on:
- ap_cache_cacheable_headers - which knows the HTTP rules.
The intention is to make sure that caching modules do no longer have to
understand the section in RFC 2616 deling with hop-by-hop.
Right now _out is just a wrapper for ap_cache_cacheable_headers - but
I expect that we'll have to populate it soon. And to avoid an API
version
bump I'd thus rather not do it as a #define for now.
Thanks,
Dw.
Index: CHANGES
===================================================================
--- CHANGES (revision 649044)
+++ CHANGES (working copy)
@@ -2,6 +2,14 @@
Changes with Apache 2.3.0
[ When backported to 2.2.x, remove entry from this file ]
+ *) cache: remove ap_cache_cacheable_hdrs_out() which was used
+ for both in- and out-put headers; and replace it by a single
+ ap_cache_cacheable_headers() wrapped in a in- and out-put
+ specific ap_cache_cacheable_hdrs_in()/out(). The latter
+ which will also merge error and ensure content-type. To keep
+ cache modules consistent with ease. This API change bumps
+ up the minor MM by one [Dirk-Willem van Gulik].
+
*) mod_rewrite: Allow Cookie option to set secure and HttpOnly flags.
PR 44799 [Christian Wenz <christian wenz.org>]
Index: modules/cache/mod_cache.h
===================================================================
--- modules/cache/mod_cache.h (revision 649044)
+++ modules/cache/mod_cache.h (working copy)
@@ -286,9 +286,27 @@
const char *key, char **val);
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
+/* Create a new table consisting of those elements from an
+ * headers table that are allowed to be stored in a cache.
*/
+CACHE_DECLARE(apr_table_t *)ap_cache_cacheable_headers(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_headers_in(request_rec
* r);
+
+/* 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_headers_out(request_rec * r);
+
+/* Legacy call - functionally equivalent to ap_cache_cacheable_headers.
+ * @deprecated @see ap_cache_cacheable_headers
+ */
CACHE_DECLARE(apr_table_t *)ap_cache_cacheable_hdrs_out(apr_pool_t
*pool,
apr_table_t *t,
server_rec *s);
Index: modules/cache/cache_util.c
===================================================================
--- modules/cache/cache_util.c (revision 649044)
+++ modules/cache/cache_util.c (working copy)
@@ -577,10 +577,10 @@
return apr_pstrdup(p, hashfile);
}
-/* Create a new table consisting of those elements from an input
+/* Create a new table consisting of those elements from an
* 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_headers(apr_pool_t
*pool,
apr_table_t *t,
server_rec *s)
{
@@ -588,12 +588,20 @@
char **header;
int i;
+ /* Short circuit the common case that there are not
+ * (yet) any headers populated.
+ */
+ if (t == NULL) {
+ return apr_table_make(pool, 10);
+ };
+
/* 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
*/
apr_table_t *headers_out;
headers_out = apr_table_copy(pool, t);
+
apr_table_unset(headers_out, "Connection");
apr_table_unset(headers_out, "Keep-Alive");
apr_table_unset(headers_out, "Proxy-Authenticate");
@@ -605,6 +613,7 @@
conf = (cache_server_conf *)ap_get_module_config(s->module_config,
&cache_module);
+
/* Remove the user defined headers set with CacheIgnoreHeaders.
* This may break RFC 2616 compliance on behalf of the
administrator.
*/
@@ -614,3 +623,44 @@
}
return headers_out;
}
+
+/* Legacy call - functionally equivalent to ap_cache_cacheable_headers.
+ * @deprecated @see ap_cache_cacheable_headers
+ */
+CACHE_DECLARE(apr_table_t *)ap_cache_cacheable_hdrs_out(apr_pool_t *p,
+ apr_table_t *t,
+ server_rec *s)
+{
+ return ap_cache_cacheable_headers(p,t,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_headers_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_headers_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: include/ap_mmn.h
===================================================================
--- include/ap_mmn.h (revision 649044)
+++ include/ap_mmn.h (working copy)
@@ -156,6 +156,8 @@
* 20080403.1 (2.3.0-dev) Add authn/z hook and provider registration
wrappers.
* 20080403.2 (2.3.0-dev) Add ap_escape_path_segment_buffer() and
ap_unescape_all().
* 20080407.0 (2.3.0-dev) Remove ap_graceful_stop_signalled.
+ * 20080407.1 Deprecate ap_cache_cacheable_hdrs_out and
add two
+ * generalized ap_cache_cacheable_headers_(in|
out).
*/
#define MODULE_MAGIC_COOKIE 0x41503234UL /* "AP24" */
@@ -163,7 +165,7 @@
#ifndef MODULE_MAGIC_NUMBER_MAJOR
#define MODULE_MAGIC_NUMBER_MAJOR 20080407
#endif
-#define MODULE_MAGIC_NUMBER_MINOR 0 /* 0...n */
+#define MODULE_MAGIC_NUMBER_MINOR 1 /* 0...n */
/**
* Determine if the server's current MODULE_MAGIC_NUMBER is at least a