Author: breser
Date: Wed Dec 19 02:59:02 2012
New Revision: 1423739
URL: http://svn.apache.org/viewvc?rev=1423739&view=rev
Log:
On the 'in-repo-authz' branch: Allow mod_authz_svn to log the full error chain.
Due to the changes to support in-repo-authz errors are segmented and finding
the first/"best" message is no longer sufficient to fully log the issue. This
change also moves the duplicated code for logging svn_errors into a shared
function.
* subversion/mod_authz_svn/mod_authz_svn.c
(LOG_ARGS_SIGNATURE,LOG_ARGS_CASCADE,log_access_verdict): Move towards the
start of the file so that log_svn_error() can share the macros.
(log_svn_error): New function.
(get_access_conf,req_check_access,subreq_bypass2): Use log_svn_error().
Modified:
subversion/branches/in-repo-authz/subversion/mod_authz_svn/mod_authz_svn.c
Modified:
subversion/branches/in-repo-authz/subversion/mod_authz_svn/mod_authz_svn.c
URL:
http://svn.apache.org/viewvc/subversion/branches/in-repo-authz/subversion/mod_authz_svn/mod_authz_svn.c?rev=1423739&r1=1423738&r2=1423739&view=diff
==============================================================================
--- subversion/branches/in-repo-authz/subversion/mod_authz_svn/mod_authz_svn.c
(original)
+++ subversion/branches/in-repo-authz/subversion/mod_authz_svn/mod_authz_svn.c
Wed Dec 19 02:59:02 2012
@@ -168,6 +168,102 @@ static const command_rec authz_svn_cmds[
{ NULL }
};
+
+/* The macros LOG_ARGS_SIGNATURE and LOG_ARGS_CASCADE are expanded as formal
+ * and actual parameters to log_access_verdict with respect to HTTPD version.
+ */
+#if AP_MODULE_MAGIC_AT_LEAST(20100606,0)
+#define LOG_ARGS_SIGNATURE const char *file, int line, int module_index
+#define LOG_ARGS_CASCADE file, line, module_index
+#else
+#define LOG_ARGS_SIGNATURE const char *file, int line
+#define LOG_ARGS_CASCADE file, line
+#endif
+
+/* Log a message indicating the access control decision made about a
+ * request. The macro LOG_ARGS_SIGNATURE expands to FILE, LINE and
+ * MODULE_INDEX in HTTPD 2.3 as APLOG_MARK macro has been changed for
+ * per-module loglevel configuration. It expands to FILE and LINE
+ * in older server versions. ALLOWED is boolean.
+ * REPOS_PATH and DEST_REPOS_PATH are information
+ * about the request. DEST_REPOS_PATH may be NULL. */
+static void
+log_access_verdict(LOG_ARGS_SIGNATURE,
+ const request_rec *r, int allowed,
+ const char *repos_path, const char *dest_repos_path)
+{
+ int level = allowed ? APLOG_INFO : APLOG_ERR;
+ const char *verdict = allowed ? "granted" : "denied";
+
+ if (r->user)
+ {
+ if (dest_repos_path)
+ ap_log_rerror(LOG_ARGS_CASCADE, level, 0, r,
+ "Access %s: '%s' %s %s %s", verdict, r->user,
+ r->method, repos_path, dest_repos_path);
+ else
+ ap_log_rerror(LOG_ARGS_CASCADE, level, 0, r,
+ "Access %s: '%s' %s %s", verdict, r->user,
+ r->method, repos_path);
+ }
+ else
+ {
+ if (dest_repos_path)
+ ap_log_rerror(LOG_ARGS_CASCADE, level, 0, r,
+ "Access %s: - %s %s %s", verdict,
+ r->method, repos_path, dest_repos_path);
+ else
+ ap_log_rerror(LOG_ARGS_CASCADE, level, 0, r,
+ "Access %s: - %s %s", verdict,
+ r->method, repos_path);
+ }
+}
+
+/* Log a message indiciating the ERR encountered during the request R.
+ * LOG_ARGS_SIGNATURE expands as in log_access_verdict() above.
+ * PREFIX is inserted at the start of the message. The rest of the
+ * message is generated by combining the message for each error in the
+ * chain of ERR, excluding for trace errors. ERR will be cleared
+ * when finished. */
+static void
+log_svn_error(LOG_ARGS_SIGNATURE,
+ request_rec *r, const char *prefix,
+ svn_error_t *err, apr_pool_t *scratch_pool)
+{
+ svn_error_t *err_pos = svn_error_purge_tracing(err);
+ svn_stringbuf_t *buff = svn_stringbuf_create(prefix, scratch_pool);
+
+ /* Build the error chain into a space separated stringbuf. */
+ while (err_pos)
+ {
+ if (err_pos->message)
+ {
+ svn_stringbuf_appendcstr(buff, err_pos->message);
+ svn_stringbuf_appendbyte(buff, ' ');
+ }
+ else
+ {
+ char strerr[256];
+
+ svn_stringbuf_appendcstr(buff, svn_strerror(err->apr_err, strerr,
+ sizeof(strerr)));
+ }
+
+ err_pos = err_pos->child;
+ }
+
+ ap_log_rerror(LOG_ARGS_CASCADE, APLOG_ERR,
+ /* If it is an error code that APR can make sense of, then
+ show it, otherwise, pass zero to avoid putting "APR does
+ not understand this error code" in the error log. */
+ ((err->apr_err >= APR_OS_START_USERERR &&
+ err->apr_err < APR_OS_START_CANONERR) ?
+ 0 : err->apr_err),
+ r, "%s", buff->data);
+
+ svn_error_clear(err);
+}
+
/*
* Get the, possibly cached, svn_authz_t for this request.
*/
@@ -182,7 +278,6 @@ get_access_conf(request_rec *r, authz_sv
svn_authz_t *access_conf = NULL;
svn_error_t *svn_err;
dav_error *dav_err;
- char errbuf[256];
dav_err = dav_svn_get_repos_path(r, conf->base_path, &repos_path);
if (dav_err)
@@ -220,17 +315,9 @@ get_access_conf(request_rec *r, authz_sv
TRUE, repos_path, r->connection->pool);
if (svn_err)
{
- ap_log_rerror(APLOG_MARK, APLOG_ERR,
- /* If it is an error code that APR can make sense
- of, then show it, otherwise, pass zero to avoid
- putting "APR does not understand this error code"
- in the error log. */
- ((svn_err->apr_err >= APR_OS_START_USERERR &&
- svn_err->apr_err < APR_OS_START_CANONERR) ?
- 0 : svn_err->apr_err),
- r, "Failed to load the AuthzSVNAccessFile: %s",
- svn_err_best_message(svn_err, errbuf, sizeof(errbuf)));
- svn_error_clear(svn_err);
+ log_svn_error(APLOG_MARK, r,
+ "Failed to load the AuthzSVNAccessFile: ",
+ svn_err, scratch_pool);
access_conf = NULL;
}
else
@@ -299,7 +386,6 @@ req_check_access(request_rec *r,
svn_boolean_t authz_access_granted = FALSE;
svn_authz_t *access_conf = NULL;
svn_error_t *svn_err;
- char errbuf[256];
const char *username_to_authorize = get_username_to_authorize(r, conf,
r->pool);
@@ -472,19 +558,9 @@ req_check_access(request_rec *r,
r->pool);
if (svn_err)
{
- ap_log_rerror(APLOG_MARK, APLOG_ERR,
- /* If it is an error code that APR can make
- sense of, then show it, otherwise, pass
- zero to avoid putting "APR does not
- understand this error code" in the error
- log. */
- ((svn_err->apr_err >= APR_OS_START_USERERR &&
- svn_err->apr_err < APR_OS_START_CANONERR) ?
- 0 : svn_err->apr_err),
- r, "Failed to perform access control: %s",
- svn_err_best_message(svn_err, errbuf,
- sizeof(errbuf)));
- svn_error_clear(svn_err);
+ log_svn_error(APLOG_MARK, r,
+ "Failed to perform access control: ",
+ svn_err, r->pool);
return DECLINED;
}
@@ -519,17 +595,9 @@ req_check_access(request_rec *r,
r->pool);
if (svn_err)
{
- ap_log_rerror(APLOG_MARK, APLOG_ERR,
- /* If it is an error code that APR can make sense
- of, then show it, otherwise, pass zero to avoid
- putting "APR does not understand this error code"
- in the error log. */
- ((svn_err->apr_err >= APR_OS_START_USERERR &&
- svn_err->apr_err < APR_OS_START_CANONERR) ?
- 0 : svn_err->apr_err),
- r, "Failed to perform access control: %s",
- svn_err_best_message(svn_err, errbuf, sizeof(errbuf)));
- svn_error_clear(svn_err);
+ log_svn_error(APLOG_MARK, r,
+ "Failed to perform access control: ",
+ svn_err, r->pool);
return DECLINED;
}
@@ -544,56 +612,6 @@ req_check_access(request_rec *r,
return OK;
}
-/* The macros LOG_ARGS_SIGNATURE and LOG_ARGS_CASCADE are expanded as formal
- * and actual parameters to log_access_verdict with respect to HTTPD version.
- */
-#if AP_MODULE_MAGIC_AT_LEAST(20100606,0)
-#define LOG_ARGS_SIGNATURE const char *file, int line, int module_index
-#define LOG_ARGS_CASCADE file, line, module_index
-#else
-#define LOG_ARGS_SIGNATURE const char *file, int line
-#define LOG_ARGS_CASCADE file, line
-#endif
-
-/* Log a message indicating the access control decision made about a
- * request. The macro LOG_ARGS_SIGNATURE expands to FILE, LINE and
- * MODULE_INDEX in HTTPD 2.3 as APLOG_MARK macro has been changed for
- * per-module loglevel configuration. It expands to FILE and LINE
- * in older server versions. ALLOWED is boolean.
- * REPOS_PATH and DEST_REPOS_PATH are information
- * about the request. DEST_REPOS_PATH may be NULL. */
-static void
-log_access_verdict(LOG_ARGS_SIGNATURE,
- const request_rec *r, int allowed,
- const char *repos_path, const char *dest_repos_path)
-{
- int level = allowed ? APLOG_INFO : APLOG_ERR;
- const char *verdict = allowed ? "granted" : "denied";
-
- if (r->user)
- {
- if (dest_repos_path)
- ap_log_rerror(LOG_ARGS_CASCADE, level, 0, r,
- "Access %s: '%s' %s %s %s", verdict, r->user,
- r->method, repos_path, dest_repos_path);
- else
- ap_log_rerror(LOG_ARGS_CASCADE, level, 0, r,
- "Access %s: '%s' %s %s", verdict, r->user,
- r->method, repos_path);
- }
- else
- {
- if (dest_repos_path)
- ap_log_rerror(LOG_ARGS_CASCADE, level, 0, r,
- "Access %s: - %s %s %s", verdict,
- r->method, repos_path, dest_repos_path);
- else
- ap_log_rerror(LOG_ARGS_CASCADE, level, 0, r,
- "Access %s: - %s %s", verdict,
- r->method, repos_path);
- }
-}
-
/*
* Implementation of subreq_bypass with scratch_pool parameter.
*/
@@ -607,7 +625,6 @@ subreq_bypass2(request_rec *r,
svn_authz_t *access_conf = NULL;
authz_svn_config_rec *conf = NULL;
svn_boolean_t authz_access_granted = FALSE;
- char errbuf[256];
const char *username_to_authorize;
conf = ap_get_module_config(r->per_dir_config,
@@ -640,18 +657,9 @@ subreq_bypass2(request_rec *r,
scratch_pool);
if (svn_err)
{
- ap_log_rerror(APLOG_MARK, APLOG_ERR,
- /* If it is an error code that APR can make
- sense of, then show it, otherwise, pass
- zero to avoid putting "APR does not
- understand this error code" in the error
- log. */
- ((svn_err->apr_err >= APR_OS_START_USERERR &&
- svn_err->apr_err < APR_OS_START_CANONERR) ?
- 0 : svn_err->apr_err),
- r, "Failed to perform access control: %s",
- svn_err_best_message(svn_err, errbuf, sizeof(errbuf)));
- svn_error_clear(svn_err);
+ log_svn_error(APLOG_MARK, r,
+ "Failed to perform access control: ",
+ svn_err, scratch_pool);
return HTTP_FORBIDDEN;
}
if (!authz_access_granted)
