[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1239966 - in /subversion/trunk/subversion/mod_dav_svn: dav_svn.h mod_dav_svn.c repos.c

From: Daniel Shahaf <danielsh_at_elego.de>
Date: Fri, 3 Feb 2012 11:07:39 +0200

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"))

Suggest:

    const char *var, *val;
    const char *equals = strchr(arg1, '=');

    if (equals)
      /* edge case: equals == var. What to do? */
      var = apr_pstrndup(cmd->pool, arg1, equals - arg1);
    else
      var = arg1;

    if (equals)
      val = equals+1;
    else
      val = "1";

> + dir_conf_t *conf = config;
> +
> + apr_hash_set(conf->hooks_env,
> + APR_ARRAY_IDX(var, 0, const char *),
> + APR_HASH_KEY_STRING,
> + APR_ARRAY_IDX(var, 1, const char *));
> + }
> +
> + return NULL;
> +}
> +
>
> /** Accessor functions for the module's configuration state **/
>
> @@ -805,6 +830,15 @@ dav_svn__get_compression_level(void)
> return svn__compression_level;
> }
>
> +/* Copy the environment given as key/value pairs of ENV_HASH into
> + * an array of C strings allocated in RESULT_POOL.
> + * Use SCRATCH_POOL for temporary allocations. */
> +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?

(And yes, if someone does SVNHooksEnv "X=1" "X=2" and then gets X=1 in
their hooks, they really have only themselves to blame; but I still
wonder why we need the double-translation.)

> + (const char *)svn__apr_hash_index_key(hi),
> + (const char *)svn__apr_hash_index_val(hi));
> + envp++;
> + }
> + *envp = NULL;
>
> + return env;
> +}
>
> static dav_error *
> get_resource(request_rec *r,

Cheers,

Daniel
Received on 2012-02-03 10:08:50 CET

This is an archived mail posted to the Subversion Dev mailing list.