forget quick_handler, make stuff cachable anyways

2002-07-31 Thread Brian Degenhardt

Just wanted to voice my opinion here.  I do a lot of work with caching
proxies and it would be really nice to have apache be intelligent to
how things are cached.  There's two types of changes that would really
help caches and proxies, internal and external to apache:

1) Make apache modules add apropriate headers to inform caches of the
   cacheability of objects.  Does something vary based on cookies? put
   a Vary: cookie in the outgoing headers.  Are we using an AAA
   plugin, put Cache-Control: private in the headers.  This would
   really make the complex behavoir of apache modules easier to deal
   with for downstream caches.  I think it would be a mistake to use
   apache-specific mechanisms such as looking at the filter stack to
   try to figure out this information.  Let's use HTTP for it.

2) Make mod_cache aware of the stuff set by (1).  This doesn't have to
   be intelligent.  Squid doesn't cache anything with a Vary header in
   it.  Sounds good to me.  Note that moving mod_cache forward in the
   request cycle only satisfies situations where your modules are
   running inside of apache.  If the mod_cache instance is running on
   a separate proxy machine downstream from the machine running the
   AAA plugin, you're screwed unless the correct headers are set and
   read.

Now, I haven't looked at all of the code to make sure that these
things are satisfied.  I know that mod_rewrite does a good job of
setting the Vary header based on rewrite rules but that's the extent
of my knowledge.  I just wanted to get this in everyone's mind as a
valuable goal.  If you guys want, I can audit the existing plugins to
see how they behave along these lines.

cheers

-bmd




Re: quick_handler hook is completely bogus.

2002-07-30 Thread Brian Degenhardt

I'm approaching this from a caching perspective, so when a module uses
quick_handler for non-caching mechanisms, my comments do not apply but
here's an option:

What if modules were required to set the Vary: header appropriately
and have mod_cache_* honor it?  This way, you're caching problem is
fixed for not only stuff that quick_handler screws up but for stuff
that any downstream proxy screws up.  If you're module absosmurfly has
to run on every request, do Vary: * and you're problem is solved.

-bmd

On Tue, Jul 30, 2002 at 12:40:56PM -0700, Ryan Bloom wrote:
 
 I realize that this is a strong statement, but I believe that I can back
 it up.  My reasons for not liking this hook at all:
 
 1)  If I have a page that I have served and it gets put in the cache,
 then it will be served out of the quick_handler phase.  However, if I
 then add or modify a .htaccess file to deny access to that page, then my
 changes won't be honored until the page expires from the cache.  This is
 a security hole, because I don't know of anyway to invalidate cached
 pages.  (This one if from a conversation with wrowe).  [ I guess it
 might be possible to clear the cache with a graceful restart. ]
 
 2)  If I have a page that uses access checking to ensure that only
 certain people can request the page, the cache_filter will put it in the
 quick handler.  However, the page may not be allowed to people who will
 request it from the cache.  I may be wrong about this one, but I see how
 the cache disallows pages that require authentication.  I do not see how
 it can disallow caching of pages that require access_checking.
 
 3)  It isn't possible for a module author to circumvent the
 quick_handler phase.  If I write a module that doesn't want to allow the
 quick_handler phase, for security reasons, I can't enforce it.
 
 While I understand that we are giving people a lot of rope and asking
 them to use it wisely, this phase gives too much rope, and invites
 people to hang themselves.
 
 I believe that this hook should be removed, and all content should be
 served out of the handler phase.  If we are looking to remove some
 request phases, then we should make it possible to avoid individual
 phases when serving requests, not completely skip all of them.
 
 Ryan
 
 --
 Ryan Bloom
 [EMAIL PROTECTED]   [EMAIL PROTECTED]
 
 



Re: [PATCH] mod_rewrite and cookie setting

2002-07-17 Thread Brian Degenhardt

On Tue, Jul 16, 2002 at 12:15:41PM -0700, Adam Sussman wrote:
 On Tue, Jul 16, 2002 at 10:26:49AM -0700, Ian Holsman wrote:
  Adam Sussman wrote:
   The new cookie setting feature of mod_rewrite adds the Set-Cookie header
   to r-headers_out.  Shouldn't this be r-err_headers_out instead?
   
   The error headers are always present whereas the the normal headers do not
   appear under error conditions.  In applications where I have an apache
   module setting cookies, I have always found that setting err_headers_out
   gives me the complete coverage that I want.
   
   Thoughts?
  yep.. a couple of them
  the original patch has err_headers_out and it didn't work as we would 
  get multiple cookies back on a simple request on GET / on a standard 
  install.
  
 
 hmm... I cannot reproduce this behaviour.  So far as I can see, the only
 difference is whether or not the cookie header appears in non-200 reponses.
 Can you show me the configuration you used?

Here's an example that will trigger the same cookie being set twice
when the cookies are in err_headers_out:

RewriteRule ^(.*)$ - [CO=MYCOOKIE:$1:.apache.org]

If you used this and requested / it would get an internal redirecto to
/index.html, therefore you'd get two SetCookie headers, one for
MYCOOKIE=/ and another for MYCOOKIE=/index.html
This could be a problem for you.

However, if it's not in err_headers_out there's even a bigger problem.
mod_rewrite uses internal redirects for rewrite rules that are placed
in Directory and Location tags because the directory_walk and
location_walk phases execute after the translate_name phase.
Therefore, if you don't put the cookie in err_headers_out, it will get
set in r-main, then the internal_redirect issues and the cookie never
gets sent to the requestor.

So we're screwed either way.  One way I've thought of to fix this is
to alter mod_rewrite so that it translates the name in the
map_to_storage phase for rules inside of Directory and Location
sections.  I haven't tried this though.  Any thoughts?

-bmd



[PATCH] mod_example.c better comments

2002-06-14 Thread Brian Degenhardt

Some of the comments were out of order or incomplete in
mod_exmple.c.  It was also missing ap_hook_create_request()

This brings it up to date a bit.

Sorry if this is kinda trivial, I'm probably going to generate a lot
more silly fixes like this.  Let me know if there's an optimal
way/time for stuff like this.

-bmd

--- mod_example.c.old   Fri Jun 14 15:54:56 2002
+++ mod_example.c   Fri Jun 14 17:15:21 2002
 -657,31 +657,6 
 /* see the individual handler comments below for details.   */
 /*  */
 /*--*/
-/* 
- * This function is called during server initialisation.  Any information
- * that needs to be recorded must be in static cells, since there's no
- * configuration record.
- *
- * There is no return value.
- */
-
-/* 
- * This function is called when an heavy-weight process (such as a child) is
- * being run down or destroyed.  As with the child initialisation function,
- * any information that needs to be recorded must be in static cells, since
- * there's no configuration record.
- *
- * There is no return value.
- */
-
-/* 
- * This function is called during server initialisation when an heavy-weight
- * process (such as a child) is being initialised.  As with the
- * module initialisation function, any information that needs to be recorded
- * must be in static cells, since there's no configuration record.
- *
- * There is no return value.
- */
 
 /*
  * This function gets called to create a per-directory configuration
 -844,8 +819,14 
 }
 
 /*
- * This routine is called before the server processes the configuration
- * files.  There is no return value.
+ * This routine was registered by ap_hook_pre_config()
+ *
+ * It is called before the server processes the configuration files
+ * during server initialisation.  Any information that needs to be
+ * recorded must be in static cells, since there's no configuration
+ * record.
+ *
+ * The return value must be OK or apache will not start up.
  */
 static int x_pre_config(apr_pool_t *pconf, apr_pool_t *plog,
 apr_pool_t *ptemp)
 -859,12 +840,11 
 }
 
 /*
- * This routine is called to perform any module-specific fixing of header
- * fields, et cetera.  It is invoked just before any content-handler.
+ * This routine is registered by ap_hook_post_config()
+ * It is called just after the server processes the configuration
+ * files.  It is mostly analogous to old apache _init routines.
  *
- * The return value is OK, DECLINED, or HTTP_mumble.  If we return OK, the
- * server will still call any remaining modules with an handler for this
- * phase.
+ * The return value must be OK or apache will not start up.
  */
 static int x_post_config(apr_pool_t *pconf, apr_pool_t *plog,
   apr_pool_t *ptemp, server_rec *s)
 -877,12 +857,13 
 }
 
 /*
- * This routine is called to perform any module-specific fixing of header
- * fields, et cetera.  It is invoked just before any content-handler.
+ * This function is registered by ap_hook_open_logs()
+ * It is responsible for opening any logfiles. It is assumed that if
+ * you are going to fail the open_logs phase, then you will print out
+ * your own message that explains what has gone wrong.  The server
+ * doesn't need to do that for you.
  *
- * The return value is OK, DECLINED, or HTTP_mumble.  If we return OK, the
- * server will still call any remaining modules with an handler for this
- * phase.
+ * This function must return OK or apache will not start up.
  */
 static int x_open_logs(apr_pool_t *pconf, apr_pool_t *plog,
 apr_pool_t *ptemp, server_rec *s)
 -895,7 +876,16 
 }
 
 /*
- * All our process-death routine does is add its trace to the log.
+ * This is the cleanup routine registered by
+ * apr_pool_cleanup_register() which executes when when an
+ * heavy-weight process (such as a child) is being run down or
+ * destroyed.  As with the child initialisation function, any
+ * information that needs to be recorded must be in static cells,
+ * since there's no configuration record.  Note that it is not
+ * registered in x_register_hooks but inside of x_child_init. 
+ *
+ * This function should return APR_SUCCESS on success.  On error,
+ * see the apr docs for appropriate errorcodes, or use APR_EGENERAL
  */
 static apr_status_t x_child_exit(void *data)
 {
 -914,7 +904,13 
 }
 
 /*
- * All our process initialiser does is add its trace to the log.
+ * This function is registered by ap_hook_child_init()
+ * It is called during server initialisation when an heavy-weight
+ * process (such as a child) is being initialised.  As with the module
+ * initialisation function, any information that needs to be recorded
+ * must be in static cells, since there's no configuration record.
+ *
+ * Note that it registers a cleanup function
  */
 static void x_child_init(apr_pool_t *p, 

questions from my trivial patch

2002-06-14 Thread Brian Degenhardt

The functions for ap_hook_open_logs, ap_hook_post_config, and
ap_hook_pre_config will fail if DECLINE is returned.  Is that
intentional?

In the quick_handler, returning HTTP_mumble will result in that status
being returned in a real request, but it will fallback to the real
handler in a subrequest.  Is that intentional?

What's the difference between adding your filters inside your
ap_hook_insert_filter hook versus calling ap_register_*_filter right
inside of the register_hooks callback?

-bmd