git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, don@goodman-wilson.com, stolee@gmail.com,
	sandals@crustytoothpaste.net, Matt Rogers <mattr94@gmail.com>,
	Eric Sunshine <sunshine@sunshineco.com>,
	Taylor Blau <me@ttaylorr.com>,
	Phillip Wood <phillip.wood123@gmail.com>,
	Alban Gruin <alban.gruin@gmail.com>, Johannes Sixt <j6t@kdbg.org>,
	Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: Re: [PATCH v2 02/12] fmt-merge-msg: introduce a way to override the main branch name
Date: Tue, 16 Jun 2020 09:04:56 -0400	[thread overview]
Message-ID: <20200616130456.GF666057@coredump.intra.peff.net> (raw)
In-Reply-To: <f4d547391537e5c3b0b4a07adb41b6aa56541fc3.1592225416.git.gitgitgadget@gmail.com>

On Mon, Jun 15, 2020 at 12:50:06PM +0000, Johannes Schindelin via GitGitGadget wrote:

> +char *repo_main_branch_name(struct repository *r)
> +{
> +	const char *config_key = "core.mainbranch";
> +	const char *config_display_key = "core.mainBranch";
> +	const char *fall_back = "master";
> +	char *name = NULL, *ret;
> +
> +	if (repo_config_get_string(r, config_key, &name) < 0)
> +		die(_("could not retrieve `%s`"), config_display_key);
> +
> +	ret = name ? name : xstrdup(fall_back);
> +
> +	if (check_refname_format(ret, REFNAME_ALLOW_ONELEVEL))
> +		die(_("invalid branch name: %s = %s"),
> +		    config_display_key, name);

Ah, this fixes the "we do not check the format of the short name" issue
I pointed out in v1 (sorry, I just realized that v2 existed so I'll
resume reviewing from there; I do still think this might make life
easier for callers by returning a const pointer).

I'm not sure if this check_refname_format() is valid, though. IIRC we've
had issues where "ONELEVEL" was used to check a branch name, but misses
some cases. The more full check done by strbuf_check_branch_ref()
actually creates the full refname and checks that. It also catches stuff
like refs/heads/HEAD.

I doubt that it matters too much for us to be completely thorough here
(unlike some other spots, we are not enforcing rules against potentially
malicious names, but rather just helping the user realize early that
their config is bogus). So I'm not sure how careful we want to be.

-Peff

  parent reply	other threads:[~2020-06-16 13:04 UTC|newest]

Thread overview: 180+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-10 21:19 [PATCH 0/9] Allow overriding the default name of the default branch Johannes Schindelin via GitGitGadget
2020-06-10 21:19 ` [PATCH 1/9] init: allow overriding the default branch name for new repositories Don Goodman-Wilson via GitGitGadget
2020-06-10 23:22   ` brian m. carlson
2020-06-11  0:16   ` Eric Sunshine
2020-06-11 14:09     ` Johannes Schindelin
2020-06-11 15:28       ` Junio C Hamano
2020-06-16 12:45     ` Jeff King
2020-06-16 12:47       ` Jeff King
2020-06-18 13:08         ` Johannes Schindelin
2020-06-23 20:32       ` Johannes Schindelin
2020-06-11  9:35   ` Phillip Wood
2020-06-12 11:55     ` Johannes Schindelin
2020-06-12 16:51       ` Junio C Hamano
2020-06-14 22:00         ` Johannes Schindelin
2020-06-15 10:00         ` Phillip Wood
2020-06-11 10:23   ` Alban Gruin
2020-06-11 23:14     ` Junio C Hamano
2020-06-11 23:46       ` brian m. carlson
2020-06-12 12:45         ` Johannes Schindelin
2020-06-13 18:01       ` Alban Gruin
2020-06-14  8:57         ` Johannes Schindelin
2020-06-16 12:25           ` Jeff King
2020-06-18 10:17             ` Johannes Schindelin
2020-06-10 21:19 ` [PATCH 2/9] remote: respect `core.defaultBranchName` Johannes Schindelin via GitGitGadget
2020-06-16 12:35   ` Jeff King
2020-06-18 10:21     ` Johannes Schindelin
2020-06-18 11:50       ` Jeff King
2020-06-23 21:15         ` Johannes Schindelin
2020-06-10 21:19 ` [PATCH 3/9] send-pack/transport-helper: " Johannes Schindelin via GitGitGadget
2020-06-10 21:19 ` [PATCH 4/9] testsvn: " Johannes Schindelin via GitGitGadget
2020-06-10 21:19 ` [PATCH 5/9] submodule: use the (possibly overridden) default branch name Johannes Schindelin via GitGitGadget
2020-06-15 10:46   ` Denton Liu
2020-06-10 21:19 ` [PATCH 6/9] clone: learn about the possibly-configured " Johannes Schindelin via GitGitGadget
2020-06-10 22:58   ` Junio C Hamano
2020-06-10 21:19 ` [PATCH 7/9] fmt-merge-msg: " Johannes Schindelin via GitGitGadget
2020-06-10 22:59   ` Junio C Hamano
2020-06-10 21:19 ` [PATCH 8/9] fast-export: respect the possibly-overridden " Johannes Schindelin via GitGitGadget
2020-06-10 21:54   ` Matt Rogers
2020-06-10 23:25     ` Junio C Hamano
2020-06-10 23:39     ` brian m. carlson
2020-06-11  0:20       ` Matt Rogers
2020-06-11  5:26         ` Junio C Hamano
2020-06-11 14:05           ` Johannes Schindelin
2020-06-11 15:05             ` Re* " Junio C Hamano
2020-06-11 16:44               ` Junio C Hamano
2020-06-11 18:18                 ` Junio C Hamano
2020-06-12 12:07                   ` Johannes Schindelin
2020-06-12 12:32                     ` Junio C Hamano
2020-06-12 12:03               ` Johannes Schindelin
2020-06-12 12:50                 ` Junio C Hamano
2020-06-12 12:53               ` Johannes Schindelin
2020-06-12 13:18                 ` Johannes Schindelin
2020-06-12 15:19                   ` Junio C Hamano
2020-06-12 15:22                     ` Junio C Hamano
2020-06-13  5:00                       ` Johannes Schindelin
2020-06-12 15:14                 ` Junio C Hamano
2020-06-13 11:49                   ` Johannes Sixt
2020-06-13 16:25                     ` Junio C Hamano
2020-06-13 14:47                       ` Johannes Schindelin
2020-06-13 18:49                         ` Junio C Hamano
2020-06-14  8:55                           ` Johannes Schindelin
2020-06-17 20:06                             ` Junio C Hamano
2020-06-23 21:11                               ` Johannes Schindelin
2020-06-23 21:32                                 ` Junio C Hamano
2020-06-13 14:44                   ` Johannes Schindelin
2020-06-11 13:57     ` Johannes Schindelin
2020-06-11 18:19       ` Junio C Hamano
2020-06-12 12:07         ` Johannes Schindelin
2020-06-10 21:19 ` [PATCH 9/9] Document how the default branch name can be overridden Johannes Schindelin via GitGitGadget
2020-06-11  0:18   ` Junio C Hamano
2020-06-10 23:11 ` [PATCH 0/9] Allow overriding the default name of the default branch Junio C Hamano
2020-06-11  5:42   ` Junio C Hamano
2020-06-11 13:44   ` Johannes Schindelin
2020-06-11 14:44     ` Junio C Hamano
2020-06-10 23:41 ` brian m. carlson
2020-06-11  1:07 ` Taylor Blau
2020-06-11 14:33   ` Johannes Schindelin
2020-06-15 10:03 ` Pratyush Yadav
2020-06-14 22:26   ` Johannes Schindelin
2020-06-16  0:19     ` Denton Liu
2020-06-23 20:10       ` Johannes Schindelin
2020-06-15 23:10   ` brian m. carlson
2020-06-15 12:50 ` [PATCH v2 00/12] " Johannes Schindelin via GitGitGadget
2020-06-15 12:50   ` [PATCH v2 01/12] fast-export: do anonymize the primary branch name Junio C Hamano via GitGitGadget
2020-06-16 12:58     ` Jeff King
2020-06-17 18:16       ` Junio C Hamano
2020-06-17 21:23         ` Jeff King
2020-06-18  2:06           ` Elijah Newren
2020-06-18  6:30             ` Junio C Hamano
2020-06-18  7:13               ` Elijah Newren
2020-06-18 11:45             ` Jeff King
2020-06-15 12:50   ` [PATCH v2 02/12] fmt-merge-msg: introduce a way to override the main " Johannes Schindelin via GitGitGadget
2020-06-15 15:00     ` Phillip Wood
2020-06-23 12:31       ` Johannes Schindelin
2020-06-15 17:05     ` Junio C Hamano
2020-06-23 19:19       ` Johannes Schindelin
2020-06-16  8:46     ` Ævar Arnfjörð Bjarmason
2020-06-17 18:21       ` Junio C Hamano
2020-06-16 13:04     ` Jeff King [this message]
2020-06-17 18:23       ` Junio C Hamano
2020-06-18 13:15         ` Johannes Schindelin
2020-06-17 20:56     ` Johannes Sixt
2020-06-17 21:16       ` Junio C Hamano
2020-06-23 21:12         ` Johannes Schindelin
2020-06-15 12:50   ` [PATCH v2 03/12] send-pack/transport-helper: respect `core.mainBranch` Johannes Schindelin via GitGitGadget
2020-06-15 12:50   ` [PATCH v2 04/12] git_main_branch_name(): optionally report the full ref name Johannes Schindelin via GitGitGadget
2020-06-15 15:04     ` Phillip Wood
2020-06-23 19:17       ` Johannes Schindelin
2020-06-15 12:50   ` [PATCH v2 05/12] fast-export: handle overridden main branch names correctly Johannes Schindelin via GitGitGadget
2020-06-15 15:05     ` Phillip Wood
2020-06-16 13:10       ` Jeff King
2020-06-16 15:49         ` Phillip Wood
2020-06-18 10:08         ` Johannes Schindelin
2020-06-15 17:09     ` Junio C Hamano
2020-06-23 19:22       ` Johannes Schindelin
2020-06-15 12:50   ` [PATCH v2 06/12] branch -m: adjust `core.mainBranch` if necessary Johannes Schindelin via GitGitGadget
2020-06-15 12:50   ` [PATCH v2 07/12] init: allow specifying the main branch name for the new repository Johannes Schindelin via GitGitGadget
2020-06-15 12:50   ` [PATCH v2 08/12] init: allow overriding the default main branch name via the config Don Goodman-Wilson via GitGitGadget
2020-06-15 12:50   ` [PATCH v2 09/12] clone: handle overridden main branch names Johannes Schindelin via GitGitGadget
2020-06-16 13:22     ` Jeff King
2020-06-23 20:58       ` Johannes Schindelin
2020-06-15 12:50   ` [PATCH v2 10/12] remote: learn about the possibly-overridden default main branch name Johannes Schindelin via GitGitGadget
2020-06-15 12:50   ` [PATCH v2 11/12] submodule: use the correct default for the " Johannes Schindelin via GitGitGadget
2020-06-16 13:46     ` Jeff King
2020-06-23 21:03       ` Johannes Schindelin
2020-06-23 21:14         ` Jeff King
2020-06-15 12:50   ` [PATCH v2 12/12] testsvn: respect `init.defaultBranch` Johannes Schindelin via GitGitGadget
2020-06-16 13:51     ` Jeff King
2020-06-23 21:07       ` Johannes Schindelin
2020-06-23 22:33   ` [PATCH v3 0/8] Allow overriding the default name of the default branch Johannes Schindelin via GitGitGadget
2020-06-23 22:33     ` [PATCH v3 1/8] fmt-merge-msg: stop treating `master` specially Johannes Schindelin via GitGitGadget
2020-06-24 16:16       ` Junio C Hamano
2020-06-25 13:07         ` Johannes Schindelin
2020-06-23 22:33     ` [PATCH v3 2/8] send-pack/transport-helper: avoid mentioning a particular branch Johannes Schindelin via GitGitGadget
2020-06-24  0:36       ` Junio C Hamano
2020-06-24 12:44         ` Johannes Schindelin
2020-06-24 15:44           ` Junio C Hamano
2020-06-25 13:05             ` Johannes Schindelin
2020-06-23 22:33     ` [PATCH v3 3/8] submodule: use a better fall-back for missing remote.<name>.branch Johannes Schindelin via GitGitGadget
2020-06-24  2:18       ` Philippe Blain
2020-06-24 12:51         ` Johannes Schindelin
2020-06-23 22:33     ` [PATCH v3 4/8] init: allow specifying the initial branch name for the new repository Johannes Schindelin via GitGitGadget
2020-06-24  0:58       ` Junio C Hamano
2020-06-24 12:55         ` Johannes Schindelin
2020-06-23 22:33     ` [PATCH v3 5/8] init: allow setting the default for the initial branch name via the config Don Goodman-Wilson via GitGitGadget
2020-06-24  1:05       ` Junio C Hamano
2020-06-24 12:56         ` Johannes Schindelin
2020-06-24 16:25           ` Junio C Hamano
2020-06-23 22:33     ` [PATCH v3 6/8] clone: use configured default branch name when appropriate Johannes Schindelin via GitGitGadget
2020-06-23 22:33     ` [PATCH v3 7/8] remote: use the " Johannes Schindelin via GitGitGadget
2020-06-24  1:10       ` Junio C Hamano
2020-06-24 13:00         ` Johannes Schindelin
2020-06-23 22:33     ` [PATCH v3 8/8] testsvn: respect `init.defaultBranch` Johannes Schindelin via GitGitGadget
2020-06-24 14:46     ` [PATCH v4 0/9] Allow overriding the default name of the default branch Johannes Schindelin via GitGitGadget
2020-06-24 14:46       ` [PATCH v4 1/9] fmt-merge-msg: stop treating `master` specially Johannes Schindelin via GitGitGadget
2020-06-29 16:20         ` Đoàn Trần Công Danh
2020-06-29 13:27           ` Johannes Schindelin
2020-06-30 15:05             ` Đoàn Trần Công Danh
2020-07-01 10:39               ` Johannes Schindelin
2020-07-01 19:54                 ` Junio C Hamano
2020-06-24 14:46       ` [PATCH v4 2/9] send-pack/transport-helper: avoid mentioning a particular branch Johannes Schindelin via GitGitGadget
2020-06-24 14:46       ` [PATCH v4 3/9] submodule: fall back to remote's HEAD for missing remote.<name>.branch Johannes Schindelin via GitGitGadget
2020-06-24 16:17         ` Junio C Hamano
2020-06-24 14:46       ` [PATCH v4 4/9] docs: add missing diamond brackets Johannes Schindelin via GitGitGadget
2020-06-24 14:46       ` [PATCH v4 5/9] init: allow specifying the initial branch name for the new repository Johannes Schindelin via GitGitGadget
2020-06-24 14:46       ` [PATCH v4 6/9] init: allow setting the default for the initial branch name via the config Don Goodman-Wilson via GitGitGadget
2020-06-24 14:46       ` [PATCH v4 7/9] clone: use configured default branch name when appropriate Johannes Schindelin via GitGitGadget
2020-06-24 14:46       ` [PATCH v4 8/9] remote: use the " Johannes Schindelin via GitGitGadget
2020-06-24 14:46       ` [PATCH v4 9/9] testsvn: respect `init.defaultBranch` Johannes Schindelin via GitGitGadget
2020-06-24 16:26       ` [PATCH v4 0/9] Allow overriding the default name of the default branch Junio C Hamano
2020-06-25 13:03         ` Johannes Schindelin
2020-06-29 22:41       ` brian m. carlson
2020-07-12 13:03         ` Edward Thomson
2020-07-12  8:19           ` Johannes Schindelin
     [not found]             ` <CA+WKDT1GMNTY5N862-7ui70D6-b1u6fuUkvctEYo+57aJGbjmw@mail.gmail.com>
2020-07-14 14:55               ` Johannes Schindelin
2020-06-16  9:47 ` [PATCH " Ævar Arnfjörð Bjarmason
2020-06-16 14:09   ` Jeff King
2020-06-16 14:24     ` Jeff King
2020-06-23 20:28       ` Johannes Schindelin
2020-06-17 20:28   ` 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

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

  git send-email \
    --in-reply-to=20200616130456.GF666057@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=alban.gruin@gmail.com \
    --cc=don@goodman-wilson.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=j6t@kdbg.org \
    --cc=johannes.schindelin@gmx.de \
    --cc=mattr94@gmail.com \
    --cc=me@ttaylorr.com \
    --cc=phillip.wood123@gmail.com \
    --cc=sandals@crustytoothpaste.net \
    --cc=stolee@gmail.com \
    --cc=sunshine@sunshineco.com \
    --subject='Re: [PATCH v2 02/12] fmt-merge-msg: introduce a way to override the main branch name' \
    /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

This is a public inbox, see mirroring instructions
on how to clone and mirror all data and code used for this inbox