You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Peter Samuelson <pe...@p12n.org> on 2011/11/20 06:45:57 UTC
[RFC] Make mod_dav_svn + mod_authz_svn load-order-independent
Currently if you load mod_authz_svn, you have to make sure mod_dav_svn
is loaded before it, not after. I'm sure this is usually not a problem
- but in Debian/Ubuntu, the Apache module machinery enables modules by
use of tiny config files that each load one module. The filenames
determine load order, so a config file named 'authz_svn.load' is read
before one named 'dav_svn.load'.
In the past we got around this by shipping a single 'dav_svn.load' that
loads _both_ modules, but I never liked that solution, as most people
don't need mod_authz_svn at all.
Thus, appended is a patch to mod_authz_svn to pull in the two dav_svn
functions in such a way as to work even if the modules are loaded in
the "wrong" order.
Question: do we care about breaking third-party Apache modules that use
the two functions used by mod_authz_svn? (These third-party modules
would need to do the same thing I'm patching mod_authz_svn to do.) If
so, we can leave the original prototype in the public mod_dav_svn.h
instead of moving it, as I've done below.
--
Peter Samuelson | org-tld!p12n!peter | http://p12n.org/
[[[
Use apr_optional.h facilities to import mod_dav_svn functions into
mod_authz_svn.
* subversion/include/mod_dav_svn.h
(): Convert prototypes for dav_svn_split_uri and
dav_svn_get_repos_path to APR optional function types. Move the original prototypes...
* subversion/mod_dav_svn/dav_svn.h
(): ...here.
* subversion/mod_dav_svn/mod_dav_svn.c
(register_hooks): Register the two functions for export.
* subversion/mod_authz_svn/mod_authz_svn.c
(dav_svn_get_repos_path, dav_svn_split_uri): Declare as static
function pointers.
(register_hooks): Populate function pointers by calling...
(import_dav_svn): New.
]]]
Index: subversion/include/mod_dav_svn.h
===================================================================
--- subversion/include/mod_dav_svn.h (revision 1204126)
+++ subversion/include/mod_dav_svn.h (working copy)
@@ -30,6 +30,7 @@
#include <httpd.h>
#include <mod_dav.h>
+#include <apr_optional.h>
#ifdef __cplusplus
@@ -75,22 +76,24 @@ extern "C" {
- @a repos_path: A/B/alpha
- @a trailing_slash: FALSE
*/
-AP_MODULE_DECLARE(dav_error *) dav_svn_split_uri(request_rec *r,
- const char *uri,
- const char *root_path,
- const char **cleaned_uri,
- int *trailing_slash,
- const char **repos_basename,
- const char **relative_path,
- const char **repos_path);
+APR_DECLARE_OPTIONAL_FN(dav_error *, dav_svn_split_uri,
+ (request_rec *r,
+ const char *uri,
+ const char *root_path,
+ const char **cleaned_uri,
+ int *trailing_slash,
+ const char **repos_basename,
+ const char **relative_path,
+ const char **repos_path));
/**
* Given an apache request @a r and a @a root_path to the svn location
* block, set @a *repos_path to the path of the repository on disk. */
-AP_MODULE_DECLARE(dav_error *) dav_svn_get_repos_path(request_rec *r,
- const char *root_path,
- const char **repos_path);
+APR_DECLARE_OPTIONAL_FN(dav_error *, dav_svn_get_repos_path,
+ (request_rec *r,
+ const char *root_path,
+ const char **repos_path));
#ifdef __cplusplus
}
Index: subversion/mod_authz_svn/mod_authz_svn.c
===================================================================
--- subversion/mod_authz_svn/mod_authz_svn.c (revision 1204126)
+++ subversion/mod_authz_svn/mod_authz_svn.c (working copy)
@@ -64,6 +64,9 @@ typedef struct authz_svn_config_rec {
const char *force_username_case;
} authz_svn_config_rec;
+static APR_OPTIONAL_FN_TYPE(dav_svn_get_repos_path) *dav_svn_get_repos_path;
+static APR_OPTIONAL_FN_TYPE(dav_svn_split_uri) *dav_svn_split_uri;
+
/*
* Configuration
*/
@@ -777,6 +780,12 @@ auth_checker(request_rec *r)
return OK;
}
+static void import_dav_svn(void)
+{
+ dav_svn_get_repos_path = APR_RETRIEVE_OPTIONAL_FN(dav_svn_get_repos_path);
+ dav_svn_split_uri = APR_RETRIEVE_OPTIONAL_FN(dav_svn_split_uri);
+}
+
/*
* Module flesh
*/
@@ -798,6 +807,7 @@ register_hooks(apr_pool_t *p)
AUTHZ_SVN__SUBREQ_BYPASS_PROV_NAME,
AUTHZ_SVN__SUBREQ_BYPASS_PROV_VER,
(void*)subreq_bypass);
+ ap_hook_optional_fn_retrieve(import_dav_svn,NULL,NULL,APR_HOOK_MIDDLE);
}
module AP_MODULE_DECLARE_DATA authz_svn_module =
Index: subversion/mod_dav_svn/mod_dav_svn.c
===================================================================
--- subversion/mod_dav_svn/mod_dav_svn.c (revision 1204126)
+++ subversion/mod_dav_svn/mod_dav_svn.c (working copy)
@@ -1040,6 +1040,9 @@ static dav_provider provider =
static void
register_hooks(apr_pool_t *pconf)
{
+ APR_REGISTER_OPTIONAL_FN(dav_svn_get_repos_path);
+ APR_REGISTER_OPTIONAL_FN(dav_svn_split_uri);
+
ap_hook_pre_config(init_dso, NULL, NULL, APR_HOOK_REALLY_FIRST);
ap_hook_post_config(init, NULL, NULL, APR_HOOK_MIDDLE);
Index: subversion/mod_dav_svn/dav_svn.h
===================================================================
--- subversion/mod_dav_svn/dav_svn.h (revision 1204126)
+++ subversion/mod_dav_svn/dav_svn.h (working copy)
@@ -446,6 +446,17 @@ int dav_svn__method_post(request_rec *r);
/*** repos.c ***/
+/* See doc in public mod_dav_svn.h */
+dav_error *
+dav_svn_split_uri(request_rec *r,
+ const char *uri,
+ const char *root_path,
+ const char **cleaned_uri,
+ int *trailing_slash,
+ const char **repos_basename,
+ const char **relative_path,
+ const char **repos_path);
+
/* generate an ETag for RESOURCE and return it, allocated in POOL. */
const char *
dav_svn__getetag(const dav_resource *resource, apr_pool_t *pool);
@@ -492,7 +503,13 @@ extern const dav_hooks_repository dav_svn__hooks_r
/*** deadprops.c ***/
extern const dav_hooks_propdb dav_svn__hooks_propdb;
+/* See doc in public mod_dav_svn.h */
+dav_error *
+dav_svn_get_repos_path(request_rec *r,
+ const char *root_path,
+ const char **repos_path);
+
/*** lock.c ***/
extern const dav_hooks_locks dav_svn__hooks_locks;
Re: [RFC] Make mod_dav_svn + mod_authz_svn load-order-independent
Posted by Greg Stein <gs...@gmail.com>.
On Nov 20, 2011 12:46 AM, "Peter Samuelson" <pe...@p12n.org> wrote:
>
>
> Currently if you load mod_authz_svn, you have to make sure mod_dav_svn
> is loaded before it, not after. I'm sure this is usually not a problem
> - but in Debian/Ubuntu, the Apache module machinery enables modules by
> use of tiny config files that each load one module. The filenames
> determine load order, so a config file named 'authz_svn.load' is read
> before one named 'dav_svn.load'.
>
> In the past we got around this by shipping a single 'dav_svn.load' that
> loads _both_ modules, but I never liked that solution, as most people
> don't need mod_authz_svn at all.
>
> Thus, appended is a patch to mod_authz_svn to pull in the two dav_svn
> functions in such a way as to work even if the modules are loaded in
> the "wrong" order.
>
> Question: do we care about breaking third-party Apache modules that use
> the two functions used by mod_authz_svn? (These third-party modules
> would need to do the same thing I'm patching mod_authz_svn to do.) If
> so, we can leave the original prototype in the public mod_dav_svn.h
> instead of moving it, as I've done below.
Yes, we care. I would bet that CollabNet uses them for its custom authz.
Cheers,
-g