Re: CacheIgnoreHeaders not working correctly

2009-02-11 Thread Lars Eilebrecht 
Ruediger Pluem wrote:

 http://svn.apache.org/viewvc?view=revrevision=649162
 http://svn.apache.org/viewvc?view=revrevision=649791

Hmm ... I'm not sure I understand the logic in this one:

CACHE_DECLARE(apr_table_t *)ap_cache_cacheable_headers_out(request_rec * r)
{
apr_table_t *headers_out;

headers_out = apr_table_overlay(r-pool, r-headers_out,
r-err_headers_out);

apr_table_clear(r-err_headers_out);

headers_out = ap_cache_cacheable_headers(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));
}

return headers_out;
}
 

It merges r-headers_out and r-err_headers_out into headers_out,
then clears r-err_headers_out (?), and then ap_cache_cacheable_headers()
gets called with r-headers_out instead of headers_out?

ciao...
-- 
Lars Eilebrecht
l...@eilebrecht.net

Re: CacheIgnoreHeaders not working correctly

2009-02-11 Thread Ruediger Pluem 


On 02/11/2009 02:10 PM, Lars Eilebrecht wrote:
 Ruediger Pluem wrote:
 
 http://svn.apache.org/viewvc?view=revrevision=649162
 http://svn.apache.org/viewvc?view=revrevision=649791
 
 Hmm ... I'm not sure I understand the logic in this one:
 
 CACHE_DECLARE(apr_table_t *)ap_cache_cacheable_headers_out(request_rec * r)
 {
 apr_table_t *headers_out;
 
 headers_out = apr_table_overlay(r-pool, r-headers_out,
 r-err_headers_out);
 
 apr_table_clear(r-err_headers_out);
 
 headers_out = ap_cache_cacheable_headers(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));
 }
 
 return headers_out;
 }
  
 
 It merges r-headers_out and r-err_headers_out into headers_out,
 then clears r-err_headers_out (?), and then ap_cache_cacheable_headers()
 gets called with r-headers_out instead of headers_out?

Missed this one :-).

http://svn.apache.org/viewvc?view=revrevision=649793

Regards

Rüdiger

Re: CacheIgnoreHeaders not working correctly

2009-02-10 Thread Lars Eilebrecht 
Lars Eilebrecht wrote:

[...]
 So it copies r-headers_out to the local headers_out variable, and
 removes all unwanted headers. However, then r-err_headers_out
 gets merged into headers_out which is then stored in the cache.
 
 Is there a reason why this is done? This could lead to quite a
 number of headers being stored in the cache such as Set-Cookie.
 Which happens in my case as the custom module operates on
 r-err_headers_out.
 
 So a potential fix would be to merge r-headers_out and
 r-err_headers_out into the local headers_out variable, then
 filter the unwanted headers, and store the result.
 
 This seems to work, but maybe I'm missing something.

Anyone any comments about this patch?
It fixes the issue, but I'm not 100% if I may be missing something
regarding the handling of err_headers_out in mod_disk_cache.

--snip--

--- mod_disk_cache.c.orig   2009-02-10 11:08:41.0 +
+++ mod_disk_cache.c2009-02-10 10:47:48.0 +
@@ -912,7 +912,9 @@
 if (r-headers_out) {
 apr_table_t *headers_out;
 
-headers_out = ap_cache_cacheable_hdrs_out(r-pool, r-headers_out,
+headers_out = apr_table_overlay(r-pool, r-headers_out,
+r-err_headers_out);
+headers_out = ap_cache_cacheable_hdrs_out(r-pool, headers_out,
   r-server);
 
 if (!apr_table_get(headers_out, Content-Type)
@@ -921,8 +923,6 @@
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;

--snip--

ciao...
-- 
Lars Eilebrecht
l...@eilebrecht.net

Re: CacheIgnoreHeaders not working correctly

2009-02-10 Thread Ruediger Pluem 


On 02/09/2009 09:21 PM, Lars Eilebrecht wrote:
 Hi,
 
 I have a question about the header handling logic of
 mod_cache/mod_disk_cache.
 
 With an installation running mod_disk_cache and a custom module
 which fiddles with Cookie and Set-Cookie headers I am running into
 the problem that mod_disk_cache was storing Set-Cookie headers
 in the cache. It is ignoring the CacheIgnoreHeaders Set-Cookie.
 
 In mod_disk_cache's store_header() function we have this code:
 
 apr_table_t *headers_out;
 
 headers_out = ap_cache_cacheable_hdrs_out(r-pool, r-headers_out,
   r-server);
 [...]
 headers_out = apr_table_overlay(r-pool, headers_out,
 r-err_headers_out);
 rv = store_table(dobj-hfd, headers_out);
 
 So it copies r-headers_out to the local headers_out variable, and
 removes all unwanted headers. However, then r-err_headers_out
 gets merged into headers_out which is then stored in the cache.
 
 Is there a reason why this is done? This could lead to quite a
 number of headers being stored in the cache such as Set-Cookie.
 Which happens in my case as the custom module operates on
 r-err_headers_out.
 
 So a potential fix would be to merge r-headers_out and
 r-err_headers_out into the local headers_out variable, then
 filter the unwanted headers, and store the result.
 
 This seems to work, but maybe I'm missing something.

Have a look at

http://svn.apache.org/viewvc?view=revrevision=649162
http://svn.apache.org/viewvc?view=revrevision=649791

Regards

Rüdiger

Re: CacheIgnoreHeaders not working correctly

2009-02-10 Thread Graham Leggett 

Ruediger Pluem wrote:


Have a look at

http://svn.apache.org/viewvc?view=revrevision=649162
http://svn.apache.org/viewvc?view=revrevision=649791


This needed an MMN bump though, so it won't work for v2.2. :(

Regards,
Graham
--


smime.p7s
Description: S/MIME Cryptographic Signature

CacheIgnoreHeaders not working correctly

2009-02-09 Thread Lars Eilebrecht 
Hi,

I have a question about the header handling logic of
mod_cache/mod_disk_cache.

With an installation running mod_disk_cache and a custom module
which fiddles with Cookie and Set-Cookie headers I am running into
the problem that mod_disk_cache was storing Set-Cookie headers
in the cache. It is ignoring the CacheIgnoreHeaders Set-Cookie.

In mod_disk_cache's store_header() function we have this code:

apr_table_t *headers_out;

headers_out = ap_cache_cacheable_hdrs_out(r-pool, r-headers_out,
  r-server);
[...]
headers_out = apr_table_overlay(r-pool, headers_out,
r-err_headers_out);
rv = store_table(dobj-hfd, headers_out);

So it copies r-headers_out to the local headers_out variable, and
removes all unwanted headers. However, then r-err_headers_out
gets merged into headers_out which is then stored in the cache.

Is there a reason why this is done? This could lead to quite a
number of headers being stored in the cache such as Set-Cookie.
Which happens in my case as the custom module operates on
r-err_headers_out.

So a potential fix would be to merge r-headers_out and
r-err_headers_out into the local headers_out variable, then
filter the unwanted headers, and store the result.

This seems to work, but maybe I'm missing something.

ciao...
-- 
Lars Eilebrecht
l...@eilebrecht.net