Emily Shaffer <emilyshaffer@xxxxxxxxxx> writes:
> Teach bugreport to gather the values of config options which are present
> in 'bugreport-config-safelist.h'.
>
> Many config options are sensitive, and many Git add-ons use config
> options which git-core does not know about; it is better only to gather
> config options which we know to be safe, rather than excluding options
> which we know to be unsafe.
>
> Taking the build-time generated array and putting it into a set saves us
> time - since git_config_bugreport() is called for every option the user
> has configured, performing option lookup in constant time is a useful
> optimization.
Interesting. I actually was expecting the look-up to go the other
way around. We know the safe keys, so iterate over them and grab
their values, if defined. No need for hashes or anything, but just
a simple linear list of safe stuff.
But that is too simple-minded. If we wanted to safelist foo.*.bar,
where '*' can be anything, walking on the list of safe variables
would not work. We must have a hash table that knows "foo.*.bar" is
allowed, and while walking all the configuration keys, when we see
foo.a.bar, we consult "foo.*.bar" as well as "foo.a.bar" to see if
it is whitelisted, or something like that.
But then I am not sure if this implementation does something like
this for three-level names. If not, I do not see much point in use
of the hash there either.
Puzzled.
>
> Signed-off-by: Emily Shaffer <emilyshaffer@xxxxxxxxxx>
> ---
> bugreport.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 71 insertions(+), 1 deletion(-)
>
> diff --git a/bugreport.c b/bugreport.c
> index 759cc0b0f8..1fca28f0b9 100644
> --- a/bugreport.c
> +++ b/bugreport.c
> @@ -6,6 +6,9 @@
> #include "help.h"
> #include <gnu/libc-version.h>
> #include "run-command.h"
> +#include "config.h"
> +#include "bugreport-config-safelist.h"
> +#include "khash.h"
>
> static void get_http_version_info(struct strbuf *http_info)
> {
> @@ -18,6 +21,41 @@ static void get_http_version_info(struct strbuf *http_info)
> strbuf_addstr(http_info, "'git-http-fetch -V' not supported\n");
> }
>
> +KHASH_INIT(cfg_set, const char*, int, 0, kh_str_hash_func, kh_str_hash_equal);
> +
> +struct cfgset {
> + kh_cfg_set_t set;
> +};
> +
> +struct cfgset safelist;
> +
> +static void cfgset_init(struct cfgset *set, size_t initial_size)
> +{
> + memset(&set->set, 0, sizeof(set->set));
> + if (initial_size)
> + kh_resize_cfg_set(&set->set, initial_size);
> +}
> +
> +static int cfgset_insert(struct cfgset *set, const char *cfg_key)
> +{
> + int added;
> + kh_put_cfg_set(&set->set, cfg_key, &added);
> + printf("ESS: added %s\n", cfg_key);
> + return !added;
> +}
> +
> +static int cfgset_contains(struct cfgset *set, const char *cfg_key)
> +{
> + khiter_t pos = kh_get_cfg_set(&set->set, cfg_key);
> + return pos != kh_end(&set->set);
> +}
> +
> +static void cfgset_clear(struct cfgset *set)
> +{
> + kh_release_cfg_set(&set->set);
> + cfgset_init(set, 0);
> +}
> +
> static void get_system_info(struct strbuf *sys_info)
> {
> struct strbuf version_info = STRBUF_INIT;
> @@ -53,6 +91,36 @@ static void get_system_info(struct strbuf *sys_info)
> strbuf_complete_line(sys_info);
> }
>
> +static void gather_safelist()
> +{
> + int index;
> + int safelist_len = sizeof(bugreport_config_safelist) / sizeof(const char *);
> + cfgset_init(&safelist, safelist_len);
> + for (index = 0; index < safelist_len; index++)
> + cfgset_insert(&safelist, bugreport_config_safelist[index]);
> +
> +}
> +
> +static int git_config_bugreport(const char *var, const char *value, void *cb)
> +{
> + struct strbuf *config_info = (struct strbuf *)cb;
> +
> + if (cfgset_contains(&safelist, var))
> + strbuf_addf(config_info,
> + "%s (%s) : %s\n",
> + var, config_scope_to_string(current_config_scope()),
> + value);
> +
> + return 0;
> +}
> +
> +static void get_safelisted_config(struct strbuf *config_info)
> +{
> + gather_safelist();
> + git_config(git_config_bugreport, config_info);
> + cfgset_clear(&safelist);
> +}
> +
> static const char * const bugreport_usage[] = {
> N_("git bugreport [-o|--output <file>]"),
> NULL
> @@ -114,10 +182,12 @@ int cmd_main(int argc, const char **argv)
>
> get_bug_template(&buffer);
>
> - // add other contents
> get_header(&buffer, "System Info");
> get_system_info(&buffer);
>
> + get_header(&buffer, "Safelisted Config Info");
> + get_safelisted_config(&buffer);
> +
> report = fopen_for_writing(report_path.buf);
> strbuf_write(&buffer, report);
> fclose(report);
