From: Philip Oakley <philipoakley@iee.email>
To: Taylor Blau <me@ttaylorr.com>, git@vger.kernel.org
Cc: christian.couder@gmail.com, peff@peff.net, james@jramsay.com.au
Subject: Re: [RFC PATCH 2/2] upload-pack.c: allow banning certain object filter(s)
Date: Wed, 18 Mar 2020 11:18:16 +0000 [thread overview]
Message-ID: <13dd0152-b20a-51e1-5940-5e4b67242e9b@iee.email> (raw)
In-Reply-To: <888d9484cf4130e90f451134c236a290a6c5e18d.1584477196.git.me@ttaylorr.com>
Hi
On 17/03/2020 20:39, Taylor Blau wrote:
> Git clients may ask the server for a partial set of objects, where the
> set of objects being requested is refined by one or more object filters.
> Server administrators can configure 'git upload-pack' to allow or ban
> these filters by setting the 'uploadpack.allowFilter' variable to
> 'true' or 'false', respectively.
>
> However, administrators using bitmaps may wish to allow certain kinds of
> object filters, but ban others. Specifically, they may wish to allow
> object filters that can be optimized by the use of bitmaps, while
> rejecting other object filters which aren't and represent a perceived
> performance degradation (as well as an increased load factor on the
> server).
>
> Allow configuring 'git upload-pack' to support object filters on a
> case-by-case basis by introducing a new configuration variable and
> section:
>
> - 'uploadpack.filter.allow'
>
> - 'uploadpack.filter.<kind>.allow'
>
> where '<kind>' may be one of 'blob:none', 'blob:limit', 'tree:depth',
> and so on. The additional '.' between 'filter' and '<kind>' is part of
> the sub-section.
>
> Setting the second configuration variable for any valid value of
> '<kind>' explicitly allows or disallows restricting that kind of object
> filter.
>
> 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
> maintain backwards compatibility. Note that this differs from
> 'uploadpack.allowfilter', which controls whether or not the 'filter'
> capability is advertised.
>
> NB: this introduces an unfortunate possibility that attempt to write the
> ERR sideband will cause a SIGPIPE. This can be prevented by some of
> SZEDZER's previous work, but it is silenced in 't' for now.
> ---
> Documentation/config/uploadpack.txt | 12 ++++++
> t/t5616-partial-clone.sh | 23 ++++++++++
> upload-pack.c | 67 +++++++++++++++++++++++++++++
> 3 files changed, 102 insertions(+)
>
> diff --git a/Documentation/config/uploadpack.txt b/Documentation/config/uploadpack.txt
> index ed1c835695..6213bd619c 100644
> --- a/Documentation/config/uploadpack.txt
> +++ b/Documentation/config/uploadpack.txt
> @@ -57,6 +57,18 @@ uploadpack.allowFilter::
> If this option is set, `upload-pack` will support partial
> clone and partial fetch object filtering.
>
> +uploadpack.filter.allow::
> + Provides a default value for unspecified object filters (see: the
> + below configuration variable).
> + Defaults to `true`.
> +
> +uploadpack.filter.<filter>.allow::
> + Explicitly allow or ban the object filter corresponding to `<filter>`,
> + where `<filter>` may be one of: `blob:none`, `blob:limit`, `tree:depth`,
> + `sparse:oid`, or `combine`. If using combined filters, both `combine`
> + and all of the nested filter kinds must be allowed.
Doesn't the man page at least need the part from the commit message "The
additional '.' between 'filter' and '<kind>' is part of
the sub-section." as it's not a common mechanism (other comments not
withstanding)
Philip
> + Defaults to `uploadpack.filter.allow`.
> +
> 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 77bb91e976..ee1af9b682 100755
> --- a/t/t5616-partial-clone.sh
> +++ b/t/t5616-partial-clone.sh
> @@ -235,6 +235,29 @@ test_expect_success 'implicitly construct combine: filter with repeated flags' '
> test_cmp unique_types.expected unique_types.actual
> '
>
> +test_expect_success 'upload-pack fails banned object filters' '
> + # Ensure that configuration keys are normalized by capitalizing
> + # "blob:None" below:
> + test_config -C srv.bare uploadpack.filter.blob:None.allow false &&
> + test_must_fail ok=sigpipe git clone --no-checkout --filter.blob:none \
> + "file://$(pwd)/srv.bare" pc3
> +'
> +
> +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:depth.allow true &&
> + test_config -C srv.bare uploadpack.filter.blob:none.allow false &&
> + test_must_fail ok=sigpipe git clone --no-checkout --filter=tree:1 \
> + --filter=blob:none "file://$(pwd)/srv.bare" pc3
> +'
> +
> +test_expect_success 'upload-pack fails banned object filters with fallback' '
> + test_config -C srv.bare uploadpack.filter.allow false &&
> + test_must_fail ok=sigpipe git clone --no-checkout --filter=blob:none \
> + "file://$(pwd)/srv.bare" pc3
> +'
> +
> 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 c53249cac1..81f2701f99 100644
> --- a/upload-pack.c
> +++ b/upload-pack.c
> @@ -69,6 +69,8 @@ static int filter_capability_requested;
> static int allow_filter;
> static int allow_ref_in_want;
> static struct list_objects_filter_options filter_options;
> +static struct string_list allowed_filters = STRING_LIST_INIT_DUP;
> +static int allow_filter_fallback = 1;
>
> static int allow_sideband_all;
>
> @@ -848,6 +850,45 @@ static int process_deepen_not(const char *line, struct string_list *deepen_not,
> return 0;
> }
>
> +static int allows_filter_choice(enum list_objects_filter_choice c)
> +{
> + const char *key = list_object_filter_config_name(c);
> + struct string_list_item *item = string_list_lookup(&allowed_filters,
> + key);
> + if (item)
> + return (intptr_t) item->util;
> + return allow_filter_fallback;
> +}
> +
> +static struct list_objects_filter_options *banned_filter(
> + struct list_objects_filter_options *opts)
> +{
> + size_t i;
> +
> + if (!allows_filter_choice(opts->choice))
> + return opts;
> +
> + if (opts->choice == LOFC_COMBINE)
> + for (i = 0; i < opts->sub_nr; i++) {
> + struct list_objects_filter_options *sub = &opts->sub[i];
> + if (banned_filter(sub))
> + return sub;
> + }
> + return NULL;
> +}
> +
> +static void die_if_using_banned_filter(struct packet_writer *w,
> + struct list_objects_filter_options *opts)
> +{
> + struct list_objects_filter_options *banned = banned_filter(opts);
> + if (!banned)
> + return;
> +
> + packet_writer_error(w, _("filter '%s' not supported\n"),
> + list_object_filter_config_name(banned->choice));
> + die(_("git upload-pack: banned object filter requested"));
> +}
> +
> static void receive_needs(struct packet_reader *reader, struct object_array *want_obj)
> {
> struct object_array shallows = OBJECT_ARRAY_INIT;
> @@ -885,6 +926,7 @@ static void receive_needs(struct packet_reader *reader, struct object_array *wan
> die("git upload-pack: filtering capability not negotiated");
> list_objects_filter_die_if_populated(&filter_options);
> parse_list_objects_filter(&filter_options, arg);
> + die_if_using_banned_filter(&writer, &filter_options);
> continue;
> }
>
> @@ -1044,6 +1086,9 @@ static int find_symref(const char *refname, const struct object_id *oid,
>
> static int upload_pack_config(const char *var, const char *value, void *unused)
> {
> + const char *sub, *key;
> + int sub_len;
> +
> if (!strcmp("uploadpack.allowtipsha1inwant", var)) {
> if (git_config_bool(var, value))
> allow_unadvertised_object_request |= ALLOW_TIP_SHA1;
> @@ -1065,6 +1110,26 @@ static int upload_pack_config(const char *var, const char *value, void *unused)
> keepalive = -1;
> } else if (!strcmp("uploadpack.allowfilter", var)) {
> allow_filter = git_config_bool(var, value);
> + } else if (!parse_config_key(var, "uploadpack", &sub, &sub_len, &key) &&
> + key && !strcmp(key, "allow")) {
> + if (sub && skip_prefix(sub, "filter.", &sub) && sub_len >= 7) {
> + struct string_list_item *item;
> + char *spec;
> +
> + /*
> + * normalize the filter, and chomp off '.allow' from the
> + * end
> + */
> + spec = xstrdup_tolower(sub);
> + spec[sub_len - 7] = 0;
> +
> + item = string_list_insert(&allowed_filters, spec);
> + item->util = (void *) (intptr_t) git_config_bool(var, value);
> +
> + free(spec);
> + } else if (!strcmp("uploadpack.filter.allow", var)) {
> + allow_filter_fallback = git_config_bool(var, value);
> + }
> } else if (!strcmp("uploadpack.allowrefinwant", var)) {
> allow_ref_in_want = git_config_bool(var, value);
> } else if (!strcmp("uploadpack.allowsidebandall", var)) {
> @@ -1308,6 +1373,8 @@ static void process_args(struct packet_reader *request,
> if (allow_filter && skip_prefix(arg, "filter ", &p)) {
> list_objects_filter_die_if_populated(&filter_options);
> parse_list_objects_filter(&filter_options, p);
> + die_if_using_banned_filter(&data->writer,
> + &filter_options);
> continue;
> }
>
next prev parent reply other threads:[~2020-03-18 11:18 UTC|newest]
Thread overview: 125+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-12 3:55 Notes from Git Contributor Summit, Los Angeles (April 5, 2020) James Ramsay
2020-03-12 3:56 ` [TOPIC 1/17] Reftable James Ramsay
2020-03-12 3:56 ` [TOPIC 2/17] Hooks in the future James Ramsay
2020-03-12 14:16 ` Emily Shaffer
2020-03-13 17:56 ` Junio C Hamano
2020-04-07 23:01 ` Emily Shaffer
2020-04-07 23:51 ` Emily Shaffer
2020-04-08 0:40 ` Junio C Hamano
2020-04-08 1:09 ` Emily Shaffer
2020-04-10 21:31 ` Jeff King
2020-04-13 19:15 ` Emily Shaffer
2020-04-13 21:52 ` Jeff King
2020-04-14 0:54 ` [RFC PATCH v2 0/2] configuration-based hook management (was: [TOPIC 2/17] Hooks in the future) Emily Shaffer
2020-04-14 0:54 ` [RFC PATCH v2 1/2] hook: scaffolding for git-hook subcommand Emily Shaffer
2020-04-14 0:54 ` [RFC PATCH v2 2/2] hook: add --list mode Emily Shaffer
2020-04-14 15:15 ` [RFC PATCH v2 0/2] configuration-based hook management Phillip Wood
2020-04-14 19:24 ` Emily Shaffer
2020-04-14 20:27 ` Jeff King
2020-04-15 10:01 ` Phillip Wood
2020-04-14 20:03 ` Josh Steadmon
2020-04-15 10:08 ` Phillip Wood
2020-04-14 20:32 ` Jeff King
2020-04-15 10:01 ` Phillip Wood
2020-04-15 14:51 ` Junio C Hamano
2020-04-15 20:30 ` Emily Shaffer
2020-04-15 22:19 ` Junio C Hamano
2020-04-15 3:45 ` [TOPIC 2/17] Hooks in the future Jonathan Nieder
2020-04-15 20:59 ` Emily Shaffer
2020-04-20 23:53 ` [PATCH] doc: propose hooks managed by the config Emily Shaffer
2020-04-21 0:22 ` Emily Shaffer
2020-04-21 1:20 ` Junio C Hamano
2020-04-24 23:14 ` Emily Shaffer
2020-04-25 20:57 ` brian m. carlson
2020-05-06 21:33 ` Emily Shaffer
2020-05-06 23:13 ` brian m. carlson
2020-05-19 20:10 ` Emily Shaffer
2020-04-15 22:42 ` [TOPIC 2/17] Hooks in the future Jeff King
2020-04-15 22:48 ` Emily Shaffer
2020-04-15 22:57 ` Jeff King
2020-03-12 3:57 ` [TOPIC 3/17] Obliterate James Ramsay
2020-03-12 18:06 ` Konstantin Ryabitsev
2020-03-15 22:19 ` Damien Robert
2020-03-16 12:55 ` Konstantin Tokarev
2020-03-26 22:27 ` Damien Robert
2020-03-16 16:32 ` Elijah Newren
2020-03-26 22:30 ` Damien Robert
2020-03-16 18:32 ` Phillip Susi
2020-03-26 22:37 ` Damien Robert
2020-03-16 20:01 ` Philip Oakley
2020-05-16 2:21 ` nbelakovski
2020-03-12 3:58 ` [TOPIC 4/17] Sparse checkout James Ramsay
2020-03-12 4:00 ` [TOPIC 5/17] Partial Clone James Ramsay
2020-03-17 7:38 ` Allowing only blob filtering was: " Christian Couder
2020-03-17 20:39 ` [RFC PATCH 0/2] upload-pack.c: limit allowed filter choices Taylor Blau
2020-03-17 20:39 ` [RFC PATCH 1/2] list_objects_filter_options: introduce 'list_object_filter_config_name' Taylor Blau
2020-03-17 20:53 ` Eric Sunshine
2020-03-18 10:03 ` Jeff King
2020-03-18 19:40 ` Junio C Hamano
2020-03-18 22:38 ` Eric Sunshine
2020-03-19 17:15 ` Jeff King
2020-03-18 21:05 ` Taylor Blau
2020-03-17 20:39 ` [RFC PATCH 2/2] upload-pack.c: allow banning certain object filter(s) Taylor Blau
2020-03-17 21:11 ` Eric Sunshine
2020-03-18 21:18 ` Taylor Blau
2020-03-18 11:18 ` Philip Oakley [this message]
2020-03-18 21:20 ` Taylor Blau
2020-03-18 10:18 ` [RFC PATCH 0/2] upload-pack.c: limit allowed filter choices Jeff King
2020-03-18 18:26 ` Re*: " Junio C Hamano
2020-03-19 17:03 ` Jeff King
2020-03-18 21:28 ` Taylor Blau
2020-03-18 22:41 ` Junio C Hamano
2020-03-19 17:10 ` Jeff King
2020-03-19 17:09 ` Jeff King
2020-04-17 9:41 ` Christian Couder
2020-04-17 17:40 ` Taylor Blau
2020-04-17 18:06 ` Jeff King
2020-04-21 12:34 ` Christian Couder
2020-04-22 20:41 ` Taylor Blau
2020-04-22 20:42 ` Taylor Blau
2020-04-21 12:17 ` Christian Couder
2020-03-12 4:01 ` [TOPIC 6/17] GC strategies James Ramsay
2020-03-12 4:02 ` [TOPIC 7/17] Background operations/maintenance James Ramsay
2020-03-12 4:03 ` [TOPIC 8/17] Push performance James Ramsay
2020-03-12 4:04 ` [TOPIC 9/17] Obsolescence markers and evolve James Ramsay
2020-05-09 21:31 ` Noam Soloveichik
2020-05-15 22:26 ` Jeff King
2020-03-12 4:05 ` [TOPIC 10/17] Expel ‘git shell’? James Ramsay
2020-03-12 4:07 ` [TOPIC 11/17] GPL enforcement James Ramsay
2020-03-12 4:08 ` [TOPIC 12/17] Test harness improvements James Ramsay
2020-03-12 4:09 ` [TOPIC 13/17] Cross implementation test suite James Ramsay
2020-03-12 4:11 ` [TOPIC 14/17] Aspects of merge-ort: cool, or crimes against humanity? James Ramsay
2020-03-12 4:13 ` [TOPIC 15/17] Reachability checks James Ramsay
2020-03-12 4:14 ` [TOPIC 16/17] “I want a reviewer” James Ramsay
2020-03-12 13:31 ` Emily Shaffer
2020-03-12 17:31 ` Konstantin Ryabitsev
2020-03-12 17:42 ` Jonathan Nieder
2020-03-12 18:00 ` Konstantin Ryabitsev
2020-03-17 0:43 ` Philippe Blain
2020-03-13 21:25 ` Eric Wong
2020-03-14 17:27 ` Jeff King
2020-03-15 0:36 ` inbox indexing wishlist [was: [TOPIC 16/17] “I want a reviewer”] Eric Wong
2020-03-12 4:16 ` [TOPIC 17/17] Security James Ramsay
2020-03-12 14:38 ` Notes from Git Contributor Summit, Los Angeles (April 5, 2020) Derrick Stolee
2020-03-13 20:47 ` Jeff King
2020-03-15 18:42 ` Jakub Narebski
2020-03-16 19:31 ` Jeff King
-- strict thread matches above, loose matches on Subject: below --
2019-12-10 2:33 [PATCH 0/6] configuration-based hook management Emily Shaffer
2019-12-10 2:33 ` [PATCH 1/6] hook: scaffolding for git-hook subcommand Emily Shaffer
2019-12-12 9:41 ` Bert Wesarg
2019-12-12 10:47 ` SZEDER Gábor
2019-12-10 2:33 ` [PATCH 2/6] config: add string mapping for enum config_scope Emily Shaffer
2019-12-10 11:16 ` Philip Oakley
2019-12-10 17:21 ` Philip Oakley
2019-12-10 2:33 ` [PATCH 3/6] hook: add --list mode Emily Shaffer
2019-12-12 9:38 ` Bert Wesarg
2019-12-12 10:58 ` SZEDER Gábor
2019-12-10 2:33 ` [PATCH 4/6] hook: support reordering of hook list Emily Shaffer
2019-12-11 19:21 ` Junio C Hamano
2019-12-10 2:33 ` [PATCH 5/6] hook: remove prior hook with '---' Emily Shaffer
2019-12-10 2:33 ` [PATCH 6/6] hook: teach --porcelain mode Emily Shaffer
2019-12-11 19:33 ` Junio C Hamano
2019-12-11 22:00 ` Emily Shaffer
2019-12-11 22:07 ` Junio C Hamano
2019-12-11 23:15 ` Emily Shaffer
2019-12-11 22:42 ` [PATCH 0/6] configuration-based hook management Junio C Hamano
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=13dd0152-b20a-51e1-5940-5e4b67242e9b@iee.email \
--to=philipoakley@iee.email \
--cc=christian.couder@gmail.com \
--cc=git@vger.kernel.org \
--cc=james@jramsay.com.au \
--cc=me@ttaylorr.com \
--cc=peff@peff.net \
/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).