On Fri, Feb 03, 2012 at 11:07:39AM +0200, Daniel Shahaf wrote:
> stsp_at_apache.org wrote on Fri, Feb 03, 2012 at 01:02:08 -0000:
> > +static const char *
> > +SVNHooksEnv_cmd(cmd_parms *cmd, void *config, const char *arg1)
> > +{
> > + apr_array_header_t *var;
> > +
> > + var = svn_cstring_split(arg1, "=", TRUE, cmd->pool);
> > + if (var && var->nelts == 2)
> > + {
>
> With this code, if an envvar's value legitimately contains a '=', it'll
> be silently skipped. (Example:
> putenv("config-option=servers:global:http-library=serf"))
Ah, you're right. It should require at least two elements and
combine the second element with any that follow.
> > +static const char **
> > +env_from_env_hash(apr_hash_t *env_hash,
> > + apr_pool_t *result_pool,
> > + apr_pool_t *scratch_pool)
> > +{
> > + apr_hash_index_t *hi;
> > + const char **env;
> > + const char **envp;
> > +
> > + env = apr_palloc(result_pool,
> > + sizeof(const char *) * (apr_hash_count(env_hash) + 1));
> > + envp = env;
> > + for (hi = apr_hash_first(scratch_pool, env_hash); hi; hi = apr_hash_next(hi))
> > + {
> > + *envp = apr_psprintf(result_pool, "%s=%s",
>
> Heh. So you parse an array of VAR=VAL lines into a hash, and then
> unparse it back? Why not carry it as an array (perhaps an APR array) of
> "VAR=VAL" values?
We need to parse it once anyway to verify the VAR=VAL syntax.
So this was mainly done to make sure we're using the right syntax
for the evecve() env parameter (see e.g. the putenv(3) man page).
After making this change I realised it would be nicer just let the
svn_repos API accept an apr_hash_t and perform the translation to
const char** internally. This would also make it trivial to have
svn_repos API calls to remove or add variables, or merge environments.
Merging might in fact be necessary if we want to support custom hook
environments over all RA protocols. Think about where users could define
custom hook env for file:// access? If it ends up being a config file
in the repository it makes no sense for mod_dav_svn and svnserve to ignore
that file...
Received on 2012-02-03 11:35:21 CET