git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Taylor Blau <me@ttaylorr.com>
To: git@vger.kernel.org
Cc: peff@peff.net, chriscool@tuxfamily.org, gitster@pobox.com,
	szeder.dev@gmail.com
Subject: [PATCH v4 3/3] upload-pack.c: introduce 'uploadpackfilter.tree.maxDepth'
Date: Mon, 3 Aug 2020 14:00:17 -0400	[thread overview]
Message-ID: <a0e7731a55b6cda3e1dd549725692a2d4c9b0900.1596476929.git.me@ttaylorr.com> (raw)
In-Reply-To: <cover.1596476928.git.me@ttaylorr.com>

In b79cf959b2 (upload-pack.c: allow banning certain object filter(s),
2020-02-26), we introduced functionality to disallow certain object
filters from being chosen from within 'git upload-pack'. Traditionally,
administrators use this functionality to disallow filters that are known
to perform slowly, for e.g., those that do not have bitmap-level
filtering.

In the past, the '--filter=tree:<n>' was one such filter that does not
have bitmap-level filtering support, and so was likely to be banned by
administrators.

However, in the previous couple of commits, we introduced bitmap-level
filtering for the case when 'n' is equal to '0', i.e., as if we had a
'--filter=tree:none' choice.

While it would be sufficient to simply write

  $ git config uploadpackfilter.tree.allow true

(since it would allow all values of 'n'), we would like to be able to
allow this filter for certain values of 'n', i.e., those no greater than
some pre-specified maximum.

In order to do this, introduce a new configuration key, as follows:

  $ git config uploadpackfilter.tree.maxDepth <m>

where '<m>' specifies the maximum allowed value of 'n' in the filter
'tree:n'. Administrators who wish to allow for only the value '0' can
write:

  $ git config uploadpackfilter.tree.allow true
  $ git config uploadpackfilter.tree.maxDepth 0

which allows '--filter=tree:0', but no other values.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
 Documentation/config/uploadpack.txt |  6 ++++++
 t/t5616-partial-clone.sh            |  9 +++++++++
 upload-pack.c                       | 18 ++++++++++++++++++
 3 files changed, 33 insertions(+)

diff --git a/Documentation/config/uploadpack.txt b/Documentation/config/uploadpack.txt
index fffe8ac648..ee7b3ac94f 100644
--- a/Documentation/config/uploadpack.txt
+++ b/Documentation/config/uploadpack.txt
@@ -69,6 +69,12 @@ uploadpackfilter.<filter>.allow::
 	combined filters, both `combine` and all of the nested filter
 	kinds must be allowed. Defaults to `uploadpackfilter.allow`.
 
+uploadpackfilter.tree.maxDepth::
+	Only allow `--filter=tree=<n>` when `n` is no more than the value of
+	`uploadpackfilter.tree.maxDepth`. If set, this also implies
+	`uploadpackfilter.tree.allow=true`, unless this configuration
+	variable had already been set. Has no effect if unset.
+
 uploadpack.allowRefInWant::
 	If this option is set, `upload-pack` will support the `ref-in-want`
 	feature of the protocol version 2 `fetch` command.  This feature
diff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh
index aa68782587..96ab45feab 100755
--- a/t/t5616-partial-clone.sh
+++ b/t/t5616-partial-clone.sh
@@ -259,6 +259,15 @@ test_expect_success 'upload-pack fails banned object filters with fallback' '
 	grep "filter '\''blob:none'\'' not supported" err
 '
 
+test_expect_success 'upload-pack limits tree depth filters' '
+	test_config -C srv.bare uploadpackfilter.allow false &&
+	test_config -C srv.bare uploadpackfilter.tree.allow true &&
+	test_config -C srv.bare uploadpackfilter.tree.maxDepth 0 &&
+	test_must_fail ok=sigpipe git clone --no-checkout --filter=tree:1 \
+		"file://$(pwd)/srv.bare" pc3 2>err &&
+	grep "tree filter allows max depth 0, but got 1" err
+'
+
 test_expect_success 'partial clone fetches blobs pointed to by refs even if normally filtered out' '
 	rm -rf src dst &&
 	git init src &&
diff --git a/upload-pack.c b/upload-pack.c
index 2098af8156..fbd2db04cd 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -105,6 +105,7 @@ struct upload_pack_data {
 	unsigned use_include_tag : 1;
 	unsigned allow_filter : 1;
 	unsigned allow_filter_fallback : 1;
+	unsigned long tree_filter_max_depth;
 
 	unsigned done : 1;					/* v2 only */
 	unsigned allow_ref_in_want : 1;				/* v2 only */
@@ -136,6 +137,7 @@ static void upload_pack_data_init(struct upload_pack_data *data)
 	data->extra_edge_obj = extra_edge_obj;
 	data->allowed_filters = allowed_filters;
 	data->allow_filter_fallback = 1;
+	data->tree_filter_max_depth = ULONG_MAX;
 	packet_writer_init(&data->writer, 1);
 
 	data->keepalive = 5;
@@ -1020,6 +1022,13 @@ static void check_one_filter(struct upload_pack_data *data,
 
 	if (!allowed)
 		send_err_and_die(data, "filter '%s' not supported", key);
+
+	if (opts->choice == LOFC_TREE_DEPTH &&
+	    opts->tree_exclude_depth > data->tree_filter_max_depth)
+		send_err_and_die(data,
+				 "tree filter allows max depth %lu, but got %lu",
+				 data->tree_filter_max_depth,
+				 opts->tree_exclude_depth);
 }
 
 static void check_filter_recurse(struct upload_pack_data *data,
@@ -1249,6 +1258,15 @@ static int parse_object_filter_config(const char *var, const char *value,
 	if (!strcmp(key, "allow"))
 		string_list_insert(&data->allowed_filters, buf.buf)->util =
 			(void *)(intptr_t)git_config_bool(var, value);
+	else if (!strcmp(buf.buf, "tree") && !strcmp(key, "maxdepth")) {
+		if (!value) {
+			strbuf_release(&buf);
+			return config_error_nonbool(var);
+		}
+		string_list_insert(&data->allowed_filters, buf.buf)->util =
+			(void *)(intptr_t)1;
+		data->tree_filter_max_depth = git_config_ulong(var, value);
+	}
 
 	strbuf_release(&buf);
 	return 0;
-- 
2.28.0.rc1.13.ge78abce653

  parent reply	other threads:[~2020-08-03 18:00 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-23  1:48 [PATCH v2 0/4] upload-pack: custom allowed object filters Taylor Blau
2020-07-23  1:48 ` [PATCH v2 1/4] list_objects_filter_options: introduce 'list_object_filter_config_name' Taylor Blau
2020-07-23  1:49 ` [PATCH v2 2/4] upload-pack.c: allow banning certain object filter(s) Taylor Blau
2020-07-23  1:49 ` [PATCH v2 3/4] upload-pack.c: pass 'struct list_objects_filter_options *' Taylor Blau
2020-07-23  1:49 ` [PATCH v2 4/4] upload-pack.c: introduce 'uploadpackfilter.tree.maxDepth' Taylor Blau
2020-07-23 20:43 ` [PATCH v2 0/4] upload-pack: custom allowed object filters SZEDER Gábor
2020-07-24 16:51   ` Taylor Blau
2020-07-24 19:51     ` Jeff King
2020-07-27 14:25       ` Taylor Blau
2020-07-27 19:34     ` SZEDER Gábor
2020-07-27 19:36       ` Taylor Blau
2020-07-27 19:42         ` Jeff King
2020-07-27 19:59         ` SZEDER Gábor
2020-07-27 20:03           ` Taylor Blau
2020-07-31 20:26 ` [PATCH v3 " Taylor Blau
2020-07-31 20:26   ` [PATCH v3 1/4] list_objects_filter_options: introduce 'list_object_filter_config_name' Taylor Blau
2020-07-31 20:26   ` [PATCH v3 2/4] upload-pack.c: allow banning certain object filter(s) Taylor Blau
2020-07-31 20:54     ` Jeff King
2020-07-31 21:20       ` Taylor Blau
2020-07-31 20:26   ` [PATCH v3 3/4] upload-pack.c: pass 'struct list_objects_filter_options *' Taylor Blau
2020-07-31 20:26   ` [PATCH v3 4/4] upload-pack.c: introduce 'uploadpackfilter.tree.maxDepth' Taylor Blau
2020-07-31 21:01     ` Jeff King
2020-07-31 21:22       ` Jeff King
2020-07-31 21:30         ` Taylor Blau
2020-07-31 21:29       ` Taylor Blau
2020-07-31 21:36         ` Jeff King
2020-07-31 21:43           ` Jeff King
2020-08-03 18:00 ` [PATCH v4 0/3] upload-pack: custom allowed object filters Taylor Blau
2020-08-03 18:00   ` [PATCH v4 2/3] upload-pack.c: allow banning certain object filter(s) Taylor Blau
2020-08-03 18:00   ` [PATCH v4 1/3] list_objects_filter_options: introduce 'list_object_filter_config_name' Taylor Blau
2020-08-03 18:00   ` Taylor Blau [this message]
2020-08-04  0:37   ` [PATCH v4 0/3] upload-pack: custom allowed object filters Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a0e7731a55b6cda3e1dd549725692a2d4c9b0900.1596476929.git.me@ttaylorr.com \
    --to=me@ttaylorr.com \
    --cc=chriscool@tuxfamily.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    --cc=szeder.dev@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).