Re: [PATCH 3/3] *: use allowlist and denylist

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Stolee,

On Wed, 13 Jul 2022, Derrick Stolee via GitGitGadget wrote:

> From: Derrick Stolee <derrickstolee@github.com>
>
> Using "allowlist" and "denylist" is a more precise definition of the
> functionality they provide. The previous color-based words assume
> cultural interpretation to provide the meaning.
>
> These changes to the Git codebase are mostly cosmetic. Several comments
> are updated. The renamed protocol_allowlist() method is local to
> transport.c so does not update any header file API definition. There are
> some untranslated error messages that are reworded, so this _might_
> affect error parsers. However, two of the three error messages are
> around option parsing, so they are "fast failures". The one perhaps
> meaningful change is the logerror() in daemon.c.

I do consider `git daemon` less important these days because we have two
transports that are secure (which `git://` is not): HTTPS and SSH. That
suggests that the `daemon.c` change might have a very low impact to begin
with.

The other changes affect `cvsserver` which I consider even less important.
In fact, I would be in favor of deprecating it soon and of deleting it in
due time.

All the changes in this patch look good to me: ACK!

Thank you!
Dscho

>
> After this change, the only remaining uses of the previous words are
> in release notes for older versions of Git.
>
> Signed-off-by: Derrick Stolee <derrickstolee@github.com>
> ---
>  daemon.c           |  8 ++++----
>  git-cvsserver.perl |  2 +-
>  sha1dc/sha1.c      | 12 ++++++------
>  transport.c        |  8 ++++----
>  4 files changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/daemon.c b/daemon.c
> index 58f1077885c..ed7c53b1110 100644
> --- a/daemon.c
> +++ b/daemon.c
> @@ -279,7 +279,7 @@ static const char *path_ok(const char *directory, struct hostinfo *hi)
>  		/* The validation is done on the paths after enter_repo
>  		 * appends optional {.git,.git/.git} and friends, but
>  		 * it does not use getcwd().  So if your /pub is
> -		 * a symlink to /mnt/pub, you can whitelist /pub and
> +		 * a symlink to /mnt/pub, you can allowlist /pub and
>  		 * do not have to say /mnt/pub.
>  		 * Do not say /pub/.
>  		 */
> @@ -298,7 +298,7 @@ static const char *path_ok(const char *directory, struct hostinfo *hi)
>  			return path;
>  	}
>
> -	logerror("'%s': not in whitelist", path);
> +	logerror("'%s': not in allowlist", path);
>  	return NULL;		/* Fallthrough. Deny by default */
>  }
>
> @@ -403,7 +403,7 @@ static int run_service(const char *dir, struct daemon_service *service,
>  	 * a "git-daemon-export-ok" flag that says that the other side
>  	 * is ok with us doing this.
>  	 *
> -	 * path_ok() uses enter_repo() and does whitelist checking.
> +	 * path_ok() uses enter_repo() and does allowlist checking.
>  	 * We only need to make sure the repository is exported.
>  	 */
>
> @@ -1444,7 +1444,7 @@ int cmd_main(int argc, const char **argv)
>  		cred = prepare_credentials(user_name, group_name);
>
>  	if (strict_paths && (!ok_paths || !*ok_paths))
> -		die("option --strict-paths requires a whitelist");
> +		die("option --strict-paths requires a allowlist");
>
>  	if (base_path && !is_directory(base_path))
>  		die("base-path '%s' does not exist or is not a directory",
> diff --git a/git-cvsserver.perl b/git-cvsserver.perl
> index 4c8118010a8..7d13b0a5ac1 100755
> --- a/git-cvsserver.perl
> +++ b/git-cvsserver.perl
> @@ -152,7 +152,7 @@ $state->{allowed_roots} = [ @ARGV ];
>
>  # don't export the whole system unless the users requests it
>  if ($state->{'export-all'} && !@{$state->{allowed_roots}}) {
> -    die "--export-all can only be used together with an explicit whitelist\n";
> +    die "--export-all can only be used together with an explicit allowlist\n";
>  }
>
>  # Environment handling for running under git-shell
> diff --git a/sha1dc/sha1.c b/sha1dc/sha1.c
> index dede2cbddf9..b4a5f23c1ec 100644
> --- a/sha1dc/sha1.c
> +++ b/sha1dc/sha1.c
> @@ -86,30 +86,30 @@
>         defined(__MIPSEB__) || defined(__MIPSEB) || defined(_MIPSEB) || \
>         defined(__sparc))
>  /*
> - * Should define Big Endian for a whitelist of known processors. See
> + * Should define Big Endian for a allowlist of known processors. See
>   * https://sourceforge.net/p/predef/wiki/Endianness/ and
>   * http://www.oracle.com/technetwork/server-storage/solaris/portingtosolaris-138514.html
>   */
>  #define SHA1DC_BIGENDIAN
>
> -/* Not under GCC-alike or glibc or *BSD or newlib or <processor whitelist> */
> +/* Not under GCC-alike or glibc or *BSD or newlib or <processor allowlist> */
>  #elif (defined(_AIX) || defined(__hpux))
>
>  /*
> - * Defines Big Endian on a whitelist of OSs that are known to be Big
> + * Defines Big Endian on a allowlist of OSs that are known to be Big
>   * Endian-only. See
>   * https://lore.kernel.org/git/93056823-2740-d072-1ebd-46b440b33d7e@felt.demon.nl/
>   */
>  #define SHA1DC_BIGENDIAN
>
> -/* Not under GCC-alike or glibc or *BSD or newlib or <processor whitelist> or <os whitelist> */
> +/* Not under GCC-alike or glibc or *BSD or newlib or <processor allowlist> or <os allowlist> */
>  #elif defined(SHA1DC_ON_INTEL_LIKE_PROCESSOR)
>  /*
>   * As a last resort before we do anything else we're not 100% sure
> - * about below, we blacklist specific processors here. We could add
> + * about below, we denylist specific processors here. We could add
>   * more, see e.g. https://wiki.debian.org/ArchitectureSpecificsMemo
>   */
> -#else /* Not under GCC-alike or glibc or *BSD or newlib or <processor whitelist> or <os whitelist> or <processor blacklist> */
> +#else /* Not under GCC-alike or glibc or *BSD or newlib or <processor allowlist> or <os allowlist> or <processor denylist> */
>
>  /* We do nothing more here for now */
>  /*#error "Uncomment this to see if you fall through all the detection"*/
> diff --git a/transport.c b/transport.c
> index 52db7a3cb09..321bbe382cc 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -940,7 +940,7 @@ static int external_specification_len(const char *url)
>  	return strchr(url, ':') - url;
>  }
>
> -static const struct string_list *protocol_whitelist(void)
> +static const struct string_list *protocol_allowlist(void)
>  {
>  	static int enabled = -1;
>  	static struct string_list allowed = STRING_LIST_INIT_DUP;
> @@ -1020,9 +1020,9 @@ static enum protocol_allow_config get_protocol_config(const char *type)
>
>  int is_transport_allowed(const char *type, int from_user)
>  {
> -	const struct string_list *whitelist = protocol_whitelist();
> -	if (whitelist)
> -		return string_list_has_string(whitelist, type);
> +	const struct string_list *allowlist = protocol_allowlist();
> +	if (allowlist)
> +		return string_list_has_string(allowlist, type);
>
>  	switch (get_protocol_config(type)) {
>  	case PROTOCOL_ALLOW_ALWAYS:
> --
> gitgitgadget
>