You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Allan Edwards <ak...@meepzor.com> on 2001/01/08 20:03:56 UTC

windows build & handler hook

A number of modules need updating for the
handler hook in order to get windows building 
again. I will be committing the following patch 
shortly (after some additional testing).

Allan
--------------------------------------------------------------------------

Index: libhttpd.def
===================================================================
RCS file: /home/cvs/httpd-2.0/libhttpd.def,v
retrieving revision 1.49
diff -u -d -b -r1.49 libhttpd.def
--- libhttpd.def	2001/01/04 22:37:27	1.49
+++ libhttpd.def	2001/01/08 18:43:08
@@ -376,6 +376,7 @@
         ap_hook_post_config
         ap_hook_open_logs
         ap_hook_child_init
+        ap_hook_handler
 
         ap_get_status_table
         ap_run_default_port
Index: modules/aaa/mod_auth_anon.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/aaa/mod_auth_anon.c,v
retrieving revision 1.19
diff -u -d -b -r1.19 mod_auth_anon.c
--- modules/aaa/mod_auth_anon.c	2000/12/19 15:09:00	1.19
+++ modules/aaa/mod_auth_anon.c	2001/01/08 18:43:31
@@ -308,6 +308,5 @@
     NULL,			/* server config */
     NULL,			/* merge server config */
     anon_auth_cmds,		/* command apr_table_t */
-    NULL,			/* handlers */
     register_hooks		/* register hooks */
 };
Index: modules/aaa/mod_auth_db.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/aaa/mod_auth_db.c,v
retrieving revision 1.20
diff -u -d -b -r1.20 mod_auth_db.c
--- modules/aaa/mod_auth_db.c	2000/12/19 19:44:16	1.20
+++ modules/aaa/mod_auth_db.c	2001/01/08 18:43:31
@@ -409,7 +409,6 @@
     NULL,			/* server config */
     NULL,			/* merge server config */
     db_auth_cmds,		/* command apr_table_t */
-    NULL,			/* handlers */
     register_hooks		/* register hooks */
 };
 
Index: modules/aaa/mod_auth_dbm.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/aaa/mod_auth_dbm.c,v
retrieving revision 1.22
diff -u -d -b -r1.22 mod_auth_dbm.c
--- modules/aaa/mod_auth_dbm.c	2000/12/19 15:09:01	1.22
+++ modules/aaa/mod_auth_dbm.c	2001/01/08 18:43:32
@@ -349,6 +349,5 @@
     NULL,			/* server config */
     NULL,			/* merge server config */
     dbm_auth_cmds,		/* command apr_table_t */
-    NULL,			/* handlers */
     register_hooks              /* register hooks */
 };
Index: modules/aaa/mod_auth_digest.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/aaa/mod_auth_digest.c,v
retrieving revision 1.27
diff -u -d -b -r1.27 mod_auth_digest.c
--- modules/aaa/mod_auth_digest.c	2000/12/19 15:09:02	1.27
+++ modules/aaa/mod_auth_digest.c	2001/01/08 18:43:32
@@ -2070,7 +2070,6 @@
     NULL,			/* server config */
     NULL,			/* merge server config */
     digest_cmds,		/* command table */
-    NULL,			/* handlers */
     register_hooks		/* register hooks */
 };
 
Index: modules/cache/mod_file_cache.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/cache/mod_file_cache.c,v
retrieving revision 1.34
diff -u -d -b -r1.34 mod_file_cache.c
--- modules/cache/mod_file_cache.c	2000/12/13 13:22:51	1.34
+++ modules/cache/mod_file_cache.c	2001/01/08 18:43:33
@@ -416,12 +416,16 @@
     return OK;
 }
 
-static int file_cache_handler(request_rec *r) 
+static int file_cache_handler(const char *handler, request_rec *r) 
 {
     a_file *match;
     int errstatus;
     int rc = OK;
 
+    if (strcmp(handler, "*.*")) {
+        return DECLINED;
+    }
+
     /* we don't handle anything but GET */
     if (r->method_number != M_GET) return DECLINED;
 
@@ -473,6 +477,7 @@
 
 static void register_hooks(void)
 {
+    ap_hook_handler(file_cache_handler, NULL, NULL, AP_HOOK_MIDDLE);
     ap_hook_post_config(file_cache_post_config, NULL, NULL, AP_HOOK_MIDDLE);
     ap_hook_translate_name(file_cache_xlat, NULL, NULL, AP_HOOK_MIDDLE);
     /* This trick doesn't work apparently because the translate hooks
@@ -483,12 +488,6 @@
 
 }
 
-static const handler_rec file_cache_handlers[] =
-{
-    { "*/*", file_cache_handler },
-    { NULL }
-};
-
 module AP_MODULE_DECLARE_DATA file_cache_module =
 {
     STANDARD20_MODULE_STUFF,
@@ -497,6 +496,5 @@
     create_server_config,     /* create per-server config structure */
     NULL,                     /* merge per-server config structures */
     file_cache_cmds,          /* command handlers */
-    file_cache_handlers,      /* handlers */
     register_hooks            /* register hooks */
 };
Index: modules/dav/fs/mod_dav_fs.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/dav/fs/mod_dav_fs.c,v
retrieving revision 1.10
diff -u -d -b -r1.10 mod_dav_fs.c
--- modules/dav/fs/mod_dav_fs.c	2000/10/14 18:24:33	1.10
+++ modules/dav/fs/mod_dav_fs.c	2001/01/08 18:43:37
@@ -138,6 +138,5 @@
     dav_fs_create_server_config,	/* server config */
     dav_fs_merge_server_config,	/* merge server config */
     dav_fs_cmds,		/* command table */
-    NULL,                       /* handlers */
     register_hooks,             /* register hooks */
 };
Index: modules/dav/main/mod_dav.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/dav/main/mod_dav.c,v
retrieving revision 1.33
diff -u -d -b -r1.33 mod_dav.c
--- modules/dav/main/mod_dav.c	2000/11/27 12:54:21	1.33
+++ modules/dav/main/mod_dav.c	2001/01/08 18:43:39
@@ -3784,10 +3784,14 @@
 /*
  * Response handler for DAV resources
  */
-static int dav_handler(request_rec *r)
+static int dav_handler(const char *handler, request_rec *r)
 {
     dav_dir_conf *conf;
 
+    if (strcmp(handler, "dav-handler")) {
+	return DECLINED;
+    }
+
     /* quickly ignore any HTTP/0.9 requests */
     if (r->assbackwards) {
 	return DECLINED;
@@ -4004,6 +4008,7 @@
 
 static void register_hooks(void)
 {
+    ap_hook_handler(dav_handler, NULL, NULL, AP_HOOK_MIDDLE);
     ap_hook_post_config(dav_init_handler, NULL, NULL, AP_HOOK_MIDDLE);
     ap_hook_type_checker(dav_type_checker, NULL, NULL, AP_HOOK_FIRST);
 
@@ -4044,12 +4049,6 @@
     { NULL }
 };
 
-static const handler_rec dav_handlers[] =
-{
-    {"dav-handler", dav_handler},
-    { NULL }
-};
-
 module DAV_DECLARE_DATA dav_module =
 {
     STANDARD20_MODULE_STUFF,
@@ -4058,7 +4057,6 @@
     dav_create_server_config,	/* server config */
     dav_merge_server_config,	/* merge server config */
     dav_cmds,			/* command table */
-    dav_handlers,		/* handlers */
     register_hooks,             /* register hooks */
 };
 
Index: modules/generators/mod_status.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/generators/mod_status.c,v
retrieving revision 1.14
diff -u -d -b -r1.14 mod_status.c
--- modules/generators/mod_status.c	2000/10/16 06:05:07	1.14
+++ modules/generators/mod_status.c	2001/01/08 18:43:42
@@ -78,12 +78,16 @@
     return 1;
 }
 
-static int status_handler(request_rec *r)
+static int status_handler(const char *handler, request_rec *r)
 {
     int i;
     apr_array_header_t *server_status;
     ap_status_table_row_t *status_rows;
 
+    if (strcmp(handler, STATUS_MAGIC_TYPE) && strcmp(handler, "server-status")) {
+        return DECLINED;
+    }
+
     r->allowed = (1 << M_GET);
     if (r->method_number != M_GET)
 	return DECLINED;
@@ -120,12 +124,10 @@
     return 0;
 }
 
-static const handler_rec status_handlers[] =
+static void register_hooks(void)
 {
-    {STATUS_MAGIC_TYPE, status_handler},
-    {"server-status", status_handler},
-    {NULL}
-};
+    ap_hook_handler(status_handler, NULL, NULL, AP_HOOK_MIDDLE);
+}
 
 module AP_MODULE_DECLARE_DATA status_module =
 {
@@ -135,6 +137,5 @@
     NULL,			/* server config */
     NULL,			/* merge server config */
     NULL,			/* command table */
-    status_handlers,		/* handlers */
-    NULL                        /* register hooks */
+    register_hooks	/* register hooks */
 };
Index: modules/mappers/mod_rewrite.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/mappers/mod_rewrite.c,v
retrieving revision 1.57
diff -u -d -b -r1.57 mod_rewrite.c
--- modules/mappers/mod_rewrite.c	2001/01/03 16:47:02	1.57
+++ modules/mappers/mod_rewrite.c	2001/01/08 18:43:46
@@ -210,14 +210,9 @@
     { NULL }
 };
 
-    /* the apr_table_t of content handlers we provide */
-static const handler_rec handler_table[] = {
-    { "redirect-handler", handler_redirect },
-    { NULL }
-};
-
 static void register_hooks(void)
 {
+    ap_hook_handler(handler_redirect, NULL, NULL, AP_HOOK_MIDDLE);
     ap_hook_post_config(init_module,NULL,NULL,AP_HOOK_MIDDLE);
     ap_hook_child_init(init_child,NULL,NULL,AP_HOOK_MIDDLE);
 
@@ -234,7 +229,6 @@
    config_server_create,        /* create per-server config structures */
    config_server_merge,         /* merge  per-server config structures */
    command_table,               /* apr_table_t of config file commands  */
-   handler_table,               /* [#8] MIME-typed-dispatched handlers */
    register_hooks               /* register hooks                      */
 };
 
@@ -1626,8 +1620,12 @@
 **
 */
 
-static int handler_redirect(request_rec *r)
+static int handler_redirect(const char *handler, request_rec *r)
 {
+    if (strcmp(handler, "redirect-handler")) {
+        return DECLINED;
+    }
+
     /* just make sure that we are really meant! */
     if (strncmp(r->filename, "redirect:", 9) != 0) {
         return DECLINED;
Index: modules/mappers/mod_so.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/mappers/mod_so.c,v
retrieving revision 1.31
diff -u -d -b -r1.31 mod_so.c
--- modules/mappers/mod_so.c	2001/01/02 18:32:49	1.31
+++ modules/mappers/mod_so.c	2001/01/08 18:43:46
@@ -379,6 +379,5 @@
    so_sconf_create,		/* server config */
    NULL,			    /* merge server config */
    so_cmds,			    /* command apr_table_t */
-   NULL,			    /* handlers */
    NULL				    /* register hooks */
 };
Index: modules/mappers/mod_speling.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/mappers/mod_speling.c,v
retrieving revision 1.23
diff -u -d -b -r1.23 mod_speling.c
--- modules/mappers/mod_speling.c	2001/01/05 19:40:03	1.23
+++ modules/mappers/mod_speling.c	2001/01/08 18:43:46
@@ -564,6 +564,5 @@
     create_mconfig_for_server,  /* server config */
     NULL,                       /* merge server config */
     speling_cmds,               /* command apr_table_t */
-    NULL,                       /* handlers */
     register_hooks		/* register hooks */
 };
Index: modules/metadata/mod_cern_meta.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/metadata/mod_cern_meta.c,v
retrieving revision 1.25
diff -u -d -b -r1.25 mod_cern_meta.c
--- modules/metadata/mod_cern_meta.c	2000/12/01 21:49:25	1.25
+++ modules/metadata/mod_cern_meta.c	2001/01/08 18:43:48
@@ -397,6 +397,5 @@
     NULL,			/* server config */
     NULL,			/* merge server configs */
     cern_meta_cmds,		/* command apr_table_t */
-    NULL,			/* handlers */
     register_hooks		/* register hooks */
 };
Index: modules/metadata/mod_expires.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/metadata/mod_expires.c,v
retrieving revision 1.24
diff -u -d -b -r1.24 mod_expires.c
--- modules/metadata/mod_expires.c	2000/12/01 21:49:25	1.24
+++ modules/metadata/mod_expires.c	2001/01/08 18:43:48
@@ -516,6 +516,5 @@
     NULL,                       /* server config */
     NULL,                       /* merge server configs */
     expires_cmds,               /* command apr_table_t */
-    NULL,                       /* handlers */
     register_hooks		/* register hooks */
 };
Index: modules/metadata/mod_headers.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/metadata/mod_headers.c,v
retrieving revision 1.14
diff -u -d -b -r1.14 mod_headers.c
--- modules/metadata/mod_headers.c	2000/12/01 21:49:25	1.14
+++ modules/metadata/mod_headers.c	2001/01/08 18:43:48
@@ -262,6 +262,5 @@
     create_headers_config,      /* server config */
     merge_headers_config,       /* merge server configs */
     headers_cmds,               /* command apr_table_t */
-    NULL,                       /* handlers */
     register_hooks		/* register hooks */
 };
Index: modules/metadata/mod_usertrack.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/metadata/mod_usertrack.c,v
retrieving revision 1.22
diff -u -d -b -r1.22 mod_usertrack.c
--- modules/metadata/mod_usertrack.c	2000/12/01 21:49:26	1.22
+++ modules/metadata/mod_usertrack.c	2001/01/08 18:43:48
@@ -319,6 +319,5 @@
     make_cookie_log_state,      /* server config */
     NULL,                       /* merge server configs */
     cookie_log_cmds,            /* command apr_table_t */
-    NULL,                       /* handlers */
     register_hooks		/* register hooks */
 };
Index: os/win32/mod_isapi.c
===================================================================
RCS file: /home/cvs/httpd-2.0/os/win32/mod_isapi.c,v
retrieving revision 1.31
diff -u -d -b -r1.31 mod_isapi.c
--- os/win32/mod_isapi.c	2000/11/29 18:50:12	1.31
+++ os/win32/mod_isapi.c	2001/01/08 18:44:00
@@ -345,17 +345,22 @@
     return TRUE;
 }
 
-apr_status_t isapi_handler (request_rec *r)
+apr_status_t isapi_handler (const char *handler, request_rec *r)
 {
-    isapi_server_conf *sconf = ap_get_module_config(r->server->module_config, 
-                                                    &isapi_module);
-    apr_table_t *e = r->subprocess_env;
+    isapi_server_conf * sconf;
+    apr_table_t *e;
     apr_status_t rv;
     isapi_loaded *isa;
     isapi_cid *cid;
     DWORD read;
     int res;
     
+    if(strcmp(handler, "isapi-isa"))
+        return DECLINED;    
+
+    sconf = ap_get_module_config(r->server->module_config, &isapi_module);
+    e = r->subprocess_env;
+
     /* Use similar restrictions as CGIs
      *
      * If this fails, it's pointless to load the isapi dll.
@@ -1262,6 +1267,7 @@
 static void isapi_hooks(void)
 {
     ap_hook_post_config(isapi_post_config, NULL, NULL, AP_HOOK_MIDDLE);
+    ap_hook_handler(isapi_handler, NULL, NULL, AP_HOOK_MIDDLE);
 }
 
 static const command_rec isapi_cmds[] = {
@@ -1278,11 +1284,6 @@
 { NULL }
 };
 
-handler_rec isapi_handlers[] = {
-    { "isapi-isa", isapi_handler },
-    { NULL}
-};
-
 module isapi_module = {
    STANDARD20_MODULE_STUFF,
    NULL,                        /* create per-dir config */
@@ -1290,6 +1291,5 @@
    create_isapi_server_config,  /* server config */
    NULL,                        /* merge server config */
    isapi_cmds,                  /* command apr_table_t */
-   isapi_handlers,              /* handlers */
    isapi_hooks                  /* register hooks */
 };
Index: server/mpm/winnt/mpm_winnt.c
===================================================================
RCS file: /home/cvs/httpd-2.0/server/mpm/winnt/mpm_winnt.c,v
retrieving revision 1.119
diff -u -d -b -r1.119 mpm_winnt.c
--- server/mpm/winnt/mpm_winnt.c	2000/12/21 16:25:44	1.119
+++ server/mpm/winnt/mpm_winnt.c	2001/01/08 18:44:12
@@ -2348,6 +2348,5 @@
     NULL,			/* create per-server config structure */
     NULL,			/* merge per-server config structures */
     winnt_cmds,		        /* command apr_table_t */
-    NULL,			/* handlers */
     winnt_hooks 		/* register_hooks */
 };

Re: windows build & handler hook

Posted by Ben Laurie <be...@algroup.co.uk>.
Greg Stein wrote:
> 
> On Mon, Jan 08, 2001 at 02:03:56PM -0500, Allan Edwards wrote:
> >...
> > --- modules/cache/mod_file_cache.c    2000/12/13 13:22:51     1.34
> > +++ modules/cache/mod_file_cache.c    2001/01/08 18:43:33
> > @@ -416,12 +416,16 @@
> >      return OK;
> >  }
> >
> > -static int file_cache_handler(request_rec *r)
> > +static int file_cache_handler(const char *handler, request_rec *r)
> >  {
> >      a_file *match;
> >      int errstatus;
> >      int rc = OK;
> >
> > +    if (strcmp(handler, "*.*")) {
> > +        return DECLINED;
> > +    }
> > +
> >      /* we don't handle anything but GET */
> >      if (r->method_number != M_GET) return DECLINED;
> >
> > @@ -473,6 +477,7 @@
> >
> >  static void register_hooks(void)
> >  {
> > +    ap_hook_handler(file_cache_handler, NULL, NULL, AP_HOOK_MIDDLE);
> >      ap_hook_post_config(file_cache_post_config, NULL, NULL, AP_HOOK_MIDDLE);
> >      ap_hook_translate_name(file_cache_xlat, NULL, NULL, AP_HOOK_MIDDLE);
> >      /* This trick doesn't work apparently because the translate hooks
> > @@ -483,12 +488,6 @@
> >
> >  }
> >
> > -static const handler_rec file_cache_handlers[] =
> > -{
> > -    { "*/*", file_cache_handler },
> > -    { NULL }
> > -};
> > -
> 
> I think the strcmp() in hook function should just go away. Isn't that what
> "*/*" means? That it applies to all types?

In the old code, yes. My gut feeling is that it should really mean that
it applies to all types with a / in (i.e. all "real" MIME types, but no
"handler" types).

Cheers,

Ben.

--
http://www.apache-ssl.org/ben.html

"There is no limit to what a man can do or how far he can go if he
doesn't mind who gets the credit." - Robert Woodruff

RE: windows build & handler hook

Posted by Allan Edwards <ak...@meepzor.com>.
> More importantly, strcmp() with */* or anything/* or */anything just won't
> work.  We aren't actually comparing with those strings, we are trying to
> match those strings.  Any time a * is found in the handler string, we
> should be using strcmp_match.

Good point I should have spotted that. In this handler we can simply 
eliminate the "if" check since it is */*.
 
> Also, why did this change from "*/*" to "*.*"?
>

Unintentional, typo.  

Allan

Re: windows build & handler hook

Posted by Doug MacEachern <do...@covalent.net>.
i applied alan's patch to my tree and switched handlers back to the old
prototype using r->handler.  looking good, should i commit the lot,
including alan's patch?


Re: windows build & handler hook

Posted by rb...@covalent.net.
> > --- modules/cache/mod_file_cache.c	2000/12/13 13:22:51	1.34
> > +++ modules/cache/mod_file_cache.c	2001/01/08 18:43:33
> > @@ -416,12 +416,16 @@
> >      return OK;
> >  }
> >  
> > -static int file_cache_handler(request_rec *r) 
> > +static int file_cache_handler(const char *handler, request_rec *r) 
> >  {
> >      a_file *match;
> >      int errstatus;
> >      int rc = OK;
> >  
> > +    if (strcmp(handler, "*.*")) {
> > +        return DECLINED;
> > +    }
> > +
> >      /* we don't handle anything but GET */
> >      if (r->method_number != M_GET) return DECLINED;
> >  
> > @@ -473,6 +477,7 @@
> >  
> >  static void register_hooks(void)
> >  {
> > +    ap_hook_handler(file_cache_handler, NULL, NULL, AP_HOOK_MIDDLE);
> >      ap_hook_post_config(file_cache_post_config, NULL, NULL, AP_HOOK_MIDDLE);
> >      ap_hook_translate_name(file_cache_xlat, NULL, NULL, AP_HOOK_MIDDLE);
> >      /* This trick doesn't work apparently because the translate hooks
> > @@ -483,12 +488,6 @@
> >  
> >  }
> >  
> > -static const handler_rec file_cache_handlers[] =
> > -{
> > -    { "*/*", file_cache_handler },
> > -    { NULL }
> > -};
> > -
> 
> I think the strcmp() in hook function should just go away. Isn't that what
> "*/*" means? That it applies to all types?

More importantly, strcmp() with */* or anything/* or */anything just won't
work.  We aren't actually comparing with those strings, we are trying to
match those strings.  Any time a * is found in the handler string, we
should be using strcmp_match.

Also, why did this change from "*/*" to "*.*"?

Ryan

_______________________________________________________________________________
Ryan Bloom                        	rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------


Re: windows build & handler hook

Posted by Greg Stein <gs...@lyra.org>.
On Mon, Jan 08, 2001 at 02:03:56PM -0500, Allan Edwards wrote:
>...
> --- modules/cache/mod_file_cache.c	2000/12/13 13:22:51	1.34
> +++ modules/cache/mod_file_cache.c	2001/01/08 18:43:33
> @@ -416,12 +416,16 @@
>      return OK;
>  }
>  
> -static int file_cache_handler(request_rec *r) 
> +static int file_cache_handler(const char *handler, request_rec *r) 
>  {
>      a_file *match;
>      int errstatus;
>      int rc = OK;
>  
> +    if (strcmp(handler, "*.*")) {
> +        return DECLINED;
> +    }
> +
>      /* we don't handle anything but GET */
>      if (r->method_number != M_GET) return DECLINED;
>  
> @@ -473,6 +477,7 @@
>  
>  static void register_hooks(void)
>  {
> +    ap_hook_handler(file_cache_handler, NULL, NULL, AP_HOOK_MIDDLE);
>      ap_hook_post_config(file_cache_post_config, NULL, NULL, AP_HOOK_MIDDLE);
>      ap_hook_translate_name(file_cache_xlat, NULL, NULL, AP_HOOK_MIDDLE);
>      /* This trick doesn't work apparently because the translate hooks
> @@ -483,12 +488,6 @@
>  
>  }
>  
> -static const handler_rec file_cache_handlers[] =
> -{
> -    { "*/*", file_cache_handler },
> -    { NULL }
> -};
> -

I think the strcmp() in hook function should just go away. Isn't that what
"*/*" means? That it applies to all types?

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

Re: windows build & handler hook

Posted by Doug MacEachern <do...@covalent.net>.
On Tue, 9 Jan 2001, Ben Laurie wrote:
 
> Errr, yes. And what is wrong with this? Seems to me that you are relying
> on buggy behaviour! Clearly the intent of:

all i know is that it broke something that has worked for close to 5 
years.  something that i had considered a 'feature'.  if it actually an
old bug that you have thrown out, no worries, i can get the old behavior
again by setting r->handler myself.


Re: windows build & handler hook

Posted by Ben Laurie <be...@algroup.co.uk>.
Doug MacEachern wrote:
> 
> On Mon, 8 Jan 2001, Ben Laurie wrote:
> 
> > This is wrong - for starters, it was "*/*" and, secondly, if you want to
> > match *s, you have to use ap_strcmp_match() (err, or something like
> > that).
> 
> ap_strcmp_match is wrong too.  your comment in http_core.c is right about
> causing problems:
> 
>     /*
>      * The old way of doing handlers meant that this handler would
>      * match literally anything - this way will require handler to
>      * have a / in the middle, which probably captures the original
>      * intent, but may cause problems at first - Ben 7th Jan 01
>      */
>     if(strcmp(r->handler,"default-handler")
>        && ap_strcmp_match(r->handler,"*/*"))
>         return DECLINED;
> 
> if i have a <Location ...> with SetHandler modperl, and the handler
> returns DECLINED for a file (e.g. /location/foo.css), the match will fail
> (no /) and SERVER_ERROR will result.

Errr, yes. And what is wrong with this? Seems to me that you are relying
on buggy behaviour! Clearly the intent of:

    if (result == DECLINED && r->handler && r->filename) {
        ap_log_rerror(APLOG_MARK, APLOG_NOERRNO|APLOG_WARNING, 0, r,
            "handler \"%s\" not found for: %s", r->handler,
r->filename);
    }

is that handlers _should_ fail if they are declined. What I can't quite
figure out is how this ever got triggered at all...

Cheers,

Ben.

--
http://www.apache-ssl.org/ben.html

"There is no limit to what a man can do or how far he can go if he
doesn't mind who gets the credit." - Robert Woodruff

Re: windows build & handler hook

Posted by Doug MacEachern <do...@covalent.net>.
On Tue, 9 Jan 2001, Ben Laurie wrote:
 
> Actually, I don't think I object too much to setting r->handler. What I
> do mind is setting it to */*! At least set it to a real type/handler
> name. For example "default-handler" would be a sensible choice,
> possibly, even better would be the content type if available.
> 
> After all, if you are going to punt, you should surely make sure you
> punt to the right guy, right?

yeah, setting to r->content_type is much better.  btw, i never said
i was happy with using '*/*', i just said it worked.  i had hoped for a
more useful suggestion than 'Vomit!', like this one :)
 
> BTW, I hope this behaviour is documented - when I set a handler, I
> rather expect it to stay set and not go around serving up stuff using
> other handlers at its whim.

look closer at what's happening here:

<Location /foo>
   SetHandler modperl
   PerlHandler Apache::Foo
</Location>

first, with the current code, modperl_response_handler() is call for every
request, but returns DECLINED unless r->handler == "modperl".

Apache::Foo::handler() gets called for everything inside /foo.
it pulls all sorts of stunts, generating .html from .pod, syntax
highlighting .pm and .pl files, whatever, but punts (return DECLINED) on
things like .gif and .css files.

so with the change you made, returning DECLINED for a .gif or .css in
that location now results in SERVER_ERROR.

don't tell me that things in /foo should point outside that location for
these files or that i should have additional <Files *.{gif,css}> type
configuration.

setting r->handler = r->content_type; in this case works well, much like
having an 'UnsetHandler modperl' directive.


Re: windows build & handler hook

Posted by Ben Laurie <be...@algroup.co.uk>.
Doug MacEachern wrote:
> 
> On Mon, 8 Jan 2001 rbb@covalent.net wrote:
> > >
> > > Vomit!
> >
> > Thank you Ben.  I have been trying to phrase that exact feeling, but I
> > couldn't express it quite that well.
> 
> heh, it's funny you guys feel that way about this, i almost hurled when i
> saw that response handlers are now in the business of type/handler
> checking.  it doesn't make me feel all that more ill adding a little hack
> to get back behavior that's been around for 5 years.

Actually, I don't think I object too much to setting r->handler. What I
do mind is setting it to */*! At least set it to a real type/handler
name. For example "default-handler" would be a sensible choice,
possibly, even better would be the content type if available.

After all, if you are going to punt, you should surely make sure you
punt to the right guy, right?

BTW, I hope this behaviour is documented - when I set a handler, I
rather expect it to stay set and not go around serving up stuff using
other handlers at its whim.

Cheers,

Ben.

--
http://www.apache-ssl.org/ben.html

"There is no limit to what a man can do or how far he can go if he
doesn't mind who gets the credit." - Robert Woodruff

Re: windows build & handler hook

Posted by Doug MacEachern <do...@covalent.net>.
On Mon, 8 Jan 2001 rbb@covalent.net wrote:
> > 
> > Vomit!
> 
> Thank you Ben.  I have been trying to phrase that exact feeling, but I
> couldn't express it quite that well.

heh, it's funny you guys feel that way about this, i almost hurled when i
saw that response handlers are now in the business of type/handler
checking.  it doesn't make me feel all that more ill adding a little hack
to get back behavior that's been around for 5 years.


Re: windows build & handler hook

Posted by rb...@covalent.net.
> > > if i have a <Location ...> with SetHandler modperl, and the handler
> > > returns DECLINED for a file (e.g. /location/foo.css), the match will fail
> > > (no /) and SERVER_ERROR will result.
> > 
> > although, with the change to reuse r->handler, i can make it work like so:
> > 
> >     retval = modperl_per_dir_callback(MP_RESPONSE_HANDLER, r);
> > 
> >     if (retval == DECLINED) {
> >         r->handler = "*/*"; /* let http_core or whatever try */
> >     }
> 
> Vomit!

Thank you Ben.  I have been trying to phrase that exact feeling, but I
couldn't express it quite that well.

Ryan

_______________________________________________________________________________
Ryan Bloom                        	rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------


Re: windows build & handler hook

Posted by Ben Laurie <be...@algroup.co.uk>.
Doug MacEachern wrote:
> 
> > if i have a <Location ...> with SetHandler modperl, and the handler
> > returns DECLINED for a file (e.g. /location/foo.css), the match will fail
> > (no /) and SERVER_ERROR will result.
> 
> although, with the change to reuse r->handler, i can make it work like so:
> 
>     retval = modperl_per_dir_callback(MP_RESPONSE_HANDLER, r);
> 
>     if (retval == DECLINED) {
>         r->handler = "*/*"; /* let http_core or whatever try */
>     }

Vomit!

Cheers,

Ben.

--
http://www.apache-ssl.org/ben.html

"There is no limit to what a man can do or how far he can go if he
doesn't mind who gets the credit." - Robert Woodruff

Re: windows build & handler hook

Posted by Doug MacEachern <do...@covalent.net>.
> if i have a <Location ...> with SetHandler modperl, and the handler
> returns DECLINED for a file (e.g. /location/foo.css), the match will fail
> (no /) and SERVER_ERROR will result.

although, with the change to reuse r->handler, i can make it work like so:

    retval = modperl_per_dir_callback(MP_RESPONSE_HANDLER, r);

    if (retval == DECLINED) {
        r->handler = "*/*"; /* let http_core or whatever try */
    }




Re: windows build & handler hook

Posted by Doug MacEachern <do...@covalent.net>.
On Mon, 8 Jan 2001, Ben Laurie wrote:
 
> This is wrong - for starters, it was "*/*" and, secondly, if you want to
> match *s, you have to use ap_strcmp_match() (err, or something like
> that).

ap_strcmp_match is wrong too.  your comment in http_core.c is right about
causing problems:

    /*
     * The old way of doing handlers meant that this handler would
     * match literally anything - this way will require handler to
     * have a / in the middle, which probably captures the original
     * intent, but may cause problems at first - Ben 7th Jan 01
     */
    if(strcmp(r->handler,"default-handler")
       && ap_strcmp_match(r->handler,"*/*"))
	return DECLINED;

if i have a <Location ...> with SetHandler modperl, and the handler
returns DECLINED for a file (e.g. /location/foo.css), the match will fail
(no /) and SERVER_ERROR will result.



Re: windows build & handler hook

Posted by Ben Laurie <be...@algroup.co.uk>.
Allan Edwards wrote:
> -static int file_cache_handler(request_rec *r)
> +static int file_cache_handler(const char *handler, request_rec *r)
>  {
>      a_file *match;
>      int errstatus;
>      int rc = OK;
> 
> +    if (strcmp(handler, "*.*")) {
> +        return DECLINED;
> +    }
> +

This is wrong - for starters, it was "*/*" and, secondly, if you want to
match *s, you have to use ap_strcmp_match() (err, or something like
that).

Cheers,

Ben.

--
http://www.apache-ssl.org/ben.html

"There is no limit to what a man can do or how far he can go if he
doesn't mind who gets the credit." - Robert Woodruff

Re: windows build & handler hook

Posted by Greg Stein <gs...@lyra.org>.
Most of Allen's patch still applies: removing the "handlers" records, the
reference to those handlers, checking for the handler string in the hook
function, etc.

A smattering of handler hooks will change after Doug applies his patch. But
I think Allen's patch should be applied.

Cheers,
-g

On Mon, Jan 08, 2001 at 11:18:47AM -0800, rbb@covalent.net wrote:
> 
> It looks like the API is going to change again soon to stop passing the
> handler char * into the function.  Instead we would be using the
> r->handler field.
> 
> Doug M has a patch that he wants to send to the list as soon as he gets
> network access again.
> 
> I would prefer to not change these just to change them back before the end
> of the day.
> 
> Ryan
> 
> On Mon, 8 Jan 2001, Allan Edwards wrote:
> 
> > A number of modules need updating for the
> > handler hook in order to get windows building 
> > again. I will be committing the following patch 
> > shortly (after some additional testing).
> 
> 
> _______________________________________________________________________________
> Ryan Bloom                        	rbb@apache.org
> 406 29th St.
> San Francisco, CA 94131
> -------------------------------------------------------------------------------

-- 
Greg Stein, http://www.lyra.org/

Re: windows build & handler hook

Posted by Greg Stein <gs...@lyra.org>.
On Tue, Jan 09, 2001 at 09:30:00AM +0000, Ben Laurie wrote:
> Greg Stein wrote:
>...
> > Modules can also set r->handler as appropriate. For example, mod_dav sets it
> > in the "type check" phase. mod_actions sets it based on various criteria.
> > 
> > All of them are about getting the correct handler into r->handler. I see
> > nothing wrong with modules setting that value when appropriate.
> 
> I agree with that - my concern was setting it in a new place, for a new
> purpose. However, as Doug points out, it only gets set when its NULL, so
> I guess its OK.

IIRC, mod_actions doesn't set it only when NULL. It just blows it away.

(it snagged me on mod_dav once)

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

Re: windows build & handler hook

Posted by Ben Laurie <be...@algroup.co.uk>.
Greg Stein wrote:
> 
> On Mon, Jan 08, 2001 at 10:37:40PM +0000, Ben Laurie wrote:
> > rbb@covalent.net wrote:
> > >
> > > It looks like the API is going to change again soon to stop passing the
> > > handler char * into the function.  Instead we would be using the
> > > r->handler field.
> >
> > You can't step on that field without possibly unforeseen consequences.
> > Or maybe you can :-)
> >
> > Anyway, the defacto meaning for that field is that it is set by
> > {Add,Set}Handler, so setting it for this may be a mistake. I'm +1 on a
> > reasoned argument that says its OK to do this, or, failing that, a new
> > field.
> 
> Modules can also set r->handler as appropriate. For example, mod_dav sets it
> in the "type check" phase. mod_actions sets it based on various criteria.
> 
> All of them are about getting the correct handler into r->handler. I see
> nothing wrong with modules setting that value when appropriate.

I agree with that - my concern was setting it in a new place, for a new
purpose. However, as Doug points out, it only gets set when its NULL, so
I guess its OK.

Cheers,

Ben.

--
http://www.apache-ssl.org/ben.html

"There is no limit to what a man can do or how far he can go if he
doesn't mind who gets the credit." - Robert Woodruff

Re: windows build & handler hook

Posted by Greg Stein <gs...@lyra.org>.
On Mon, Jan 08, 2001 at 10:37:40PM +0000, Ben Laurie wrote:
> rbb@covalent.net wrote:
> > 
> > It looks like the API is going to change again soon to stop passing the
> > handler char * into the function.  Instead we would be using the
> > r->handler field.
> 
> You can't step on that field without possibly unforeseen consequences.
> Or maybe you can :-)
> 
> Anyway, the defacto meaning for that field is that it is set by
> {Add,Set}Handler, so setting it for this may be a mistake. I'm +1 on a
> reasoned argument that says its OK to do this, or, failing that, a new
> field.

Modules can also set r->handler as appropriate. For example, mod_dav sets it
in the "type check" phase. mod_actions sets it based on various criteria.

All of them are about getting the correct handler into r->handler. I see
nothing wrong with modules setting that value when appropriate.

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

Re: windows build & handler hook

Posted by Doug MacEachern <do...@covalent.net>.
On Mon, 8 Jan 2001, Ben Laurie wrote:

> rbb@covalent.net wrote:
> > 
> > It looks like the API is going to change again soon to stop passing the
> > handler char * into the function.  Instead we would be using the
> > r->handler field.
> 
> You can't step on that field without possibly unforeseen consequences.
> Or maybe you can :-)

r->handler is only stepped on if it is NULL

> Anyway, the defacto meaning for that field is that it is set by
> {Add,Set}Handler, so setting it for this may be a mistake. I'm +1 on a
> reasoned argument that says its OK to do this, or, failing that, a new
> field.

i think the defacto meaning is that the r->handler field is set by
something in the mime-type-ish phase (i say -ish because mod_rewrite sets 
it in a fixup), it's scope is in no way limited to {Add,Set}Handler

r->handler is used to dispatch handlers, which is exactly what we're using
it for here.


Re: windows build & handler hook

Posted by Ben Laurie <be...@algroup.co.uk>.
rbb@covalent.net wrote:
> 
> It looks like the API is going to change again soon to stop passing the
> handler char * into the function.  Instead we would be using the
> r->handler field.

You can't step on that field without possibly unforeseen consequences.
Or maybe you can :-)

Anyway, the defacto meaning for that field is that it is set by
{Add,Set}Handler, so setting it for this may be a mistake. I'm +1 on a
reasoned argument that says its OK to do this, or, failing that, a new
field.

Cheers,

Ben.

--
http://www.apache-ssl.org/ben.html

"There is no limit to what a man can do or how far he can go if he
doesn't mind who gets the credit." - Robert Woodruff

Re: windows build & handler hook

Posted by Doug MacEachern <do...@covalent.net>.
what i plan todo is change ap_invoke_handler to set r->handler like so:

    const char *old_handler = r->handler;

    if (!r->handler) {
        handler = r->content_type ? r->content_type : ap_default_type(r);
        if ((p=ap_strchr_c(handler, ';')) != NULL) {
	    apr_cpystrn(hbuf, handler, sizeof hbuf);
	    p2 = hbuf+(handler-p);
	    handler = hbuf;
	    /* MIME type arguments */
            while (p2 > handler && p2[-1] == ' ')
	        --p2;		/* strip trailing spaces */
	    *p2='\0';
	}
        r->handler = handler;
    }

    result = ap_run_handler(r);

    r->handler = old_handler;

it is reset here only because it might point to hbuf[], there should be no
need for it after the response phase anyhow.  with that, the
response handler prototype can go back to the way it has always been.  i'm
going to eat lunch and then work on making that change.


RE: windows build & handler hook

Posted by Doug MacEachern <do...@covalent.net>.
> No big deal reverting a few lines of code but I'll hold off. I'm just
> eager to get Windows running again...

actually, go ahead and commit your patch.  most of it will stay anyhow,
i'll just be changing:

static int handler(const char *handler, request_rec *r) 
{
    if (strcmp(handler, "*.*")) {
    ...

to:

static int handler(request_rec *r)
{ 
    if (strcmp(r->handler, "*.*")) {
    ...
     
it will probably be easier for me to work ontop of your changes, then for
you to merge with mine.


RE: windows build & handler hook

Posted by Allan Edwards <ak...@meepzor.com>.
> It looks like the API is going to change again soon to stop passing the
> handler char * into the function.  Instead we would be using the
> r->handler field.
> 
> Doug M has a patch that he wants to send to the list as soon as he gets
> network access again.
> 
> I would prefer to not change these just to change them back before the end
> of the day.

No big deal reverting a few lines of code but I'll hold off. I'm just
eager to get Windows running again...

Re: windows build & handler hook

Posted by rb...@covalent.net.
It looks like the API is going to change again soon to stop passing the
handler char * into the function.  Instead we would be using the
r->handler field.

Doug M has a patch that he wants to send to the list as soon as he gets
network access again.

I would prefer to not change these just to change them back before the end
of the day.

Ryan

On Mon, 8 Jan 2001, Allan Edwards wrote:

> A number of modules need updating for the
> handler hook in order to get windows building 
> again. I will be committing the following patch 
> shortly (after some additional testing).


_______________________________________________________________________________
Ryan Bloom                        	rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------