commit 6d2ee845c030a3b3cadd2897ed2e5ce279c1a266
Author: John Dennis <jdennis@redhat.com>
Date: Tue Aug 8 10:00:30 2017 -0400
Track file information
File information was handled inconsistently. Some configuration
directives which specified a file path replaced the file path with the
contents of the file. This made it impossible to report where the data
was read from. Other file configuration simply recorded the path. The
directives which immediately read the file contents would generate a
configuration error if the file wasn't readable, but those directives
which simply recorded the file path didn't check on the validity of
the path and relied on Lasso to report an error, however these errors
come significantly after configuration parsing because they are
evaluated in a lazy fashion on first use. The Lasso error reporting
can sometimes be cryptic making it difficult to realize the problem is
due to a improperly specified path in a configuration directive.
We want to be able to log the file pathnames where various files are
read from for diagnostic logging purposes.
This patch introduces a new struct am_file_data_t that encapsulates
all information concerning a file including it's pathname, it's stat
information, optionally it's content, when it was read, etc. as well
as maintaing error codes and an error description.
All file specifications and operations now use this mechanism for
consistency.
Signed-off-by: John Dennis <jdennis@redhat.com>
diff --git a/auth_mellon.h b/auth_mellon.h
index aff658b..a6fa34c 100644
--- a/auth_mellon.h
+++ b/auth_mellon.h
@@ -159,6 +159,30 @@ typedef enum {
extern const char *am_cond_options[];
+/*
+ * am_file_data_t is used to maintain information about a file:
+ *
+ * * The filesystem pathname
+ * * Stat information about the file (e.g. type, size, times, etc.)
+ * * If and when the file was stat'ed or read
+ * * Error code of failed operation and error string description
+ * * Contents of the file
+ * * Flag indicating if contents were generated instead of being read
+ * from a file.
+ */
+typedef struct am_file_data_t {
+ apr_pool_t *pool; /* allocation pool */
+ const char *path; /* filesystem pathname, NULL for generated file */
+ apr_time_t stat_time; /* when stat was performed, zero indicates never */
+ apr_finfo_t finfo; /* stat data */
+ char *contents; /* file contents */
+ apr_time_t read_time; /* when contents was read, zero indicates never */
+ apr_status_t rv; /* most recent result value */
+ const char *strerror; /* if rv is error then this is error description */
+ bool generated; /* true if contents generated instead of being
+ read from path */
+} am_file_data_t;
+
typedef struct {
const char *varname;
int flags;
@@ -168,8 +192,8 @@ typedef struct {
} am_cond_t;
typedef struct am_metadata {
- const char *file; /* Metadata file with one or many IdP */
- const char *chain; /* Validating chain */
+ am_file_data_t *metadata; /* Metadata file with one or many IdP */
+ am_file_data_t *chain; /* Validating chain */
} am_metadata_t;
typedef struct am_dir_cfg_rec {
@@ -201,12 +225,12 @@ typedef struct am_dir_cfg_rec {
const char *endpoint_path;
/* Lasso configuration variables. */
- const char *sp_metadata_file;
- const char *sp_private_key_file;
- const char *sp_cert_file;
+ am_file_data_t *sp_metadata_file;
+ am_file_data_t *sp_private_key_file;
+ am_file_data_t *sp_cert_file;
apr_array_header_t *idp_metadata;
- const char *idp_public_key_file;
- const char *idp_ca_file;
+ am_file_data_t *idp_public_key_file;
+ am_file_data_t *idp_ca_file;
GList *idp_ignore;
/* metadata autogeneration helper */
@@ -424,7 +448,11 @@ char *am_urlencode(apr_pool_t *pool, const char *str);
int am_urldecode(char *data);
int am_check_url(request_rec *r, const char *url);
char *am_generate_id(request_rec *r);
-char *am_getfile(apr_pool_t *conf, server_rec *s, const char *file);
+am_file_data_t *am_file_data_new(apr_pool_t *pool, const char *path);
+am_file_data_t *am_file_data_copy(apr_pool_t *pool,
+ am_file_data_t *src_file_data);
+apr_status_t am_file_read(am_file_data_t *file_data);
+apr_status_t am_file_stat(am_file_data_t *file_data);
char *am_get_endpoint_url(request_rec *r);
int am_postdir_cleanup(request_rec *s);
char *am_htmlencode(request_rec *r, const char *str);
diff --git a/auth_mellon_config.c b/auth_mellon_config.c
index 3dfab67..c3cb5e0 100644
--- a/auth_mellon_config.c
+++ b/auth_mellon_config.c
@@ -176,9 +176,8 @@ static const char *am_set_table_string_slot(cmd_parms *cmd,
return NULL;
}
-/* This function handles configuration directives which set a file
- * slot in the module configuration. If lasso is recent enough, it
- * attempts to read the file immediatly.
+/* This function handles configuration directives which set a file slot
+ * in the module configuration. The file contents are immediately read.
*
* Parameters:
* cmd_parms *cmd The command structure for this configuration
@@ -192,12 +191,15 @@ static const char *am_set_table_string_slot(cmd_parms *cmd,
* Returns:
* NULL on success or an error string on failure.
*/
-static const char *am_set_filestring_slot(cmd_parms *cmd,
+static const char *am_set_file_contents_slot(cmd_parms *cmd,
void *struct_ptr,
const char *arg)
{
- const char *data;
const char *path;
+ apr_status_t rv;
+ am_dir_cfg_rec *cfg = (am_dir_cfg_rec *)struct_ptr;
+ int offset;
+ am_file_data_t **p_file_data, *file_data;
path = ap_server_root_relative(cmd->pool, arg);
if (!path) {
@@ -205,29 +207,64 @@ static const char *am_set_filestring_slot(cmd_parms *cmd,
": Invalid file path ", arg, NULL);
}
-#ifdef HAVE_lasso_server_new_from_buffers
- data = am_getfile(cmd->pool, cmd->server, path);
- if (!data) {
- return apr_pstrcat(cmd->pool, cmd->cmd->name,
- ": Cannot read file ", path, NULL);
+ offset = (int)(long)cmd->info;
+ p_file_data = (am_file_data_t **)((char *)cfg + offset);
+ *p_file_data = am_file_data_new(cmd->pool, path);
+ file_data = *p_file_data;
+ rv = am_file_read(file_data);
+ if (rv != APR_SUCCESS) {
+ return file_data->strerror;
}
-#else
- apr_finfo_t finfo;
+
+ return NULL;
+}
+
+/* This function handles configuration directives which set a file
+ * pathname in the module configuration. The file is checked for
+ * existence.
+ *
+ * Parameters:
+ * cmd_parms *cmd The command structure for this configuration
+ * directive.
+ * void *struct_ptr Pointer to the current directory configuration.
+ * NULL if we are not in a directory configuration.
+ * This value isn't used by this function.
+ * const char *arg The string argument following this configuration
+ * directive in the configuraion file.
+ *
+ * Returns:
+ * NULL on success or an error string on failure.
+ */
+static const char *am_set_file_pathname_slot(cmd_parms *cmd,
+ void *struct_ptr,
+ const char *arg)
+{
+ const char *path;
apr_status_t rv;
- char error[64];
+ am_dir_cfg_rec *cfg = (am_dir_cfg_rec *)struct_ptr;
+ int offset;
+ am_file_data_t **p_file_data, *file_data;
- rv = apr_stat(&finfo, path, APR_FINFO_SIZE, cmd->pool);
- if(rv != 0) {
- apr_strerror(rv, error, sizeof(error));
- return apr_psprintf(cmd->pool,
- "%s - Cannot read file \"%s\" [%d] \"%s\"",
- cmd->cmd->name, path, rv, error);
+ path = ap_server_root_relative(cmd->pool, arg);
+ if (!path) {
+ return apr_pstrcat(cmd->pool, cmd->cmd->name,
+ ": Invalid file_data path ", arg, NULL);
}
- data = path;
-#endif
+ offset = (int)(long)cmd->info;
+ p_file_data = (am_file_data_t **)((char *)cfg + offset);
+ *p_file_data = am_file_data_new(cmd->pool, path);
+ file_data = *p_file_data;
+ rv = am_file_stat(file_data);
+ if (rv != APR_SUCCESS) {
+ return file_data->strerror;
+ }
+ if (file_data->finfo.filetype != APR_REG) {
+ return apr_psprintf(cmd->pool, "file \"%s\" is not a regular file",
+ file_data->path);
+ }
- return ap_set_string_slot(cmd, struct_ptr, data);
+ return NULL;
}
@@ -305,6 +342,8 @@ static const char *am_set_idp_string_slot(cmd_parms *cmd,
server_rec *s = cmd->server;
apr_pool_t *pconf = s->process->pconf;
am_dir_cfg_rec *cfg = (am_dir_cfg_rec *)struct_ptr;
+ am_file_data_t *idp_file_data = NULL;
+ am_file_data_t *chain_file_data = NULL;
#ifndef HAVE_lasso_server_load_metadata
if (chain != NULL)
@@ -313,9 +352,23 @@ static const char *am_set_idp_string_slot(cmd_parms *cmd,
"lasso_server_load_metadata()", cmd->cmd->name);
#endif /* HAVE_lasso_server_load_metadata */
+ idp_file_data = am_file_data_new(pconf, metadata);
+ if (am_file_stat(idp_file_data) != APR_SUCCESS) {
+ return idp_file_data->strerror;
+ }
+
+ if (chain) {
+ chain_file_data = am_file_data_new(pconf, chain);
+ if (am_file_stat(chain_file_data) != APR_SUCCESS) {
+ return chain_file_data->strerror;
+ }
+ } else {
+ chain_file_data = NULL;
+ }
+
am_metadata_t *idp_metadata = apr_array_push(cfg->idp_metadata);
- idp_metadata->file = apr_pstrdup(pconf, metadata);
- idp_metadata->chain = apr_pstrdup(pconf, chain);
+ idp_metadata->metadata = idp_file_data;
+ idp_metadata->chain = chain_file_data;
return NULL;
}
@@ -1230,21 +1283,21 @@ const command_rec auth_mellon_commands[] = {
),
AP_INIT_TAKE1(
"MellonSPMetadataFile",
- am_set_filestring_slot,
+ am_set_file_contents_slot,
(void *)APR_OFFSETOF(am_dir_cfg_rec, sp_metadata_file),
OR_AUTHCFG,
"Full path to xml file with metadata for the SP."
),
AP_INIT_TAKE1(
"MellonSPPrivateKeyFile",
- am_set_filestring_slot,
+ am_set_file_contents_slot,
(void *)APR_OFFSETOF(am_dir_cfg_rec, sp_private_key_file),
OR_AUTHCFG,
"Full path to pem file with the private key for the SP."
),
AP_INIT_TAKE1(
"MellonSPCertFile",
- am_set_filestring_slot,
+ am_set_file_contents_slot,
(void *)APR_OFFSETOF(am_dir_cfg_rec, sp_cert_file),
OR_AUTHCFG,
"Full path to pem file with certificate for the SP."
@@ -1267,14 +1320,14 @@ const command_rec auth_mellon_commands[] = {
),
AP_INIT_TAKE1(
"MellonIdPPublicKeyFile",
- ap_set_file_slot,
+ am_set_file_pathname_slot,
(void *)APR_OFFSETOF(am_dir_cfg_rec, idp_public_key_file),
OR_AUTHCFG,
"Full path to pem file with the public key for the IdP."
),
AP_INIT_TAKE1(
"MellonIdPCAFile",
- ap_set_file_slot,
+ am_set_file_pathname_slot,
(void *)APR_OFFSETOF(am_dir_cfg_rec, idp_ca_file),
OR_AUTHCFG,
"Full path to pem file with CA chain for the IdP."
@@ -1679,7 +1732,6 @@ void *auth_mellon_dir_merge(apr_pool_t *p, void *base, void *add)
add_cfg->no_success_error_page :
base_cfg->no_success_error_page);
-
new_cfg->sp_metadata_file = (add_cfg->sp_metadata_file ?
add_cfg->sp_metadata_file :
base_cfg->sp_metadata_file);
diff --git a/auth_mellon_handler.c b/auth_mellon_handler.c
index 2004752..08b7869 100644
--- a/auth_mellon_handler.c
+++ b/auth_mellon_handler.c
@@ -32,13 +32,6 @@ APLOG_USE_MODULE(auth_mellon);
* the ECP.rst file.
*/
-#ifdef HAVE_lasso_server_new_from_buffers
-# define SERVER_NEW lasso_server_new_from_buffers
-#else /* HAVE_lasso_server_new_from_buffers */
-# define SERVER_NEW lasso_server_new
-#endif /* HAVE_lasso_server_new_from_buffers */
-
-
#ifdef HAVE_lasso_server_new_from_buffers
/* This function generates optional metadata for a given element
@@ -130,7 +123,7 @@ static char *am_generate_metadata(apr_pool_t *p, request_rec *r)
sp_entity_id = cfg->sp_entity_id ? cfg->sp_entity_id : url;
- if (cfg->sp_cert_file) {
+ if (cfg->sp_cert_file && cfg->sp_cert_file->contents) {
char *sp_cert_file;
char *cp;
char *bp;
@@ -141,7 +134,7 @@ static char *am_generate_metadata(apr_pool_t *p, request_rec *r)
* Try to remove leading and trailing garbage, as it can
* wreak havoc XML parser if it contains [<>&]
*/
- sp_cert_file = apr_pstrdup(p, cfg->sp_cert_file);
+ sp_cert_file = apr_pstrdup(p, cfg->sp_cert_file->contents);
cp = strstr(sp_cert_file, begin);
if (cp != NULL)
@@ -237,7 +230,8 @@ static guint am_server_add_providers(am_dir_cfg_rec *cfg, request_rec *r)
const char *idp_public_key_file;
if (cfg->idp_metadata->nelts == 1)
- idp_public_key_file = cfg->idp_public_key_file;
+ idp_public_key_file = cfg->idp_public_key_file ?
+ cfg->idp_public_key_file->path : NULL;
else
idp_public_key_file = NULL;
#endif /* ! HAVE_lasso_server_load_metadata */
@@ -260,8 +254,9 @@ static guint am_server_add_providers(am_dir_cfg_rec *cfg, request_rec *r)
#ifdef HAVE_lasso_server_load_metadata
error = lasso_server_load_metadata(cfg->server,
LASSO_PROVIDER_ROLE_IDP,
- idp_metadata->file,
- idp_metadata->chain,
+ idp_metadata->metadata->path,
+ idp_metadata->chain ?
+ idp_metadata->chain->path : NULL,
cfg->idp_ignore,
&loaded_idp,
LASSO_SERVER_LOAD_METADATA_FLAG_DEFAULT);
@@ -271,7 +266,7 @@ static guint am_server_add_providers(am_dir_cfg_rec *cfg, request_rec *r)
for (idx = loaded_idp; idx != NULL; idx = idx->next) {
ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
"loaded IdP \"%s\" from \"%s\".",
- (char *)idx->data, idp_metadata->file);
+ (char *)idx->data, idp_metadata->metadata->path);
}
}
@@ -285,16 +280,17 @@ static guint am_server_add_providers(am_dir_cfg_rec *cfg, request_rec *r)
#else /* HAVE_lasso_server_load_metadata */
error = lasso_server_add_provider(cfg->server,
LASSO_PROVIDER_ROLE_IDP,
- idp_metadata->file,
+ idp_metadata->metadata->path,
idp_public_key_file,
- cfg->idp_ca_file);
+ cfg->idp_ca_file ?
+ cfg->idp_ca_file->path : NULL);
#endif /* HAVE_lasso_server_load_metadata */
if (error != 0) {
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
"Error adding metadata \"%s\" to "
"lasso server objects. Lasso error: [%i] %s",
- idp_metadata->file, error, lasso_strerror(error));
+ idp_metadata->metadata->path, error, lasso_strerror(error));
}
}
@@ -317,7 +313,10 @@ static LassoServer *am_get_lasso_server(request_rec *r)
* Try to generate missing metadata
*/
apr_pool_t *pool = r->server->process->pconf;
- cfg->sp_metadata_file = am_generate_metadata(pool, r);
+ cfg->sp_metadata_file = am_file_data_new(pool, NULL);
+ cfg->sp_metadata_file->rv = APR_SUCCESS;
+ cfg->sp_metadata_file->generated = true;
+ cfg->sp_metadata_file->contents = am_generate_metadata(pool, r);
#else
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
"Missing MellonSPMetadataFile option.");
@@ -326,11 +325,22 @@ static LassoServer *am_get_lasso_server(request_rec *r)
#endif /* HAVE_lasso_server_new_from_buffers */
}
- cfg->server = SERVER_NEW(cfg->sp_metadata_file,
- cfg->sp_private_key_file,
- NULL,
- cfg->sp_cert_file);
- if(cfg->server == NULL) {
+#ifdef HAVE_lasso_server_new_from_buffers
+ cfg->server = lasso_server_new_from_buffers(cfg->sp_metadata_file->contents,
+ cfg->sp_private_key_file ?
+ cfg->sp_private_key_file->contents : NULL,
+ NULL,
+ cfg->sp_cert_file ?
+ cfg->sp_cert_file->contents : NULL);
+#else
+ cfg->server = lasso_server_new(cfg->sp_metadata_file->path,
+ cfg->sp_private_key_file ?
+ cfg->sp_private_key_file->path : NULL,
+ NULL,
+ cfg->sp_cert_file ?
+ cfg->sp_cert_file->path : NULL);
+#endif
+ if (cfg->server == NULL) {
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
"Error initializing lasso server object. Please"
" verify the following configuration directives:"
@@ -2390,8 +2400,8 @@ static int am_handle_repost(request_rec *r)
char *charset;
char *psf_id;
char *cp;
- char *psf_filename;
- char *post_data;
+ am_file_data_t *file_data;
+ const char *post_data;
const char *post_form;
char *output;
char *return_url;
@@ -2486,14 +2496,24 @@ static int am_handle_repost(request_rec *r)
return rc;
}
- psf_filename = apr_psprintf(r->pool, "%s/%s", mod_cfg->post_dir, psf_id);
- post_data = am_getfile(r->pool, r->server, psf_filename);
- if (post_data == NULL) {
+ if ((file_data = am_file_data_new(r->pool, NULL)) == NULL) {
+ ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r,
+ "Bad repost query: cannot allocate file_data");
+ apr_table_setn(r->headers_out, "Location", return_url);
+ return HTTP_SEE_OTHER;
+ }
+
+ file_data->path = apr_psprintf(file_data->pool, "%s/%s",
+ mod_cfg->post_dir, psf_id);
+ rc = am_file_read(file_data);
+ if (rc != APR_SUCCESS) {
/* Unable to load repost data. Just redirect us instead. */
ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r,
- "Bad repost query: cannot find \"%s\"", psf_filename);
+ "Bad repost query: %s", file_data->strerror);
apr_table_setn(r->headers_out, "Location", return_url);
return HTTP_SEE_OTHER;
+ } else {
+ post_data = file_data->contents;
}
if ((post_form = (*post_mkform)(r, post_data)) == NULL) {
@@ -2556,7 +2576,7 @@ static int am_handle_metadata(request_rec *r)
cfg = cfg->inherit_server_from;
- data = cfg->sp_metadata_file;
+ data = cfg->sp_metadata_file ? cfg->sp_metadata_file->contents : NULL;
if (data == NULL)
return HTTP_INTERNAL_SERVER_ERROR;
diff --git a/auth_mellon_httpclient.c b/auth_mellon_httpclient.c
index d9688df..ecf5c0d 100644
--- a/auth_mellon_httpclient.c
+++ b/auth_mellon_httpclient.c
@@ -298,11 +298,11 @@ static CURL *am_httpclient_init_curl(request_rec *r, const char *uri,
/* If we have a CA configured, try to use it */
if (cfg->idp_ca_file != NULL) {
- res = curl_easy_setopt(curl, CURLOPT_CAINFO, cfg->idp_ca_file);
+ res = curl_easy_setopt(curl, CURLOPT_CAINFO, cfg->idp_ca_file->path);
if(res != CURLE_OK) {
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
"Failed to set SSL CA info %s:"
- " [%u] %s", cfg->idp_ca_file, res, curl_error);
+ " [%u] %s", cfg->idp_ca_file->path, res, curl_error);
goto cleanup_fail;
}
}
diff --git a/auth_mellon_session.c b/auth_mellon_session.c
index 856dbb6..3eae4a0 100644
--- a/auth_mellon_session.c
+++ b/auth_mellon_session.c
@@ -119,7 +119,6 @@ am_cache_entry_t *am_new_request_session(request_rec *r)
return NULL;
}
-
/* Set session id. */
am_cookie_set(r, session_id);
diff --git a/auth_mellon_util.c b/auth_mellon_util.c
index 19bfb41..7f8d52b 100644
--- a/auth_mellon_util.c
+++ b/auth_mellon_util.c
@@ -1022,57 +1022,179 @@ const char *am_filepath_dirname(apr_pool_t *p, const char *path)
}
/*
- * malloc a buffer and fill it with a given file
+ * Allocate and initialize a am_file_data_t
*
* Parameters:
- * apr_pool_t *conf The configuration pool. Valid as long as this
- * server_rec *s The server record for the current server.
- * const char *file The file path
+ * apr_pool_t *pool Allocation pool.
+ * const char *path If non-NULL initialize file_data->path to copy of path
*
* Returns:
- * char * The file content
+ * Newly allocated & initialized file_data_t
*/
-char *am_getfile(apr_pool_t *conf, server_rec *s, const char *file)
+am_file_data_t *am_file_data_new(apr_pool_t *pool, const char *path)
+{
+ am_file_data_t *file_data = NULL;
+
+ if ((file_data = apr_pcalloc(pool, sizeof(am_file_data_t))) == NULL) {
+ return NULL;
+ }
+
+ file_data->pool = pool;
+ file_data->rv = APR_EINIT;
+ if (path) {
+ file_data->path = apr_pstrdup(file_data->pool, path);
+ }
+
+ return file_data;
+}
+
+/*
+ * Allocate a new am_file_data_t and copy
+ *
+ * Parameters:
+ * apr_pool_t *pool Allocation pool.
+ * am_file_data_t *src_file_data The src being copied.
+ *
+ * Returns:
+ * Newly allocated & initialized from src_file_data
+ */
+am_file_data_t *am_file_data_copy(apr_pool_t *pool,
+ am_file_data_t *src_file_data)
+{
+ am_file_data_t *dst_file_data = NULL;
+
+ if ((dst_file_data = am_file_data_new(pool, src_file_data->path)) == NULL) {
+ return NULL;
+ }
+
+ dst_file_data->path = apr_pstrdup(pool, src_file_data->path);
+ dst_file_data->stat_time = src_file_data->stat_time;
+ dst_file_data->finfo = src_file_data->finfo;
+ dst_file_data->contents = apr_pstrdup(pool, src_file_data->contents);
+ dst_file_data->read_time = src_file_data->read_time;
+ dst_file_data->rv = src_file_data->rv;
+ dst_file_data->strerror = apr_pstrdup(pool, src_file_data->strerror);
+ dst_file_data->generated = src_file_data->generated;
+
+ return dst_file_data;
+}
+
+/*
+ * Peform a stat on a file to get it's properties
+ *
+ * A stat is performed on the file. If there was an error the
+ * result value is left in file_data->rv and an error description
+ * string is formatted and left in file_data->strerror and function
+ * returns the rv value. If the stat was successful the stat
+ * information is left in file_data->finfo and APR_SUCCESS
+ * set set as file_data->rv and returned as the function result.
+ *
+ * The file_data->stat_time indicates if and when the stat was
+ * performed, a zero time value indicates the operation has not yet
+ * been performed.
+ *
+ * Parameters:
+ * am_file_data_t *file_data Struct containing file information
+ *
+ * Returns:
+ * APR status code, same value as file_data->rv
+ */
+apr_status_t am_file_stat(am_file_data_t *file_data)
+{
+ char buffer[512];
+
+ if (file_data == NULL) {
+ return APR_EINVAL;
+ }
+
+ file_data->strerror = NULL;
+
+ file_data->stat_time = apr_time_now();
+ file_data->rv = apr_stat(&file_data->finfo, file_data->path,
+ APR_FINFO_SIZE, file_data->pool);
+ if (file_data->rv != APR_SUCCESS) {
+ file_data->strerror =
+ apr_psprintf(file_data->pool,
+ "apr_stat: Error opening \"%s\" [%d] \"%s\"",
+ file_data->path, file_data->rv,
+ apr_strerror(file_data->rv, buffer, sizeof(buffer)));
+ }
+
+ return file_data->rv;
+}
+
+/*
+ * Read file into dynamically allocated buffer
+ *
+ * First a stat is performed on the file. If there was an error the
+ * result value is left in file_data->rv and an error description
+ * string is formatted and left in file_data->strerror and function
+ * returns the rv value. If the stat was successful the stat
+ * information is left in file_data->finfo.
+ *
+ * A buffer is dynamically allocated and the contents of the file is
+ * read into file_data->contents. If there was an error the result
+ * value is left in file_data->rv and an error description string is
+ * formatted and left in file_data->strerror and the function returns
+ * the rv value.
+ *
+ * The file_data->stat_time and file_data->read_time indicate if and
+ * when those operations were performed, a zero time value indicates
+ * the operation has not yet been performed.
+ *
+ * Parameters:
+ * am_file_data_t *file_data Struct containing file information
+ *
+ * Returns:
+ * APR status code, same value as file_data->rv
+ */
+apr_status_t am_file_read(am_file_data_t *file_data)
{
- apr_status_t rv;
char buffer[512];
- apr_finfo_t finfo;
- char *data;
apr_file_t *fd;
apr_size_t nbytes;
- if (file == NULL)
- return NULL;
+ if (file_data == NULL) {
+ return APR_EINVAL;
+ }
+ file_data->rv = APR_SUCCESS;
+ file_data->strerror = NULL;
- if ((rv = apr_file_open(&fd, file, APR_READ, 0, conf)) != 0) {
- ap_log_error(APLOG_MARK, APLOG_ERR, rv, s,
- "apr_file_open: Error opening \"%s\" [%d] \"%s\"",
- file, rv, apr_strerror(rv, buffer, sizeof(buffer)));
- return NULL;
+ am_file_stat(file_data);
+ if (file_data->rv != APR_SUCCESS) {
+ return file_data->rv;
}
- if ((rv = apr_file_info_get(&finfo, APR_FINFO_SIZE, fd)) != 0) {
- ap_log_error(APLOG_MARK, APLOG_ERR, rv, s,
- "apr_file_info_get: Error opening \"%s\" [%d] \"%s\"",
- file, rv, apr_strerror(rv, buffer, sizeof(buffer)));
- (void)apr_file_close(fd);
- return NULL;
+ if ((file_data->rv = apr_file_open(&fd, file_data->path,
+ APR_READ, 0, file_data->pool)) != 0) {
+ file_data->strerror =
+ apr_psprintf(file_data->pool,
+ "apr_file_open: Error opening \"%s\" [%d] \"%s\"",
+ file_data->path, file_data->rv,
+ apr_strerror(file_data->rv, buffer, sizeof(buffer)));
+ return file_data->rv;
}
- nbytes = finfo.size;
- data = (char *)apr_palloc(conf, nbytes + 1);
+ file_data->read_time = apr_time_now();
+ nbytes = file_data->finfo.size;
+ file_data->contents = (char *)apr_palloc(file_data->pool, nbytes + 1);
+
+ file_data->rv = apr_file_read_full(fd, file_data->contents, nbytes, NULL);
+ if (file_data->rv != 0) {
+ file_data->strerror =
+ apr_psprintf(file_data->pool,
+ "apr_file_read_full: Error reading \"%s\" [%d] \"%s\"",
+ file_data->path, file_data->rv,
+ apr_strerror(file_data->rv, buffer, sizeof(buffer)));
+ (void)apr_file_close(fd);
+ return file_data->rv;
- rv = apr_file_read_full(fd, data, nbytes, NULL);
- if (rv != 0) {
- ap_log_error(APLOG_MARK, APLOG_ERR, rv, s,
- "apr_file_read_full: Error reading \"%s\" [%d] \"%s\"",
- file, rv, apr_strerror(rv, buffer, sizeof(buffer)));
}
- data[nbytes] = '\0';
+ file_data->contents[nbytes] = '\0';
(void)apr_file_close(fd);
- return data;
+ return file_data->rv;
}
/*
diff --git a/mod_auth_mellon.c b/mod_auth_mellon.c
index 3d44460..f632d37 100644
--- a/mod_auth_mellon.c
+++ b/mod_auth_mellon.c
@@ -220,7 +220,6 @@ static void register_hooks(apr_pool_t *p)
* r->handler and decide that it is the only handler for this URL.
*/
ap_hook_handler(am_handler, NULL, NULL, APR_HOOK_FIRST);
- return;
}