git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 0/4] upload-pack: custom allowed object filters

From
Taylor Blau <me@ttaylorr.com>
Date
Jul 23, 2020, 01:48 UTC
Message-ID
<cover.1595468657.git.me@ttaylorr.com>

Here's a much-delayed v2 of my series to teach upload-pack to limit the kinds of object filters that it is willing to server in a request.

Much is the same since last time, with two notable exceptions:
  - We use the 'uploadpackfilter' top-level configuration key instead of
    pretending that 'uploadpack.filter' is top-level, which greatly
    simplifies the call to 'parse_config_key()'.
  - Instead of writing an err packet, 'git upload-pack' simply 'die()'s,
    which propagates the error through 'git clone' always, and resolves
    a flaky set of tests that used to result in a SIGPIPE.

Thanks for your review in the meantime, and for encouraging me to send this reroll now instead of later. Maybe I'll start unloading the rest of my backlog tomorrow ;-).

Taylor Blau (4):
  list_objects_filter_options: introduce
    'list_object_filter_config_name'
  upload-pack.c: allow banning certain object filter(s)
  upload-pack.c: pass 'struct list_objects_filter_options *'
  upload-pack.c: introduce 'uploadpackfilter.tree.maxDepth'
 Documentation/config/uploadpack.txt |  18 +++++
 list-objects-filter-options.c       |  23 +++++++
 list-objects-filter-options.h       |   6 ++
 t/t5616-partial-clone.sh            |  32 +++++++++
 upload-pack.c                       | 103 ++++++++++++++++++++++++++++
 5 files changed, 182 insertions(+)
Range-diff against v1:
  2:  f0982d24e7 ! 134:  9fee52cb6d upload-pack.c: allow banning certain object filter(s)
    @@ Commit message
         Allow configuring 'git upload-pack' to support object filters on a
         case-by-case basis by introducing two new configuration variables:
    -      - 'uploadpack.filter.allow'
    -      - 'uploadpack.filter.<kind>.allow'
    +      - 'uploadpackfilter.allow'
    +      - 'uploadpackfilter.<kind>.allow'
         where '<kind>' may be one of 'blobNone', 'blobLimit', 'tree', and so on.
    @@ Commit message
         If a client requests the object filter <kind> and the respective
         configuration value is not set, 'git upload-pack' will default to the
    -    value of 'uploadpack.filter.allow', which itself defaults to 'true' to
    +    value of 'uploadpackfilter.allow', which itself defaults to 'true' to
         maintain backwards compatibility. Note that this differs from
         'uploadpack.allowfilter', which controls whether or not the 'filter'
         capability is advertised.
    @@ Documentation/config/uploadpack.txt: uploadpack.allowFilter::
      	If this option is set, `upload-pack` will support partial
      	clone and partial fetch object filtering.
    -+uploadpack.filter.allow::
    ++uploadpackfilter.allow::
     +	Provides a default value for unspecified object filters (see: the
     +	below configuration variable).
     +	Defaults to `true`.
     +
    -+uploadpack.filter.<filter>.allow::
    ++uploadpackfilter.<filter>.allow::
     +	Explicitly allow or ban the object filter corresponding to
     +	`<filter>`, where `<filter>` may be one of: `blob:none`,
     +	`blob:limit`, `tree`, `sparse:oid`, or `combine`. If using
     +	combined filters, both `combine` and all of the nested filter
    -+	kinds must be allowed.  Defaults to `uploadpack.filter.allow`.
    -++
    -+Note that the dot between 'filter' and '<filter>' is both non-standard
    -+and intentional. This is done to avoid a parsing ambiguity when
    -+specifying this configuration as an argument to Git's top-level `-c`.
    ++	kinds must be allowed. Defaults to `uploadpackfilter.allow`.
     +
      uploadpack.allowRefInWant::
      	If this option is set, `upload-pack` will support the `ref-in-want`
    @@ t/t5616-partial-clone.sh: test_expect_success 'implicitly construct combine: fil
      '
     +test_expect_success 'upload-pack fails banned object filters' '
    -+	# Test case-insensitivity by intentional use of "blob:None" rather than
    -+	# "blob:none".
    -+	test_config -C srv.bare uploadpack.filter.blob:None.allow false &&
    ++	test_config -C srv.bare uploadpackfilter.blob:none.allow false &&
     +	test_must_fail git clone --no-checkout --filter=blob:none \
     +		"file://$(pwd)/srv.bare" pc3 2>err &&
     +	test_i18ngrep "filter '\''blob:none'\'' not supported" err
     +'
     +
     +test_expect_success 'upload-pack fails banned combine object filters' '
    -+	test_config -C srv.bare uploadpack.filter.allow false &&
    -+	test_config -C srv.bare uploadpack.filter.combine.allow true &&
    -+	test_config -C srv.bare uploadpack.filter.tree.allow true &&
    -+	test_config -C srv.bare uploadpack.filter.blob:none.allow false &&
    ++	test_config -C srv.bare uploadpackfilter.allow false &&
    ++	test_config -C srv.bare uploadpackfilter.combine.allow true &&
    ++	test_config -C srv.bare uploadpackfilter.tree.allow true &&
    ++	test_config -C srv.bare uploadpackfilter.blob:none.allow false &&
     +	test_must_fail git clone --no-checkout --filter=tree:1 \
     +		--filter=blob:none "file://$(pwd)/srv.bare" pc3 2>err &&
     +	test_i18ngrep "filter '\''blob:none'\'' not supported" err
     +'
     +
     +test_expect_success 'upload-pack fails banned object filters with fallback' '
    -+	test_config -C srv.bare uploadpack.filter.allow false &&
    ++	test_config -C srv.bare uploadpackfilter.allow false &&
     +	test_must_fail git clone --no-checkout --filter=blob:none \
     +		"file://$(pwd)/srv.bare" pc3 2>err &&
     +	test_i18ngrep "filter '\''blob:none'\'' not supported" err
    @@ upload-pack.c: static int process_deepen_not(const char *line, struct string_lis
     +	if (!banned)
     +		return;
     +
    -+	packet_writer_error(&data->writer, _("filter '%s' not supported\n"),
    -+			    list_object_filter_config_name(banned->choice));
    -+	die(_("git upload-pack: banned object filter requested"));
    ++	die(_("git upload-pack: filter '%s' not supported"),
    ++	    list_object_filter_config_name(banned->choice));
     +}
     +
      static void receive_needs(struct upload_pack_data *data,
    @@ upload-pack.c: static int find_symref(const char *refname, const struct object_i
      	return 0;
      }
    -+static void parse_object_filter_config(const char *var, const char *value,
    ++static int parse_object_filter_config(const char *var, const char *value,
     +				       struct upload_pack_data *data)
     +{
    -+	struct strbuf spec = STRBUF_INIT;
    ++	struct strbuf buf = 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;
    ++	if (parse_config_key(var, "uploadpackfilter", &sub, &sub_len, &key))
    ++		return 0;
     +
    -+	if (sub != key)
    -+		strbuf_add(&spec, sub, key - sub - 1);
    -+	strbuf_tolower(&spec);
    -+
    -+	if (!strcmp(key, "allow")) {
    -+		if (spec.len)
    -+			string_list_insert(&data->allowed_filters, spec.buf)->util =
    -+				(void *)(intptr_t)git_config_bool(var, value);
    -+		else
    ++	if (!sub) {
    ++		if (!strcmp(key, "allow"))
     +			data->allow_filter_fallback = git_config_bool(var, value);
    ++		return 0;
     +	}
     +
    -+	strbuf_release(&spec);
    ++	strbuf_add(&buf, sub, sub_len);
    ++
    ++	if (!strcmp(key, "allow"))
    ++		string_list_insert(&data->allowed_filters, buf.buf)->util =
    ++			(void *)(intptr_t)git_config_bool(var, value);
    ++
    ++	strbuf_release(&buf);
    ++	return 0;
     +}
     +
      static int upload_pack_config(const char *var, const char *value, void *cb_data)
  3:  3434bd5979 = 135:  550e4e13f1 upload-pack.c: pass 'struct list_objects_filter_options *'
  4:  9fa765a71d ! 136:  bb008f7427 upload-pack.c: introduce 'uploadpack.filter.tree.maxDepth'
    @@ Metadata
     Author: Taylor Blau <me@ttaylorr.com>
      ## Commit message ##
    -    upload-pack.c: introduce 'uploadpack.filter.tree.maxDepth'
    +    upload-pack.c: introduce 'uploadpackfilter.tree.maxDepth'
         In b79cf959b2 (upload-pack.c: allow banning certain object filter(s),
         2020-02-26), we introduced functionality to disallow certain object
    @@ Commit message
         While it would be sufficient to simply write
    -      $ git config uploadpack.filter.tree.allow true
    +      $ 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
    @@ Commit message
         In order to do this, introduce a new configuration key, as follows:
    -      $ git config uploadpack.filter.tree.maxDepth <m>
    +      $ 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 uploadpack.filter.tree.allow true
    -      $ git config uploadpack.filter.tree.maxDepth 0
    +      $ git config uploadpackfilter.tree.allow true
    +      $ git config uploadpackfilter.tree.maxDepth 0
         which allows '--filter=tree:0', but no other values.
    @@ Commit message
         Signed-off-by: Taylor Blau <me@ttaylorr.com>
      ## Documentation/config/uploadpack.txt ##
    -@@ Documentation/config/uploadpack.txt: Note that the dot between 'filter' and '<filter>' is both non-standard
    - and intentional. This is done to avoid a parsing ambiguity when
    - specifying this configuration as an argument to Git's top-level `-c`.
    +@@ Documentation/config/uploadpack.txt: uploadpackfilter.<filter>.allow::
    + 	combined filters, both `combine` and all of the nested filter
    + 	kinds must be allowed. Defaults to `uploadpackfilter.allow`.
    -+uploadpack.filter.tree.maxDepth::
    ++uploadpackfilter.tree.maxDepth::
     +	Only allow `--filter=tree=<n>` when `n` is no more than the value of
    -+	`uploadpack.filter.tree.maxDepth`. If set, this also implies
    -+	`uploadpack.filter.tree.allow=true`, unless this configuration
    ++	`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::
    @@ t/t5616-partial-clone.sh: test_expect_success 'upload-pack fails banned object f
      '
     +test_expect_success 'upload-pack limits tree depth filters' '
    -+	test_config -C srv.bare uploadpack.filter.allow false &&
    -+	test_config -C srv.bare uploadpack.filter.tree.allow true &&
    -+	test_config -C srv.bare uploadpack.filter.tree.maxDepth 0 &&
    ++	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
     +'
    @@ upload-pack.c: static void die_if_using_banned_filter(struct upload_pack_data *d
      	if (!banned)
      		return;
    --	packet_writer_error(&data->writer, _("filter '%s' not supported\n"),
    --			    list_object_filter_config_name(banned->choice));
    +-	die(_("git upload-pack: filter '%s' not supported"),
    +-	    list_object_filter_config_name(banned->choice));
     +	strbuf_addf(&buf, _("filter '%s' not supported"),
     +		    list_object_filter_config_name(banned->choice));
     +	if (banned->choice == LOFC_TREE_DEPTH &&
    @@ upload-pack.c: static void die_if_using_banned_filter(struct upload_pack_data *d
     +		strbuf_addf(&buf, _(" (maximum depth: %lu, but got: %lu)"),
     +			    data->tree_filter_max_depth,
     +			    banned->tree_exclude_depth);
    -+
    -+	packet_writer_error(&data->writer, "%s\n", buf.buf);
    -+
    -+	strbuf_release(&buf);
    -+
    - 	die(_("git upload-pack: banned object filter requested"));
    ++	die("%s", buf.buf);
      }
    -@@ upload-pack.c: static void parse_object_filter_config(const char *var, const char *value,
    - 				(void *)(intptr_t)git_config_bool(var, value);
    - 		else
    - 			data->allow_filter_fallback = git_config_bool(var, value);
    -+	} else if (!strcmp(spec.buf, "tree") && !strcmp(key, "maxdepth")) {
    -+		string_list_insert(&data->allowed_filters, "tree")->util
    -+			= (void *) (intptr_t) 1;
    + static void receive_needs(struct upload_pack_data *data,
    +@@ upload-pack.c: 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(&spec);
    + 	strbuf_release(&buf);
    + 	return 0;
--
2.28.0.rc1.13.ge78abce653
Next: Taylor Blau
Message 1 of 32 in “upload-pack: custom allowed object filters”
  1. 0/4 upload-pack: custom allowed object filtersTaylor Blau, Jul 23, 2020
  2. 1/4 list_objects_filter_options: introduce 'list_object_filter_config_name'Taylor Blau, Jul 23, 2020
  3. 2/4 upload-pack.c: allow banning certain object filter(s)Taylor Blau, Jul 23, 2020
  4. 3/4 upload-pack.c: pass 'struct list_objects_filter_options *'Taylor Blau, Jul 23, 2020
  5. 4/4 upload-pack.c: introduce 'uploadpackfilter.tree.maxDepth'Taylor Blau, Jul 23, 2020
  6. SZEDER GáborJul 23, 2020
  7. Taylor BlauJul 24, 2020
  8. Jeff KingJul 24, 2020
  9. Taylor BlauJul 27, 2020
  10. SZEDER GáborJul 27, 2020
  11. Taylor BlauJul 27, 2020
  12. Jeff KingJul 27, 2020
  13. SZEDER GáborJul 27, 2020
  14. Taylor BlauJul 27, 2020
  15. 0/4 upload-pack: custom allowed object filtersTaylor Blau, Jul 31, 2020
  16. 1/4 list_objects_filter_options: introduce 'list_object_filter_config_name'Taylor Blau, Jul 31, 2020
  17. 2/4 upload-pack.c: allow banning certain object filter(s)Taylor Blau, Jul 31, 2020
  18. Jeff KingJul 31, 2020
  19. Taylor BlauJul 31, 2020
  20. 3/4 upload-pack.c: pass 'struct list_objects_filter_options *'Taylor Blau, Jul 31, 2020
  21. 4/4 upload-pack.c: introduce 'uploadpackfilter.tree.maxDepth'Taylor Blau, Jul 31, 2020
  22. Jeff KingJul 31, 2020
  23. Jeff KingJul 31, 2020
  24. Taylor BlauJul 31, 2020
  25. Taylor BlauJul 31, 2020
  26. Jeff KingJul 31, 2020
  27. Jeff KingJul 31, 2020
  28. 0/3 upload-pack: custom allowed object filtersTaylor Blau, Aug 3, 2020
  29. 2/3 upload-pack.c: allow banning certain object filter(s)Taylor Blau, Aug 3, 2020
  30. 1/3 list_objects_filter_options: introduce 'list_object_filter_config_name'Taylor Blau, Aug 3, 2020
  31. 3/3 upload-pack.c: introduce 'uploadpackfilter.tree.maxDepth'Taylor Blau, Aug 3, 2020
  32. Jeff KingAug 4, 2020

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.