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@xxxxxxxxxxxx>
## 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@xxxxxxxxxxxx>
## 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
