Re: [PATCH 2/4] upload-pack.c: allow banning certain object filter(s)

From: Jeff King <peff@peff.net>
Date: Jul 8, 2020, 08:45 UTC
Message-ID: <20200708084527.GB2324177@coredump.intra.peff.net>
In-Reply-To: <f0982d24e74155f6c0e405e5e3ae8c3e579f798a.1593720075.git.me@ttaylorr.com>

On Thu, Jul 02, 2020 at 04:06:32PM -0400, Taylor Blau wrote:

Show 11 quoted lines

> +static void parse_object_filter_config(const char *var, const char *value,
> +				       struct upload_pack_data *data)
> +{
> +	struct strbuf spec = STRBUF_INIT;
> +	const char *sub, *key;
> +	size_t sub_len;
> +
> +	if (parse_config_key(var, "uploadpack", &sub, &sub_len, &key))
> +		return;
> +	if (!sub || !skip_prefix(sub, "filter.", &sub))
> +		return;

Just while I'm thinking about the config name and case-sensitivity (from the cover letter): if we did want to use this scheme, then skip_iprefix() would make this behave more like a regular part of the section name.

But I'd prefer to just do away with it by using a scheme that doesn't have the extra layer of dots.

> +	if (sub != key)
> +		strbuf_add(&spec, sub, key - sub - 1);
> +	strbuf_tolower(&spec);

On the flip side, I'd actually consider _not_ matching the filter name case-insensitively. We don't do so elsewhere (e.g., "git rev-list --filter=BLOB:NONE" will complain).

-Peff