{"thread":{"id":"58152","subject":"[PATCH 0/3] Use \"allowlist\" and \"denylist\" tree-wide","startedAt":"2022-07-13T13:21:00Z","lastAt":"2022-07-31T00:35:15Z","messageCount":38,"participants":["Derrick Stolee via GitGitGadget","Johannes Schindelin","Jeff King","Junio C Hamano","Derrick Stolee","Ævar Arnfjörð Bjarmason","Phillip Wood"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"458973","messageId":"pull.1274.git.1657718450.gitgitgadget@gmail.com","threadId":"58152","inReplyTo":null,"subject":"[PATCH 0/3] Use \"allowlist\" and \"denylist\" tree-wide","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-07-13T13:20:47Z","receivedAt":"2022-07-13T13:21:00Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"The terms \"allowlist\" and \"denylist\" are self-defining. One \"allows\" things\nwhile the other \"denies\" things.\n\nThese are better terms over \"whitelist\" and \"blacklist\" which require prior\nknowledge of the terms or cultural expectations around what each color\n\"means\".\n\nThis series replaces (almost) all uses of these terms with allowlist and\ndenylist. The only exceptions are in release notes for older Git versions.\n\nThere is no meaningful functional change, although one logging message in\ndaemon.c is changed and I'm unfamiliar with exactly how that might be\nconsumed.\n\nSome recommend using \"blocklist\", but I personally prefer \"denylist\". To me,\n\"blocking\" something seems permanent. \"Denying\" something seems ephemeral\nand related to a specific request being denied due to some (possibly\nmutable) state. I'm open to suggestions here. There are many fewer\nreplacements needed in this case.\n\nI did not make any change to our CodingGuidelines. Hopefully having clear\nusage throughout the codebase will be enough to promote using consistent\nterminology.\n\nThanks, -Stolee\n\nDerrick Stolee (3):\n Documentation: use allowlist and denylist\n t/*: use allowlist\n *: use allowlist and denylist\n\n Documentation/git-cvsserver.txt | 2 +-\n Documentation/git-daemon.txt | 10 +++++-----\n Documentation/git.txt | 2 +-\n daemon.c | 8 ++++----\n git-cvsserver.perl | 2 +-\n sha1dc/sha1.c | 12 ++++++------\n t/README | 4 ++--\n t/lib-proto-disable.sh | 6 +++---\n t/t5812-proto-disable-http.sh | 2 +-\n t/t5815-submodule-protos.sh | 4 ++--\n t/t9400-git-cvsserver-server.sh | 2 +-\n t/test-lib-functions.sh | 2 +-\n t/test-lib.sh | 2 +-\n transport.c | 8 ++++----\n 14 files changed, 33 insertions(+), 33 deletions(-)\n\n\nbase-commit: e4a4b31577c7419497ac30cebe30d755b97752c5\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1274%2Fderrickstolee%2Fallow-deny-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1274/derrickstolee/allow-deny-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1274\n-- \ngitgitgadget\n"},{"id":"458974","messageId":"ec81aac05c40318755f5311a20e8f9cc55d289fc.1657718450.git.gitgitgadget@gmail.com","threadId":"58152","inReplyTo":"pull.1274.git.1657718450.gitgitgadget@gmail.com","subject":"[PATCH 1/3] Documentation: use allowlist and denylist","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-07-13T13:20:48Z","receivedAt":"2022-07-13T13:21:03Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nUsing \"allowlist\" and \"denylist\" is a more precise definition of the\nfunctionality they provide. The previous color-based words assume\ncultural interpretation to provide the meaning.\n\nFocus on replacements in the Documentation/ directory since these are\nnot functional uses.\n\nSigned-off-by: Derrick Stolee \n---\n Documentation/git-cvsserver.txt | 2 +-\n Documentation/git-daemon.txt | 10 +++++-----\n Documentation/git.txt | 2 +-\n 3 files changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-cvsserver.txt b/Documentation/git-cvsserver.txt\nindex 4dc57ed2547..40ea1f3af8e 100644\n--- a/Documentation/git-cvsserver.txt\n+++ b/Documentation/git-cvsserver.txt\n@@ -313,7 +313,7 @@ circumstances, allowing easier restricted usage through git-shell.\n \n GIT_CVSSERVER_BASE_PATH takes the place of the argument to --base-path.\n \n-GIT_CVSSERVER_ROOT specifies a single-directory whitelist. The\n+GIT_CVSSERVER_ROOT specifies a single-directory allowlist. The\n repository must still be configured to allow access through\n git-cvsserver, as described above.\n \ndiff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt\nindex fdc28c041c7..ff74a90aead 100644\n--- a/Documentation/git-daemon.txt\n+++ b/Documentation/git-daemon.txt\n@@ -33,7 +33,7 @@ It verifies that the directory has the magic file \"git-daemon-export-ok\", and\n it will refuse to export any Git directory that hasn't explicitly been marked\n for export this way (unless the `--export-all` parameter is specified). If you\n pass some directory paths as 'git daemon' arguments, you can further restrict\n-the offers to a whitelist comprising of those.\n+the offers to a allowlist comprising of those.\n \n By default, only `upload-pack` service is enabled, which serves\n 'git fetch-pack' and 'git ls-remote' clients, which are invoked\n@@ -50,7 +50,7 @@ OPTIONS\n \tMatch paths exactly (i.e. don't allow \"/foo/repo\" when the real path is\n \t\"/foo/repo.git\" or \"/foo/repo/.git\") and don't do user-relative paths.\n \t'git daemon' will refuse to start when this option is enabled and no\n-\twhitelist is specified.\n+\tallowlist is specified.\n \n --base-path=::\n \tRemap all the path requests as relative to the given path.\n@@ -73,7 +73,7 @@ OPTIONS\n \t%IP for the server's IP address, %P for the port number,\n \tand %D for the absolute path of the named repository.\n \tAfter interpolation, the path is validated against the directory\n-\twhitelist.\n+\tallowlist.\n \n --export-all::\n \tAllow pulling from all directories that look like Git repositories\n@@ -218,7 +218,7 @@ standard output to be sent to the requestor as an error message when\n it declines the service.\n \n ::\n-\tA directory to add to the whitelist of allowed directories. Unless\n+\tA directory to add to the allowlist of allowed directories. Unless\n \t--strict-paths is specified this will also include subdirectories\n \tof each named directory.\n \n@@ -264,7 +264,7 @@ git\t\t9418/tcp\t\t# Git Version Control System\n \n 'git daemon' as inetd server::\n \tTo set up 'git daemon' as an inetd service that handles any\n-\trepository under the whitelisted set of directories, /pub/foo\n+\trepository under the allowlisted set of directories, /pub/foo\n \tand /pub/bar, place an entry like the following into\n \t/etc/inetd all on one line:\n +\ndiff --git a/Documentation/git.txt b/Documentation/git.txt\nindex 302607a4967..384718ee677 100644\n--- a/Documentation/git.txt\n+++ b/Documentation/git.txt\n@@ -887,7 +887,7 @@ for full details.\n \tprotocols has `protocol..allow` set to `always`\n \t(overriding any existing configuration). In other words, any\n \tprotocol not mentioned will be disallowed (i.e., this is a\n-\twhitelist, not a blacklist). See the description of\n+\tallowlist, not a denylist). See the description of\n \t`protocol.allow` in linkgit:git-config[1] for more details.\n \n `GIT_PROTOCOL_FROM_USER`::\n-- \ngitgitgadget\n\n"},{"id":"458975","messageId":"0d82c2a09c5e6a8e63b5294d738f51fc5b82af42.1657718450.git.gitgitgadget@gmail.com","threadId":"58152","inReplyTo":"pull.1274.git.1657718450.gitgitgadget@gmail.com","subject":"[PATCH 2/3] t/*: use allowlist","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-07-13T13:20:49Z","receivedAt":"2022-07-13T13:21:05Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nUsing \"allowlist\" is a more precise definition of the functionality\nprovided. The previous color-based word assume cultural interpretation\nto provide the meaning.\n\nFocus on changes in the test scripts, since most of the changes are in\ncomments and test names. The one exception is the renamed test_allowlist\nhelper.\n\nSigned-off-by: Derrick Stolee \n---\n t/README | 4 ++--\n t/lib-proto-disable.sh | 6 +++---\n t/t5812-proto-disable-http.sh | 2 +-\n t/t5815-submodule-protos.sh | 4 ++--\n t/t9400-git-cvsserver-server.sh | 2 +-\n t/test-lib-functions.sh | 2 +-\n t/test-lib.sh | 2 +-\n 7 files changed, 11 insertions(+), 11 deletions(-)\n\ndiff --git a/t/README b/t/README\nindex 309a31133c6..0c388a952f9 100644\n--- a/t/README\n+++ b/t/README\n@@ -367,8 +367,8 @@ GIT_TEST_SPLIT_INDEX= forces split-index mode on the whole\n test suite. Accept any boolean values that are accepted by git-config.\n \n GIT_TEST_PASSING_SANITIZE_LEAK= when compiled with\n-SANITIZE=leak will run only those tests that have whitelisted\n-themselves as passing with no memory leaks. Tests can be whitelisted\n+SANITIZE=leak will run only those tests that have allowlisted\n+themselves as passing with no memory leaks. Tests can be allowlisted\n by setting \"TEST_PASSES_SANITIZE_LEAK=true\" before sourcing\n \"test-lib.sh\" itself at the top of the test script. This test mode is\n used by the \"linux-leaks\" CI target.\ndiff --git a/t/lib-proto-disable.sh b/t/lib-proto-disable.sh\nindex 83babe57d95..6f2de57ef29 100644\n--- a/t/lib-proto-disable.sh\n+++ b/t/lib-proto-disable.sh\n@@ -1,7 +1,7 @@\n # Test routines for checking protocol disabling.\n \n-# Test clone/fetch/push with GIT_ALLOW_PROTOCOL whitelist\n-test_whitelist () {\n+# Test clone/fetch/push with GIT_ALLOW_PROTOCOL allowlist\n+test_allowlist () {\n \tdesc=$1\n \tproto=$2\n \turl=$3\n@@ -183,7 +183,7 @@ test_config () {\n # $2 - machine-readable name of the protocol\n # $3 - the URL to try cloning\n test_proto () {\n-\ttest_whitelist \"$@\"\n+\ttest_allowlist \"$@\"\n \n \ttest_config \"$@\"\n }\ndiff --git a/t/t5812-proto-disable-http.sh b/t/t5812-proto-disable-http.sh\nindex af8772fadaa..d6f9cd67588 100755\n--- a/t/t5812-proto-disable-http.sh\n+++ b/t/t5812-proto-disable-http.sh\n@@ -16,7 +16,7 @@ test_expect_success 'create git-accessible repo' '\n \n test_proto \"smart http\" http \"$HTTPD_URL/smart/repo.git\"\n \n-test_expect_success 'curl redirects respect whitelist' '\n+test_expect_success 'curl redirects respect allowlist' '\n \ttest_must_fail env GIT_ALLOW_PROTOCOL=http:https \\\n \t\t\t GIT_SMART_HTTP=0 \\\n \t\tgit clone \"$HTTPD_URL/ftp-redir/repo.git\" 2>stderr &&\ndiff --git a/t/t5815-submodule-protos.sh b/t/t5815-submodule-protos.sh\nindex 06f55a1b8a0..eecc401831f 100755\n--- a/t/t5815-submodule-protos.sh\n+++ b/t/t5815-submodule-protos.sh\n@@ -1,6 +1,6 @@\n #!/bin/sh\n \n-test_description='test protocol whitelisting with submodules'\n+test_description='test protocol allowlisting with submodules'\n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/lib-proto-disable.sh\n \n@@ -36,7 +36,7 @@ test_expect_success 'update of ext not allowed' '\n \ttest_must_fail git -C dst submodule update ext-module\n '\n \n-test_expect_success 'user can override whitelist' '\n+test_expect_success 'user can override allowlist' '\n \tGIT_ALLOW_PROTOCOL=ext git -C dst submodule update ext-module\n '\n \ndiff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh\nindex 210ddf09e30..03962d598f5 100755\n--- a/t/t9400-git-cvsserver-server.sh\n+++ b/t/t9400-git-cvsserver-server.sh\n@@ -221,7 +221,7 @@ test_expect_success 'req_Root (export-all)' \\\n 'cat request-anonymous | git-cvsserver --export-all pserver \"$WORKDIR\" >log 2>&1 &&\n sed -ne \\$p log | grep \"^I LOVE YOU\\$\"'\n \n-test_expect_success 'req_Root failure (export-all w/o whitelist)' \\\n+test_expect_success 'req_Root failure (export-all w/o allowlist)' \\\n '! (cat request-anonymous | git-cvsserver --export-all pserver >log 2>&1 || false)'\n \n test_expect_success 'req_Root (everything together)' \\\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex 6da7273f1d5..5351bbd83b9 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -651,7 +651,7 @@ test_set_prereq () {\n \t\t# test_unset_prereq()\n \t\t!*)\n \t\t\t;;\n-\t\t# (Temporary?) whitelist of things we can't easily\n+\t\t# (Temporary?) allowlist of things we can't easily\n \t\t# pretend not to support\n \t\tSYMLINKS)\n \t\t\t;;\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 55857af601b..6f5fccccb75 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -1408,7 +1408,7 @@ then\n \ttest_done\n fi\n \n-# skip non-whitelisted tests when compiled with SANITIZE=leak\n+# skip non-allowlisted tests when compiled with SANITIZE=leak\n if test -n \"$SANITIZE_LEAK\"\n then\n \tif test_bool_env GIT_TEST_PASSING_SANITIZE_LEAK false\n-- \ngitgitgadget\n\n"},{"id":"458976","messageId":"8aaceedb7a8f9d95550ef4a2d147c837ede6acec.1657718450.git.gitgitgadget@gmail.com","threadId":"58152","inReplyTo":"pull.1274.git.1657718450.gitgitgadget@gmail.com","subject":"[PATCH 3/3] *: use allowlist and denylist","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-07-13T13:20:50Z","receivedAt":"2022-07-13T13:21:09Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nUsing \"allowlist\" and \"denylist\" is a more precise definition of the\nfunctionality they provide. The previous color-based words assume\ncultural interpretation to provide the meaning.\n\nThese changes to the Git codebase are mostly cosmetic. Several comments\nare updated. The renamed protocol_allowlist() method is local to\ntransport.c so does not update any header file API definition. There are\nsome untranslated error messages that are reworded, so this _might_\naffect error parsers. However, two of the three error messages are\naround option parsing, so they are \"fast failures\". The one perhaps\nmeaningful change is the logerror() in daemon.c.\n\nAfter this change, the only remaining uses of the previous words are\nin release notes for older versions of Git.\n\nSigned-off-by: Derrick Stolee \n---\n daemon.c | 8 ++++----\n git-cvsserver.perl | 2 +-\n sha1dc/sha1.c | 12 ++++++------\n transport.c | 8 ++++----\n 4 files changed, 15 insertions(+), 15 deletions(-)\n\ndiff --git a/daemon.c b/daemon.c\nindex 58f1077885c..ed7c53b1110 100644\n--- a/daemon.c\n+++ b/daemon.c\n@@ -279,7 +279,7 @@ static const char *path_ok(const char *directory, struct hostinfo *hi)\n \t\t/* The validation is done on the paths after enter_repo\n \t\t * appends optional {.git,.git/.git} and friends, but\n \t\t * it does not use getcwd(). So if your /pub is\n-\t\t * a symlink to /mnt/pub, you can whitelist /pub and\n+\t\t * a symlink to /mnt/pub, you can allowlist /pub and\n \t\t * do not have to say /mnt/pub.\n \t\t * Do not say /pub/.\n \t\t */\n@@ -298,7 +298,7 @@ static const char *path_ok(const char *directory, struct hostinfo *hi)\n \t\t\treturn path;\n \t}\n \n-\tlogerror(\"'%s': not in whitelist\", path);\n+\tlogerror(\"'%s': not in allowlist\", path);\n \treturn NULL;\t\t/* Fallthrough. Deny by default */\n }\n \n@@ -403,7 +403,7 @@ static int run_service(const char *dir, struct daemon_service *service,\n \t * a \"git-daemon-export-ok\" flag that says that the other side\n \t * is ok with us doing this.\n \t *\n-\t * path_ok() uses enter_repo() and does whitelist checking.\n+\t * path_ok() uses enter_repo() and does allowlist checking.\n \t * We only need to make sure the repository is exported.\n \t */\n \n@@ -1444,7 +1444,7 @@ int cmd_main(int argc, const char **argv)\n \t\tcred = prepare_credentials(user_name, group_name);\n \n \tif (strict_paths && (!ok_paths || !*ok_paths))\n-\t\tdie(\"option --strict-paths requires a whitelist\");\n+\t\tdie(\"option --strict-paths requires a allowlist\");\n \n \tif (base_path && !is_directory(base_path))\n \t\tdie(\"base-path '%s' does not exist or is not a directory\",\ndiff --git a/git-cvsserver.perl b/git-cvsserver.perl\nindex 4c8118010a8..7d13b0a5ac1 100755\n--- a/git-cvsserver.perl\n+++ b/git-cvsserver.perl\n@@ -152,7 +152,7 @@ $state->{allowed_roots} = [ @ARGV ];\n \n # don't export the whole system unless the users requests it\n if ($state->{'export-all'} && !@{$state->{allowed_roots}}) {\n- die \"--export-all can only be used together with an explicit whitelist\\n\";\n+ die \"--export-all can only be used together with an explicit allowlist\\n\";\n }\n \n # Environment handling for running under git-shell\ndiff --git a/sha1dc/sha1.c b/sha1dc/sha1.c\nindex dede2cbddf9..b4a5f23c1ec 100644\n--- a/sha1dc/sha1.c\n+++ b/sha1dc/sha1.c\n@@ -86,30 +86,30 @@\n defined(__MIPSEB__) || defined(__MIPSEB) || defined(_MIPSEB) || \\\n defined(__sparc))\n /*\n- * Should define Big Endian for a whitelist of known processors. See\n+ * Should define Big Endian for a allowlist of known processors. See\n * https://sourceforge.net/p/predef/wiki/Endianness/ and\n * http://www.oracle.com/technetwork/server-storage/solaris/portingtosolaris-138514.html\n */\n #define SHA1DC_BIGENDIAN\n \n-/* Not under GCC-alike or glibc or *BSD or newlib or */\n+/* Not under GCC-alike or glibc or *BSD or newlib or */\n #elif (defined(_AIX) || defined(__hpux))\n \n /*\n- * Defines Big Endian on a whitelist of OSs that are known to be Big\n+ * Defines Big Endian on a allowlist of OSs that are known to be Big\n * Endian-only. See\n * https://lore.kernel.org/git/93056823-2740-d072-1ebd-46b440b33d7e@felt.demon.nl/\n */\n #define SHA1DC_BIGENDIAN\n \n-/* Not under GCC-alike or glibc or *BSD or newlib or or */\n+/* Not under GCC-alike or glibc or *BSD or newlib or or */\n #elif defined(SHA1DC_ON_INTEL_LIKE_PROCESSOR)\n /*\n * As a last resort before we do anything else we're not 100% sure\n- * about below, we blacklist specific processors here. We could add\n+ * about below, we denylist specific processors here. We could add\n * more, see e.g. https://wiki.debian.org/ArchitectureSpecificsMemo\n */\n-#else /* Not under GCC-alike or glibc or *BSD or newlib or or or */\n+#else /* Not under GCC-alike or glibc or *BSD or newlib or or or */\n \n /* We do nothing more here for now */\n /*#error \"Uncomment this to see if you fall through all the detection\"*/\ndiff --git a/transport.c b/transport.c\nindex 52db7a3cb09..321bbe382cc 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -940,7 +940,7 @@ static int external_specification_len(const char *url)\n \treturn strchr(url, ':') - url;\n }\n \n-static const struct string_list *protocol_whitelist(void)\n+static const struct string_list *protocol_allowlist(void)\n {\n \tstatic int enabled = -1;\n \tstatic struct string_list allowed = STRING_LIST_INIT_DUP;\n@@ -1020,9 +1020,9 @@ static enum protocol_allow_config get_protocol_config(const char *type)\n \n int is_transport_allowed(const char *type, int from_user)\n {\n-\tconst struct string_list *whitelist = protocol_whitelist();\n-\tif (whitelist)\n-\t\treturn string_list_has_string(whitelist, type);\n+\tconst struct string_list *allowlist = protocol_allowlist();\n+\tif (allowlist)\n+\t\treturn string_list_has_string(allowlist, type);\n \n \tswitch (get_protocol_config(type)) {\n \tcase PROTOCOL_ALLOW_ALWAYS:\n-- \ngitgitgadget\n"},{"id":"458979","messageId":"r5351157-0p62-7rp4-p254-04rn0r63o515@tzk.qr","threadId":"58152","inReplyTo":"8aaceedb7a8f9d95550ef4a2d147c837ede6acec.1657718450.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 3/3] *: use allowlist and denylist","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2022-07-13T13:27:02Z","receivedAt":"2022-07-13T13:27:19Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Stolee,\n\nOn Wed, 13 Jul 2022, Derrick Stolee via GitGitGadget wrote:\n\n> From: Derrick Stolee \n>\n> Using \"allowlist\" and \"denylist\" is a more precise definition of the\n> functionality they provide. The previous color-based words assume\n> cultural interpretation to provide the meaning.\n>\n> These changes to the Git codebase are mostly cosmetic. Several comments\n> are updated. The renamed protocol_allowlist() method is local to\n> transport.c so does not update any header file API definition. There are\n> some untranslated error messages that are reworded, so this _might_\n> affect error parsers. However, two of the three error messages are\n> around option parsing, so they are \"fast failures\". The one perhaps\n> meaningful change is the logerror() in daemon.c.\n\nI do consider `git daemon` less important these days because we have two\ntransports that are secure (which `git://` is not): HTTPS and SSH. That\nsuggests that the `daemon.c` change might have a very low impact to begin\nwith.\n\nThe other changes affect `cvsserver` which I consider even less important.\nIn fact, I would be in favor of deprecating it soon and of deleting it in\ndue time.\n\nAll the changes in this patch look good to me: ACK!\n\nThank you!\nDscho\n\n>\n> After this change, the only remaining uses of the previous words are\n> in release notes for older versions of Git.\n>\n> Signed-off-by: Derrick Stolee \n> ---\n> daemon.c | 8 ++++----\n> git-cvsserver.perl | 2 +-\n> sha1dc/sha1.c | 12 ++++++------\n> transport.c | 8 ++++----\n> 4 files changed, 15 insertions(+), 15 deletions(-)\n>\n> diff --git a/daemon.c b/daemon.c\n> index 58f1077885c..ed7c53b1110 100644\n> --- a/daemon.c\n> +++ b/daemon.c\n> @@ -279,7 +279,7 @@ static const char *path_ok(const char *directory, struct hostinfo *hi)\n> \t\t/* The validation is done on the paths after enter_repo\n> \t\t * appends optional {.git,.git/.git} and friends, but\n> \t\t * it does not use getcwd(). So if your /pub is\n> -\t\t * a symlink to /mnt/pub, you can whitelist /pub and\n> +\t\t * a symlink to /mnt/pub, you can allowlist /pub and\n> \t\t * do not have to say /mnt/pub.\n> \t\t * Do not say /pub/.\n> \t\t */\n> @@ -298,7 +298,7 @@ static const char *path_ok(const char *directory, struct hostinfo *hi)\n> \t\t\treturn path;\n> \t}\n>\n> -\tlogerror(\"'%s': not in whitelist\", path);\n> +\tlogerror(\"'%s': not in allowlist\", path);\n> \treturn NULL;\t\t/* Fallthrough. Deny by default */\n> }\n>\n> @@ -403,7 +403,7 @@ static int run_service(const char *dir, struct daemon_service *service,\n> \t * a \"git-daemon-export-ok\" flag that says that the other side\n> \t * is ok with us doing this.\n> \t *\n> -\t * path_ok() uses enter_repo() and does whitelist checking.\n> +\t * path_ok() uses enter_repo() and does allowlist checking.\n> \t * We only need to make sure the repository is exported.\n> \t */\n>\n> @@ -1444,7 +1444,7 @@ int cmd_main(int argc, const char **argv)\n> \t\tcred = prepare_credentials(user_name, group_name);\n>\n> \tif (strict_paths && (!ok_paths || !*ok_paths))\n> -\t\tdie(\"option --strict-paths requires a whitelist\");\n> +\t\tdie(\"option --strict-paths requires a allowlist\");\n>\n> \tif (base_path && !is_directory(base_path))\n> \t\tdie(\"base-path '%s' does not exist or is not a directory\",\n> diff --git a/git-cvsserver.perl b/git-cvsserver.perl\n> index 4c8118010a8..7d13b0a5ac1 100755\n> --- a/git-cvsserver.perl\n> +++ b/git-cvsserver.perl\n> @@ -152,7 +152,7 @@ $state->{allowed_roots} = [ @ARGV ];\n>\n> # don't export the whole system unless the users requests it\n> if ($state->{'export-all'} && !@{$state->{allowed_roots}}) {\n> - die \"--export-all can only be used together with an explicit whitelist\\n\";\n> + die \"--export-all can only be used together with an explicit allowlist\\n\";\n> }\n>\n> # Environment handling for running under git-shell\n> diff --git a/sha1dc/sha1.c b/sha1dc/sha1.c\n> index dede2cbddf9..b4a5f23c1ec 100644\n> --- a/sha1dc/sha1.c\n> +++ b/sha1dc/sha1.c\n> @@ -86,30 +86,30 @@\n> defined(__MIPSEB__) || defined(__MIPSEB) || defined(_MIPSEB) || \\\n> defined(__sparc))\n> /*\n> - * Should define Big Endian for a whitelist of known processors. See\n> + * Should define Big Endian for a allowlist of known processors. See\n> * https://sourceforge.net/p/predef/wiki/Endianness/ and\n> * http://www.oracle.com/technetwork/server-storage/solaris/portingtosolaris-138514.html\n> */\n> #define SHA1DC_BIGENDIAN\n>\n> -/* Not under GCC-alike or glibc or *BSD or newlib or */\n> +/* Not under GCC-alike or glibc or *BSD or newlib or */\n> #elif (defined(_AIX) || defined(__hpux))\n>\n> /*\n> - * Defines Big Endian on a whitelist of OSs that are known to be Big\n> + * Defines Big Endian on a allowlist of OSs that are known to be Big\n> * Endian-only. See\n> * https://lore.kernel.org/git/93056823-2740-d072-1ebd-46b440b33d7e@felt.demon.nl/\n> */\n> #define SHA1DC_BIGENDIAN\n>\n> -/* Not under GCC-alike or glibc or *BSD or newlib or or */\n> +/* Not under GCC-alike or glibc or *BSD or newlib or or */\n> #elif defined(SHA1DC_ON_INTEL_LIKE_PROCESSOR)\n> /*\n> * As a last resort before we do anything else we're not 100% sure\n> - * about below, we blacklist specific processors here. We could add\n> + * about below, we denylist specific processors here. We could add\n> * more, see e.g. https://wiki.debian.org/ArchitectureSpecificsMemo\n> */\n> -#else /* Not under GCC-alike or glibc or *BSD or newlib or or or */\n> +#else /* Not under GCC-alike or glibc or *BSD or newlib or or or */\n>\n> /* We do nothing more here for now */\n> /*#error \"Uncomment this to see if you fall through all the detection\"*/\n> diff --git a/transport.c b/transport.c\n> index 52db7a3cb09..321bbe382cc 100644\n> --- a/transport.c\n> +++ b/transport.c\n> @@ -940,7 +940,7 @@ static int external_specification_len(const char *url)\n> \treturn strchr(url, ':') - url;\n> }\n>\n> -static const struct string_list *protocol_whitelist(void)\n> +static const struct string_list *protocol_allowlist(void)\n> {\n> \tstatic int enabled = -1;\n> \tstatic struct string_list allowed = STRING_LIST_INIT_DUP;\n> @@ -1020,9 +1020,9 @@ static enum protocol_allow_config get_protocol_config(const char *type)\n>\n> int is_transport_allowed(const char *type, int from_user)\n> {\n> -\tconst struct string_list *whitelist = protocol_whitelist();\n> -\tif (whitelist)\n> -\t\treturn string_list_has_string(whitelist, type);\n> +\tconst struct string_list *allowlist = protocol_allowlist();\n> +\tif (allowlist)\n> +\t\treturn string_list_has_string(allowlist, type);\n>\n> \tswitch (get_protocol_config(type)) {\n> \tcase PROTOCOL_ALLOW_ALWAYS:\n> --\n> gitgitgadget\n>\n"},{"id":"458981","messageId":"8n9169qs-9554-rqs8-0902-58n309n89471@tzk.qr","threadId":"58152","inReplyTo":"pull.1274.git.1657718450.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/3] Use \"allowlist\" and \"denylist\" tree-wide","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2022-07-13T13:29:40Z","receivedAt":"2022-07-13T13:29:55Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Stolee,\n\nOn Wed, 13 Jul 2022, Derrick Stolee via GitGitGadget wrote:\n\n> The terms \"allowlist\" and \"denylist\" are self-defining. One \"allows\" things\n> while the other \"denies\" things.\n>\n> These are better terms over \"whitelist\" and \"blacklist\" which require prior\n> knowledge of the terms or cultural expectations around what each color\n> \"means\".\n\nI agree that \"allowlist\" and \"denylist\" are much better terms. As you say,\nthey are more obvious even for non-native speakers, but we do want to take\nthe cultural implications into account and be mindful to avoid needlessly\ncontroversial language.\n\nTherefore, I am very much in favor of this patch series: ACK!\n\nThank you for putting in the work to make this happen,\nDscho\n"},{"id":"458985","messageId":"Ys7i/GWSNRHqSZNQ@coredump.intra.peff.net","threadId":"58152","inReplyTo":"ec81aac05c40318755f5311a20e8f9cc55d289fc.1657718450.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/3] Documentation: use allowlist and denylist","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-07-13T15:21:32Z","receivedAt":"2022-07-13T15:21:45Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jul 13, 2022 at 01:20:48PM +0000, Derrick Stolee via GitGitGadget wrote:\n\n> Using \"allowlist\" and \"denylist\" is a more precise definition of the\n> functionality they provide. The previous color-based words assume\n> cultural interpretation to provide the meaning.\n> \n> Focus on replacements in the Documentation/ directory since these are\n> not functional uses.\n\nThanks, the direction looks reasonable to me. I knew at least about the\none for protocol.*, which I think I introduced, and had been meaning to\ngrep for others.\n\nI think you need some grammatical fixups, though. E.g.:\n\n> diff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt\n> index fdc28c041c7..ff74a90aead 100644\n> --- a/Documentation/git-daemon.txt\n> +++ b/Documentation/git-daemon.txt\n> @@ -33,7 +33,7 @@ It verifies that the directory has the magic file \"git-daemon-export-ok\", and\n> it will refuse to export any Git directory that hasn't explicitly been marked\n> for export this way (unless the `--export-all` parameter is specified). If you\n> pass some directory paths as 'git daemon' arguments, you can further restrict\n> -the offers to a whitelist comprising of those.\n> +the offers to a allowlist comprising of those.\n\nYou'd want s/a/an/ here.\n\n> diff --git a/Documentation/git.txt b/Documentation/git.txt\n> index 302607a4967..384718ee677 100644\n> --- a/Documentation/git.txt\n> +++ b/Documentation/git.txt\n> @@ -887,7 +887,7 @@ for full details.\n> \tprotocols has `protocol..allow` set to `always`\n> \t(overriding any existing configuration). In other words, any\n> \tprotocol not mentioned will be disallowed (i.e., this is a\n> -\twhitelist, not a blacklist). See the description of\n> +\tallowlist, not a denylist). See the description of\n> \t`protocol.allow` in linkgit:git-config[1] for more details.\n\nDitto here.\n\n> 'git daemon' as inetd server::\n> \tTo set up 'git daemon' as an inetd service that handles any\n> -\trepository under the whitelisted set of directories, /pub/foo\n> +\trepository under the allowlisted set of directories, /pub/foo\n> \tand /pub/bar, place an entry like the following into\n> \t/etc/inetd all on one line:\n\nThis one is more gut feeling. Somehow \"allowlisted\" as an adjective\nseems more awkward than \"whitelisted\". Probably because I've just seen\n\"whitelisted\" so many more times. Or maybe it just crosses my personal\nline of too many syllables. ;)\n\nI don't know if there's an easy way around it. I don't have a suggestion\nthat's better than \"allowlist\" for the general term, and we want to use\nthe terms consistently. You could probably write it as:\n\n ...any repository under the set of directories in the allowlist...\n\nbut I'm sure somebody probably likes that less. :) So I register it only\nas a suggestion, not a request for a change.\n\n-Peff\n"},{"id":"458986","messageId":"Ys7jWgjhkavPxFDn@coredump.intra.peff.net","threadId":"58152","inReplyTo":"8aaceedb7a8f9d95550ef4a2d147c837ede6acec.1657718450.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 3/3] *: use allowlist and denylist","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-07-13T15:23:06Z","receivedAt":"2022-07-13T15:23:11Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jul 13, 2022 at 01:20:50PM +0000, Derrick Stolee via GitGitGadget wrote:\n\n> @@ -1444,7 +1444,7 @@ int cmd_main(int argc, const char **argv)\n> \t\tcred = prepare_credentials(user_name, group_name);\n> \n> \tif (strict_paths && (!ok_paths || !*ok_paths))\n> -\t\tdie(\"option --strict-paths requires a whitelist\");\n> +\t\tdie(\"option --strict-paths requires a allowlist\");\n\nHere's another s/a/an/ case. I think there are a few others in comments\nin other parts of the patch.\n\n-Peff\n"},{"id":"458989","messageId":"xmqqk08hdm05.fsf@gitster.g","threadId":"58152","inReplyTo":"pull.1274.git.1657718450.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/3] Use \"allowlist\" and \"denylist\" tree-wide","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-07-13T16:18:34Z","receivedAt":"2022-07-13T16:18:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Derrick Stolee via GitGitGadget\" writes:\n\n> The terms \"allowlist\" and \"denylist\" are self-defining. One \"allows\" things\n> while the other \"denies\" things.\n>\n> These are better terms over \"whitelist\" and \"blacklist\" which require prior\n> knowledge of the terms or cultural expectations around what each color\n> \"means\".\n\nHalf Devil's advocate mode on, as I got up on the wrong side of the\nbed this morning.\n\nI am very much for consistent uses of allow/deny and I think it is a\ngood idea to review and apply this series.\n\nBut I'd prefer to see us more honest to ourselves. Like it or not,\nthe code comment and documentation are targetted toward those who\ncan read English, and when you say something is whitelisted in\nEnglish, you know exactly what it means, due to shared knowledge of\nhistorical use of the word.\n\nWe are doing this change in the name of inclusion. I find it\nintellectually dishonest to avoid saying that true reason, and\ninstead say the allow/deny pair is more \"precise\". They are not\nmore precise. In fact, the fact why you have to choose between deny\nand block and defend deny over block shows that these words are less\nprecise. People who use white/black do not have to choose between\nblack and other colors and say \"white/red may be OK but we choose\nblack because...\" to defend the choice of their words.\n\nThe reason we do this change is because the project thinks that it\nis the right thing to encourage the adoption of these more inclusive\nwords, together with other projects that did the same.\n\nIn addition, they are words more widely accepted in today's world,\nand new folks are more likely to be educated with these words. As\ntime goes by, the historical white/black will be less understood, so\nit makes it a future-proofing change, as well.\n\n"},{"id":"459000","messageId":"647a4cf7-ef1f-284b-37e2-4dceb3cfae1d@github.com","threadId":"58152","inReplyTo":"xmqqk08hdm05.fsf@gitster.g","subject":"Re: [PATCH 0/3] Use \"allowlist\" and \"denylist\" tree-wide","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-07-13T18:33:05Z","receivedAt":"2022-07-13T18:33:25Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 7/13/2022 12:18 PM, Junio C Hamano wrote:\n> \"Derrick Stolee via GitGitGadget\" writes:\n> \n>> The terms \"allowlist\" and \"denylist\" are self-defining. One \"allows\" things\n>> while the other \"denies\" things.\n>>\n>> These are better terms over \"whitelist\" and \"blacklist\" which require prior\n>> knowledge of the terms or cultural expectations around what each color\n>> \"means\".\n> \n> Half Devil's advocate mode on, as I got up on the wrong side of the\n> bed this morning.\n> \n> I am very much for consistent uses of allow/deny and I think it is a\n> good idea to review and apply this series.\n> \n> But I'd prefer to see us more honest to ourselves. Like it or not,\n> the code comment and documentation are targetted toward those who\n> can read English, and when you say something is whitelisted in\n> English, you know exactly what it means, due to shared knowledge of\n> historical use of the word.\n> \n> We are doing this change in the name of inclusion. I find it\n> intellectually dishonest to avoid saying that true reason, and\n> instead say the allow/deny pair is more \"precise\". They are not\n> more precise. In fact, the fact why you have to choose between deny\n> and block and defend deny over block shows that these words are less\n> precise. People who use white/black do not have to choose between\n> black and other colors and say \"white/red may be OK but we choose\n> black because...\" to defend the choice of their words.\n> \n> The reason we do this change is because the project thinks that it\n> is the right thing to encourage the adoption of these more inclusive\n> words, together with other projects that did the same.\n> \n> In addition, they are words more widely accepted in today's world,\n> and new folks are more likely to be educated with these words. As\n> time goes by, the historical white/black will be less understood, so\n> it makes it a future-proofing change, as well.\n \nI like how you start by saying you are playing devil's advocate,\nbut then go on to add more reasons to support the work. It's good\nfeedback to make the case stronger.\n\nThanks,\n-Stolee\n"},{"id":"459001","messageId":"6b117636-d3e3-5ae9-5419-439689efb763@github.com","threadId":"58152","inReplyTo":"Ys7i/GWSNRHqSZNQ@coredump.intra.peff.net","subject":"Re: [PATCH 1/3] Documentation: use allowlist and denylist","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-07-13T18:34:33Z","receivedAt":"2022-07-13T18:34:49Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 7/13/2022 11:21 AM, Jeff King wrote:\n> On Wed, Jul 13, 2022 at 01:20:48PM +0000, Derrick Stolee via GitGitGadget wrote:\n> \n>> Using \"allowlist\" and \"denylist\" is a more precise definition of the\n>> functionality they provide. The previous color-based words assume\n>> cultural interpretation to provide the meaning.\n>>\n>> Focus on replacements in the Documentation/ directory since these are\n>> not functional uses.\n> \n> Thanks, the direction looks reasonable to me. I knew at least about the\n> one for protocol.*, which I think I introduced, and had been meaning to\n> grep for others.\n> \n> I think you need some grammatical fixups, though. E.g.:\n> \n>> diff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt\n>> index fdc28c041c7..ff74a90aead 100644\n>> --- a/Documentation/git-daemon.txt\n>> +++ b/Documentation/git-daemon.txt\n>> @@ -33,7 +33,7 @@ It verifies that the directory has the magic file \"git-daemon-export-ok\", and\n>> it will refuse to export any Git directory that hasn't explicitly been marked\n>> for export this way (unless the `--export-all` parameter is specified). If you\n>> pass some directory paths as 'git daemon' arguments, you can further restrict\n>> -the offers to a whitelist comprising of those.\n>> +the offers to a allowlist comprising of those.\n> \n> You'd want s/a/an/ here.\n\nThank you for the careful attention to detail. I should have\nbeen more careful when switching from a consonant to a vowel.\n>> 'git daemon' as inetd server::\n>> \tTo set up 'git daemon' as an inetd service that handles any\n>> -\trepository under the whitelisted set of directories, /pub/foo\n>> +\trepository under the allowlisted set of directories, /pub/foo\n>> \tand /pub/bar, place an entry like the following into\n>> \t/etc/inetd all on one line:\n> \n> This one is more gut feeling. Somehow \"allowlisted\" as an adjective\n> seems more awkward than \"whitelisted\". Probably because I've just seen\n> \"whitelisted\" so many more times. Or maybe it just crosses my personal\n> line of too many syllables. ;)\n> \n> I don't know if there's an easy way around it. I don't have a suggestion\n> that's better than \"allowlist\" for the general term, and we want to use\n> the terms consistently. You could probably write it as:\n> \n> ...any repository under the set of directories in the allowlist...\n> \n> but I'm sure somebody probably likes that less. :) So I register it only\n> as a suggestion, not a request for a change.\n\nI think that is the natural way to reword here, but I'll take some\ntime to think of alternatives before sending v2.\n\nThanks,\n-Stolee\n"},{"id":"459007","messageId":"220713.86wncg3hnj.gmgdl@evledraar.gmail.com","threadId":"58152","inReplyTo":"pull.1274.git.1657718450.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/3] Use \"allowlist\" and \"denylist\" tree-wide","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-07-13T19:42:44Z","receivedAt":"2022-07-13T20:02:46Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Jul 13 2022, Derrick Stolee via GitGitGadget wrote:\n\n> The terms \"allowlist\" and \"denylist\" are self-defining. One \"allows\" things\n> while the other \"denies\" things.\n\nI've got a preference for things that can be found in widely available\ndictionaries, these words seem to be tech neologisms.\n\nThe resulting wording also seems a bit ackward to me, e.g. we now say\nthat some tests are \"allowlisted [...] as passing with no memory\nleaks\". Are we denying or allowing them to pass? No, they're going to\neither pass or not.\n\nSo to me \"whitelist\" or \"blacklist\" is more natural when used in the\ndescriptive sense, whereas \"allow\" and \"deny\" are verbs, so that seems\nto impart a sense of actively allowing or denying something.\n\n> These are better terms over \"whitelist\" and \"blacklist\" which require prior\n> knowledge of the terms or cultural expectations around what each color\n> \"means\".\n\nApparently whitelist is defined in terms of blacklist, which per\nWikipedia originates in some 17th century play:\nhttps://en.wikipedia.org/wiki/Blacklisting#Origins_of_the_term\n\n> [...]\n> Some recommend using \"blocklist\", but I personally prefer \"denylist\". To me,\n> \"blocking\" something seems permanent. \"Denying\" something seems ephemeral\n> and related to a specific request being denied due to some (possibly\n> mutable) state. I'm open to suggestions here. There are many fewer\n> replacements needed in this case.\n\nI suspect the actual motivation is closer to that summarized in :\nhttps://en.wikipedia.org/wiki/Whitelist#Controversy\n\nPersonally I'd really prefer if we didn't take these sort of changes,\nand took the view that if something was readily understood that it was\ngood enough.\n\nThe CodingGuidelines note that we use a mix of US & UK english, so\nforbidding certain words & basically requiring some of us to keep\nabreast of the latest political trends in America is a bit too much. I'd\njust like to write code, please...\n\n> I did not make any change to our CodingGuidelines. Hopefully having clear\n> usage throughout the codebase will be enough to promote using consistent\n> terminology.\n\n...particularly since I think what's being implied here is that we can\nexpect interested parties to be setting up the relevant E-Mail filters,\nand asking patch submitters to change wording in the same way as this\nseries does.\n\nWe also have 30-40 uses of both terms in-tree, so it seems implausible\nthat people are mainly copying existing wording.\n\nA few of the hunks here are changing docs I added, and I just added\nthose \"naturally\", i.e. I happened to think of those words to describe\nwhat I was trying to get across).\n"},{"id":"459008","messageId":"220713.86sfn43hkh.gmgdl@evledraar.gmail.com","threadId":"58152","inReplyTo":"pull.1274.git.1657718450.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/3] Use \"allowlist\" and \"denylist\" tree-wide","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-07-13T20:02:45Z","receivedAt":"2022-07-13T20:04:35Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Jul 13 2022, Derrick Stolee via GitGitGadget wrote:\n\n> sha1dc/sha1.c | 12 ++++++------\n\nAside from anything else I've commented on: Please drop this part of the\nchange. If you'd like to change this take it up with upstream:\nhttps://github.com/cr-marcstevens/sha1collisiondetection/\n\nAs a \"git log\" on \"sha1dc sha1collisiondetection\" will show we try to\nkeep a 1=1 mapping to upstream with this code, this would both diverge\nus from upstream, and diverge sha1dc from our own submodule copy in\nsha1collisiondetection/\".\n"},{"id":"459009","messageId":"xmqqk08gbw91.fsf@gitster.g","threadId":"58152","inReplyTo":"ec81aac05c40318755f5311a20e8f9cc55d289fc.1657718450.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/3] Documentation: use allowlist and denylist","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-07-13T20:20:10Z","receivedAt":"2022-07-13T20:20:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Derrick Stolee via GitGitGadget\" writes:\n\n> @@ -313,7 +313,7 @@ circumstances, allowing easier restricted usage through git-shell.\n> \n> GIT_CVSSERVER_BASE_PATH takes the place of the argument to --base-path.\n> \n> -GIT_CVSSERVER_ROOT specifies a single-directory whitelist. The\n> +GIT_CVSSERVER_ROOT specifies a single-directory allowlist. The\n\nNowhere before this documentation there is mention of white/allow.\nThey consistently say \"list of allowed directories\".\n\nIt may probably make the resulting text much easier to read if we\navoid the non-word \"allowlist\", which was recently invented only to\navoid using the word \"whitelist\".\n\n GIT_CVSSERVER_ROOT specifies a single allowed directory.\n\nIOW, the original did not even have to say \"whitelist\".\n\n> diff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt\n> index fdc28c041c7..ff74a90aead 100644\n> --- a/Documentation/git-daemon.txt\n> +++ b/Documentation/git-daemon.txt\n> @@ -33,7 +33,7 @@ It verifies that the directory has the magic file \"git-daemon-export-ok\", and\n> it will refuse to export any Git directory that hasn't explicitly been marked\n> for export this way (unless the `--export-all` parameter is specified). If you\n> pass some directory paths as 'git daemon' arguments, you can further restrict\n> -the offers to a whitelist comprising of those.\n> +the offers to a allowlist comprising of those.\n\n\"a -> an\"; but I think the same suggestion as cvs-server equally\napplies here.\n\n -for export this way (unless the `--export-all` parameter is specified). If you\n -pass some directory paths as 'git daemon' arguments, you can further restrict\n -the offers to a whitelist comprising of those.\n +for export this way (unless the `--export-all` parameter is specified).\n +You can furtherrestrict the offers by passing the list of allowed directories\n +as 'git daemon' arguments.\n\nwould be much easier to understand, with or without s/white/allow/.\n\n> diff --git a/Documentation/git.txt b/Documentation/git.txt\n> index 302607a4967..384718ee677 100644\n> --- a/Documentation/git.txt\n> +++ b/Documentation/git.txt\n> @@ -887,7 +887,7 @@ for full details.\n> \tprotocols has `protocol..allow` set to `always`\n> \t(overriding any existing configuration). In other words, any\n> \tprotocol not mentioned will be disallowed (i.e., this is a\n> -\twhitelist, not a blacklist). See the description of\n> +\tallowlist, not a denylist). See the description of\n\nAs I always tell contributors in my review, whenever we see \"In\nother words\" and parenthesized follow-up explanation, we should take\nit as an admission by the author that whatever the author wrote\nbefore it is badly written and should have been expressed in a more\nclear way.\n\n In other words, only the protocols listed on this variable are\n allowed and all others are disallowed. See the description of\n ...\n\nwithout \"(i.e....)\" would be shorter and easier to read, I would\nthink.\n\n"},{"id":"459010","messageId":"xmqqfsj4bvoh.fsf@gitster.g","threadId":"58152","inReplyTo":"647a4cf7-ef1f-284b-37e2-4dceb3cfae1d@github.com","subject":"Re: [PATCH 0/3] Use \"allowlist\" and \"denylist\" tree-wide","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-07-13T20:32:30Z","receivedAt":"2022-07-13T20:32:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Derrick Stolee writes:\n\n> I like how you start by saying you are playing devil's advocate,\n> but then go on to add more reasons to support the work. It's good\n> feedback to make the case stronger.\n\nI may have offered alternatives, but I am not \"adding more\" reasons.\n"},{"id":"459022","messageId":"xmqqedyoabqp.fsf@gitster.g","threadId":"58152","inReplyTo":"220713.86wncg3hnj.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 0/3] Use \"allowlist\" and \"denylist\" tree-wide","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-07-13T22:28:30Z","receivedAt":"2022-07-13T22:28:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> On Wed, Jul 13 2022, Derrick Stolee via GitGitGadget wrote:\n>\n>> The terms \"allowlist\" and \"denylist\" are self-defining. One \"allows\" things\n>> while the other \"denies\" things.\n>\n> I've got a preference for things that can be found in widely available\n> dictionaries, these words seem to be tech neologisms.\n\nFWIW, I share the same. I suspect that \"whitelist\" may be found in\nmore dictionaries than \"allowlist\".\n\ne.g.\n\n https://www.merriam-webster.com/dictionary/allowlist\n https://www.merriam-webster.com/dictionary/whitelist\n\nA statement \"We have audience who are not native English speakers,\nand may not share cultural background\" may not be incorrect at all,\nbut that does not justify s/whitelist/allowlist/. We end up with\nsentences written with non-words that these non-natives cannot even\nlook up in dictionary.\n\nIf we can rephrase without using these invented words, we should do\nso, especially when the result becomes even easier to read than the\noriginal that used \"whitelist\". I've shown a few examples in my\nother messages in this thread.\n\nThanks.\n\n"},{"id":"459109","messageId":"52d432b9-acda-16dc-2536-9b6c66279836@github.com","threadId":"58152","inReplyTo":"xmqqedyoabqp.fsf@gitster.g","subject":"Re: [PATCH 0/3] Use \"allowlist\" and \"denylist\" tree-wide","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-07-15T02:25:46Z","receivedAt":"2022-07-15T02:25:52Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 7/13/2022 6:28 PM, Junio C Hamano wrote:\n> If we can rephrase without using these invented words, we should do\n> so, especially when the result becomes even easier to read than the\n> original that used \"whitelist\". I've shown a few examples in my\n> other messages in this thread.\n\nBased on those examples, I agree that the best thing to do is to\nrephrase to avoid the term altogether. This avoids confusion when\nthe reader does not know the term, as well as sometimes being more\nconsistent with the phrasing in the same document.\n\nThanks,\n-Stolee\n"},{"id":"459111","messageId":"19632a2d2459114484dcdaddd4db0d22dfd64fa7.1657852722.git.gitgitgadget@gmail.com","threadId":"58152","inReplyTo":"pull.1274.v2.git.1657852722.gitgitgadget@gmail.com","subject":"[PATCH v2 1/3] Documentation: remove use of whitelist","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-07-15T02:38:40Z","receivedAt":"2022-07-15T02:38:51Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThe word \"whitelist\" has cultural implications that are not inclusive.\nThankfully, it is not difficult to reword and avoid its use.\n\nRemove uses of \"whitelist\" in the Documentation/ directory. In all\ncases, we can rewrite the sentences to add clarity instead of relying on\nthe reader understanding this term.\n\nThe most substantial change is to git-daemon.txt which had several uses,\nbut we can refer to the input directories as a list of included\ndirectories, making the descriptions slightly simpler.\n\nHelped-by: Jeff King \nHelped-by: Junio C Hamano \nSigned-off-by: Derrick Stolee \n---\n Documentation/git-cvsserver.txt | 2 +-\n Documentation/git-daemon.txt | 15 +++++++--------\n Documentation/git.txt | 3 +--\n 3 files changed, 9 insertions(+), 11 deletions(-)\n\ndiff --git a/Documentation/git-cvsserver.txt b/Documentation/git-cvsserver.txt\nindex 4dc57ed2547..e90b03402a5 100644\n--- a/Documentation/git-cvsserver.txt\n+++ b/Documentation/git-cvsserver.txt\n@@ -313,7 +313,7 @@ circumstances, allowing easier restricted usage through git-shell.\n \n GIT_CVSSERVER_BASE_PATH takes the place of the argument to --base-path.\n \n-GIT_CVSSERVER_ROOT specifies a single-directory whitelist. The\n+GIT_CVSSERVER_ROOT specifies a single allowed directory. The\n repository must still be configured to allow access through\n git-cvsserver, as described above.\n \ndiff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt\nindex fdc28c041c7..7a0539cb411 100644\n--- a/Documentation/git-daemon.txt\n+++ b/Documentation/git-daemon.txt\n@@ -32,8 +32,8 @@ that service if it is enabled.\n It verifies that the directory has the magic file \"git-daemon-export-ok\", and\n it will refuse to export any Git directory that hasn't explicitly been marked\n for export this way (unless the `--export-all` parameter is specified). If you\n-pass some directory paths as 'git daemon' arguments, you can further restrict\n-the offers to a whitelist comprising of those.\n+pass some directory paths as 'git daemon' arguments, the offers are limited to\n+repositories within those directories.\n \n By default, only `upload-pack` service is enabled, which serves\n 'git fetch-pack' and 'git ls-remote' clients, which are invoked\n@@ -50,7 +50,7 @@ OPTIONS\n \tMatch paths exactly (i.e. don't allow \"/foo/repo\" when the real path is\n \t\"/foo/repo.git\" or \"/foo/repo/.git\") and don't do user-relative paths.\n \t'git daemon' will refuse to start when this option is enabled and no\n-\twhitelist is specified.\n+\tspecific directories are specified.\n \n --base-path=::\n \tRemap all the path requests as relative to the given path.\n@@ -73,7 +73,7 @@ OPTIONS\n \t%IP for the server's IP address, %P for the port number,\n \tand %D for the absolute path of the named repository.\n \tAfter interpolation, the path is validated against the directory\n-\twhitelist.\n+\tlist.\n \n --export-all::\n \tAllow pulling from all directories that look like Git repositories\n@@ -218,7 +218,7 @@ standard output to be sent to the requestor as an error message when\n it declines the service.\n \n ::\n-\tA directory to add to the whitelist of allowed directories. Unless\n+\tA directory to add to the list of allowed directories. Unless\n \t--strict-paths is specified this will also include subdirectories\n \tof each named directory.\n \n@@ -264,9 +264,8 @@ git\t\t9418/tcp\t\t# Git Version Control System\n \n 'git daemon' as inetd server::\n \tTo set up 'git daemon' as an inetd service that handles any\n-\trepository under the whitelisted set of directories, /pub/foo\n-\tand /pub/bar, place an entry like the following into\n-\t/etc/inetd all on one line:\n+\trepository in the directory list of `/pub/foo` and `/pub/bar`,\n+\tplace an entry like the following into `/etc/inetd` all on one line:\n +\n ------------------------------------------------\n \tgit stream tcp nowait nobody /usr/bin/git\ndiff --git a/Documentation/git.txt b/Documentation/git.txt\nindex 302607a4967..dd5061563eb 100644\n--- a/Documentation/git.txt\n+++ b/Documentation/git.txt\n@@ -886,8 +886,7 @@ for full details.\n \t`protocol.allow` is set to `never`, and each of the listed\n \tprotocols has `protocol..allow` set to `always`\n \t(overriding any existing configuration). In other words, any\n-\tprotocol not mentioned will be disallowed (i.e., this is a\n-\twhitelist, not a blacklist). See the description of\n+\tprotocol not mentioned will be disallowed. See the description of\n \t`protocol.allow` in linkgit:git-config[1] for more details.\n \n `GIT_PROTOCOL_FROM_USER`::\n-- \ngitgitgadget\n\n"},{"id":"459113","messageId":"pull.1274.v2.git.1657852722.gitgitgadget@gmail.com","threadId":"58152","inReplyTo":"pull.1274.git.1657718450.gitgitgadget@gmail.com","subject":"[PATCH v2 0/3] Remove use of \"whitelist\"","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-07-15T02:38:39Z","receivedAt":"2022-07-15T02:38:53Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"The word \"whitelist\" has cultural implications that make its use\nnon-inclusive.\n\nA previous version of this series recommended the replacement of\n\"allowlist\", but that term is still new and not accepted by some common\ndictionaries.\n\nInstead, this version avoids the use of \"whitelist\" by rewording the\nsentences that use it. In many cases, this improves readability since the\nterm is used suddenly without other context (and in some cases, is not\nnecessary at all).\n\nThere is one case where \"whitelist\" is replaced with \"allow_list\" but that\nis because we are operating on a string list parsed from the\nGIT_ALLOW_PROTOCOL environment variable.\n\nThanks, -Stolee\n\nDerrick Stolee (3):\n Documentation: remove use of whitelist\n t/*: avoid \"whitelist\"\n *: avoid \"whitelist\"\n\n Documentation/git-cvsserver.txt | 2 +-\n Documentation/git-daemon.txt | 15 +++++++--------\n Documentation/git.txt | 3 +--\n daemon.c | 8 ++++----\n git-cvsserver.perl | 2 +-\n t/README | 9 ++++-----\n t/lib-proto-disable.sh | 6 +++---\n t/t5812-proto-disable-http.sh | 2 +-\n t/t5815-submodule-protos.sh | 4 ++--\n t/t9400-git-cvsserver-server.sh | 2 +-\n t/test-lib-functions.sh | 2 +-\n t/test-lib.sh | 2 +-\n transport.c | 8 ++++----\n 13 files changed, 31 insertions(+), 34 deletions(-)\n\n\nbase-commit: e4a4b31577c7419497ac30cebe30d755b97752c5\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1274%2Fderrickstolee%2Fallow-deny-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1274/derrickstolee/allow-deny-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/1274\n\nRange-diff vs v1:\n\n 1: ec81aac05c4 ! 1: 19632a2d245 Documentation: use allowlist and denylist\n @@ Metadata\n Author: Derrick Stolee \n \n ## Commit message ##\n - Documentation: use allowlist and denylist\n + Documentation: remove use of whitelist\n \n - Using \"allowlist\" and \"denylist\" is a more precise definition of the\n - functionality they provide. The previous color-based words assume\n - cultural interpretation to provide the meaning.\n + The word \"whitelist\" has cultural implications that are not inclusive.\n + Thankfully, it is not difficult to reword and avoid its use.\n \n - Focus on replacements in the Documentation/ directory since these are\n - not functional uses.\n + Remove uses of \"whitelist\" in the Documentation/ directory. In all\n + cases, we can rewrite the sentences to add clarity instead of relying on\n + the reader understanding this term.\n \n + The most substantial change is to git-daemon.txt which had several uses,\n + but we can refer to the input directories as a list of included\n + directories, making the descriptions slightly simpler.\n +\n + Helped-by: Jeff King \n + Helped-by: Junio C Hamano \n Signed-off-by: Derrick Stolee \n \n ## Documentation/git-cvsserver.txt ##\n @@ Documentation/git-cvsserver.txt: circumstances, allowing easier restricted usage\n GIT_CVSSERVER_BASE_PATH takes the place of the argument to --base-path.\n \n -GIT_CVSSERVER_ROOT specifies a single-directory whitelist. The\n -+GIT_CVSSERVER_ROOT specifies a single-directory allowlist. The\n ++GIT_CVSSERVER_ROOT specifies a single allowed directory. The\n repository must still be configured to allow access through\n git-cvsserver, as described above.\n \n \n ## Documentation/git-daemon.txt ##\n -@@ Documentation/git-daemon.txt: It verifies that the directory has the magic file \"git-daemon-export-ok\", and\n +@@ Documentation/git-daemon.txt: that service if it is enabled.\n + It verifies that the directory has the magic file \"git-daemon-export-ok\", and\n it will refuse to export any Git directory that hasn't explicitly been marked\n for export this way (unless the `--export-all` parameter is specified). If you\n - pass some directory paths as 'git daemon' arguments, you can further restrict\n +-pass some directory paths as 'git daemon' arguments, you can further restrict\n -the offers to a whitelist comprising of those.\n -+the offers to a allowlist comprising of those.\n ++pass some directory paths as 'git daemon' arguments, the offers are limited to\n ++repositories within those directories.\n \n By default, only `upload-pack` service is enabled, which serves\n 'git fetch-pack' and 'git ls-remote' clients, which are invoked\n @@ Documentation/git-daemon.txt: OPTIONS\n \t\"/foo/repo.git\" or \"/foo/repo/.git\") and don't do user-relative paths.\n \t'git daemon' will refuse to start when this option is enabled and no\n -\twhitelist is specified.\n -+\tallowlist is specified.\n ++\tspecific directories are specified.\n \n --base-path=::\n \tRemap all the path requests as relative to the given path.\n @@ Documentation/git-daemon.txt: OPTIONS\n \tand %D for the absolute path of the named repository.\n \tAfter interpolation, the path is validated against the directory\n -\twhitelist.\n -+\tallowlist.\n ++\tlist.\n \n --export-all::\n \tAllow pulling from all directories that look like Git repositories\n @@ Documentation/git-daemon.txt: standard output to be sent to the requestor as an\n \n ::\n -\tA directory to add to the whitelist of allowed directories. Unless\n -+\tA directory to add to the allowlist of allowed directories. Unless\n ++\tA directory to add to the list of allowed directories. Unless\n \t--strict-paths is specified this will also include subdirectories\n \tof each named directory.\n \n @@ Documentation/git-daemon.txt: git\t\t9418/tcp\t\t# Git Version Control System\n 'git daemon' as inetd server::\n \tTo set up 'git daemon' as an inetd service that handles any\n -\trepository under the whitelisted set of directories, /pub/foo\n -+\trepository under the allowlisted set of directories, /pub/foo\n - \tand /pub/bar, place an entry like the following into\n - \t/etc/inetd all on one line:\n +-\tand /pub/bar, place an entry like the following into\n +-\t/etc/inetd all on one line:\n ++\trepository in the directory list of `/pub/foo` and `/pub/bar`,\n ++\tplace an entry like the following into `/etc/inetd` all on one line:\n +\n + ------------------------------------------------\n + \tgit stream tcp nowait nobody /usr/bin/git\n \n ## Documentation/git.txt ##\n @@ Documentation/git.txt: for full details.\n + \t`protocol.allow` is set to `never`, and each of the listed\n \tprotocols has `protocol..allow` set to `always`\n \t(overriding any existing configuration). In other words, any\n - \tprotocol not mentioned will be disallowed (i.e., this is a\n +-\tprotocol not mentioned will be disallowed (i.e., this is a\n -\twhitelist, not a blacklist). See the description of\n -+\tallowlist, not a denylist). See the description of\n ++\tprotocol not mentioned will be disallowed. See the description of\n \t`protocol.allow` in linkgit:git-config[1] for more details.\n \n `GIT_PROTOCOL_FROM_USER`::\n 2: 0d82c2a09c5 ! 2: 3c3c8c20bcb t/*: use allowlist\n @@ Metadata\n Author: Derrick Stolee \n \n ## Commit message ##\n - t/*: use allowlist\n + t/*: avoid \"whitelist\"\n \n - Using \"allowlist\" is a more precise definition of the functionality\n - provided. The previous color-based word assume cultural interpretation\n - to provide the meaning.\n + The word \"whitelist\" has cultural implications that are not inclusive.\n + Thankfully, it is not difficult to reword and avoid its use.\n \n Focus on changes in the test scripts, since most of the changes are in\n - comments and test names. The one exception is the renamed test_allowlist\n - helper.\n + comments and test names. The renamed test_allow_var helper is only used\n + once inside the widely-used test_proto helper.\n \n Signed-off-by: Derrick Stolee \n \n @@ t/README: GIT_TEST_SPLIT_INDEX= forces split-index mode on the whole\n GIT_TEST_PASSING_SANITIZE_LEAK= when compiled with\n -SANITIZE=leak will run only those tests that have whitelisted\n -themselves as passing with no memory leaks. Tests can be whitelisted\n -+SANITIZE=leak will run only those tests that have allowlisted\n -+themselves as passing with no memory leaks. Tests can be allowlisted\n - by setting \"TEST_PASSES_SANITIZE_LEAK=true\" before sourcing\n - \"test-lib.sh\" itself at the top of the test script. This test mode is\n - used by the \"linux-leaks\" CI target.\n +-by setting \"TEST_PASSES_SANITIZE_LEAK=true\" before sourcing\n +-\"test-lib.sh\" itself at the top of the test script. This test mode is\n +-used by the \"linux-leaks\" CI target.\n ++SANITIZE=leak will run only those tests that have marked themselves as\n ++passing with no memory leaks by setting \"TEST_PASSES_SANITIZE_LEAK=true\"\n ++before sourcing \"test-lib.sh\" itself at the top of the test script. This\n ++test mode is used by the \"linux-leaks\" CI target.\n + \n + GIT_TEST_PROTOCOL_VERSION=, when set, makes 'protocol.version'\n + default to n.\n \n ## t/lib-proto-disable.sh ##\n @@\n @@ t/lib-proto-disable.sh\n \n -# Test clone/fetch/push with GIT_ALLOW_PROTOCOL whitelist\n -test_whitelist () {\n -+# Test clone/fetch/push with GIT_ALLOW_PROTOCOL allowlist\n -+test_allowlist () {\n ++# Test clone/fetch/push with GIT_ALLOW_PROTOCOL environment variable\n ++test_allow_var () {\n \tdesc=$1\n \tproto=$2\n \turl=$3\n @@ t/lib-proto-disable.sh: test_config () {\n # $3 - the URL to try cloning\n test_proto () {\n -\ttest_whitelist \"$@\"\n -+\ttest_allowlist \"$@\"\n ++\ttest_allow_var \"$@\"\n \n \ttest_config \"$@\"\n }\n @@ t/t5812-proto-disable-http.sh: test_expect_success 'create git-accessible repo'\n test_proto \"smart http\" http \"$HTTPD_URL/smart/repo.git\"\n \n -test_expect_success 'curl redirects respect whitelist' '\n -+test_expect_success 'curl redirects respect allowlist' '\n ++test_expect_success 'curl redirects respect allowed protocols' '\n \ttest_must_fail env GIT_ALLOW_PROTOCOL=http:https \\\n \t\t\t GIT_SMART_HTTP=0 \\\n \t\tgit clone \"$HTTPD_URL/ftp-redir/repo.git\" 2>stderr &&\n @@ t/t5815-submodule-protos.sh\n #!/bin/sh\n \n -test_description='test protocol whitelisting with submodules'\n -+test_description='test protocol allowlisting with submodules'\n ++test_description='test protocol restrictions with submodules'\n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/lib-proto-disable.sh\n \n @@ t/t5815-submodule-protos.sh: test_expect_success 'update of ext not allowed' '\n '\n \n -test_expect_success 'user can override whitelist' '\n -+test_expect_success 'user can override allowlist' '\n ++test_expect_success 'user can override with environment variable' '\n \tGIT_ALLOW_PROTOCOL=ext git -C dst submodule update ext-module\n '\n \n @@ t/t9400-git-cvsserver-server.sh: test_expect_success 'req_Root (export-all)' \\\n sed -ne \\$p log | grep \"^I LOVE YOU\\$\"'\n \n -test_expect_success 'req_Root failure (export-all w/o whitelist)' \\\n -+test_expect_success 'req_Root failure (export-all w/o allowlist)' \\\n ++test_expect_success 'req_Root failure (export-all w/o directory)' \\\n '! (cat request-anonymous | git-cvsserver --export-all pserver >log 2>&1 || false)'\n \n test_expect_success 'req_Root (everything together)' \\\n @@ t/test-lib-functions.sh: test_set_prereq () {\n \t\t!*)\n \t\t\t;;\n -\t\t# (Temporary?) whitelist of things we can't easily\n -+\t\t# (Temporary?) allowlist of things we can't easily\n ++\t\t# (Temporary?) list of things we can't easily\n \t\t# pretend not to support\n \t\tSYMLINKS)\n \t\t\t;;\n @@ t/test-lib.sh: then\n fi\n \n -# skip non-whitelisted tests when compiled with SANITIZE=leak\n -+# skip non-allowlisted tests when compiled with SANITIZE=leak\n ++# skip unmarked tests when compiled with SANITIZE=leak\n if test -n \"$SANITIZE_LEAK\"\n then\n \tif test_bool_env GIT_TEST_PASSING_SANITIZE_LEAK false\n 3: 8aaceedb7a8 ! 3: 0d862cbbebe *: use allowlist and denylist\n @@ Metadata\n Author: Derrick Stolee \n \n ## Commit message ##\n - *: use allowlist and denylist\n + *: avoid \"whitelist\"\n \n - Using \"allowlist\" and \"denylist\" is a more precise definition of the\n - functionality they provide. The previous color-based words assume\n - cultural interpretation to provide the meaning.\n + The word \"whitelist\" has cultural implications that are not inclusive.\n + Thankfully, it is not difficult to reword and avoid its use.\n \n - These changes to the Git codebase are mostly cosmetic. Several comments\n - are updated. The renamed protocol_allowlist() method is local to\n - transport.c so does not update any header file API definition. There are\n - some untranslated error messages that are reworded, so this _might_\n - affect error parsers. However, two of the three error messages are\n - around option parsing, so they are \"fast failures\". The one perhaps\n - meaningful change is the logerror() in daemon.c.\n + A previous change already modified the documentation for git-cvsserver\n + and git-daemon to refer to a directory list. It is simple to update the\n + comments and error messages here to refer to that directory list.\n \n - After this change, the only remaining uses of the previous words are\n - in release notes for older versions of Git.\n + In the case of transport.c, the GIT_ALLOW_PROTOCOL environment variable\n + was referred to as a \"whitelist\", but the word \"allow\" is already part\n + of the variable. Replace \"whitelist\" with \"allow_list\" in these cases to\n + demonstrate that we are processing a list of allowed protocols.\n +\n + After this change, the only remaining uses of \"whitelist\" and its\n + companion \"blacklist\" are in release notes for older versions of Git and\n + in the sha1dc project, which is maintained independently.\n \n Signed-off-by: Derrick Stolee \n \n @@ daemon.c: static const char *path_ok(const char *directory, struct hostinfo *hi)\n \t\t * appends optional {.git,.git/.git} and friends, but\n \t\t * it does not use getcwd(). So if your /pub is\n -\t\t * a symlink to /mnt/pub, you can whitelist /pub and\n -+\t\t * a symlink to /mnt/pub, you can allowlist /pub and\n ++\t\t * a symlink to /mnt/pub, you can include /pub and\n \t\t * do not have to say /mnt/pub.\n \t\t * Do not say /pub/.\n \t\t */\n @@ daemon.c: static const char *path_ok(const char *directory, struct hostinfo *hi)\n \t}\n \n -\tlogerror(\"'%s': not in whitelist\", path);\n -+\tlogerror(\"'%s': not in allowlist\", path);\n ++\tlogerror(\"'%s': not in directory list\", path);\n \treturn NULL;\t\t/* Fallthrough. Deny by default */\n }\n \n @@ daemon.c: static int run_service(const char *dir, struct daemon_service *service\n \t * is ok with us doing this.\n \t *\n -\t * path_ok() uses enter_repo() and does whitelist checking.\n -+\t * path_ok() uses enter_repo() and does allowlist checking.\n ++\t * path_ok() uses enter_repo() and checks for included directories.\n \t * We only need to make sure the repository is exported.\n \t */\n \n @@ daemon.c: int cmd_main(int argc, const char **argv)\n \n \tif (strict_paths && (!ok_paths || !*ok_paths))\n -\t\tdie(\"option --strict-paths requires a whitelist\");\n -+\t\tdie(\"option --strict-paths requires a allowlist\");\n ++\t\tdie(\"option --strict-paths requires a directory list\");\n \n \tif (base_path && !is_directory(base_path))\n \t\tdie(\"base-path '%s' does not exist or is not a directory\",\n @@ git-cvsserver.perl: $state->{allowed_roots} = [ @ARGV ];\n # don't export the whole system unless the users requests it\n if ($state->{'export-all'} && !@{$state->{allowed_roots}}) {\n - die \"--export-all can only be used together with an explicit whitelist\\n\";\n -+ die \"--export-all can only be used together with an explicit allowlist\\n\";\n ++ die \"--export-all can only be used together with an explicit directory list\\n\";\n }\n \n # Environment handling for running under git-shell\n \n - ## sha1dc/sha1.c ##\n -@@\n - defined(__MIPSEB__) || defined(__MIPSEB) || defined(_MIPSEB) || \\\n - defined(__sparc))\n - /*\n -- * Should define Big Endian for a whitelist of known processors. See\n -+ * Should define Big Endian for a allowlist of known processors. See\n - * https://sourceforge.net/p/predef/wiki/Endianness/ and\n - * http://www.oracle.com/technetwork/server-storage/solaris/portingtosolaris-138514.html\n - */\n - #define SHA1DC_BIGENDIAN\n - \n --/* Not under GCC-alike or glibc or *BSD or newlib or */\n -+/* Not under GCC-alike or glibc or *BSD or newlib or */\n - #elif (defined(_AIX) || defined(__hpux))\n - \n - /*\n -- * Defines Big Endian on a whitelist of OSs that are known to be Big\n -+ * Defines Big Endian on a allowlist of OSs that are known to be Big\n - * Endian-only. See\n - * https://lore.kernel.org/git/93056823-2740-d072-1ebd-46b440b33d7e@felt.demon.nl/\n - */\n - #define SHA1DC_BIGENDIAN\n - \n --/* Not under GCC-alike or glibc or *BSD or newlib or or */\n -+/* Not under GCC-alike or glibc or *BSD or newlib or or */\n - #elif defined(SHA1DC_ON_INTEL_LIKE_PROCESSOR)\n - /*\n - * As a last resort before we do anything else we're not 100% sure\n -- * about below, we blacklist specific processors here. We could add\n -+ * about below, we denylist specific processors here. We could add\n - * more, see e.g. https://wiki.debian.org/ArchitectureSpecificsMemo\n - */\n --#else /* Not under GCC-alike or glibc or *BSD or newlib or or or */\n -+#else /* Not under GCC-alike or glibc or *BSD or newlib or or or */\n - \n - /* We do nothing more here for now */\n - /*#error \"Uncomment this to see if you fall through all the detection\"*/\n -\n ## transport.c ##\n @@ transport.c: static int external_specification_len(const char *url)\n \treturn strchr(url, ':') - url;\n }\n \n -static const struct string_list *protocol_whitelist(void)\n -+static const struct string_list *protocol_allowlist(void)\n ++static const struct string_list *protocol_allow_list(void)\n {\n \tstatic int enabled = -1;\n \tstatic struct string_list allowed = STRING_LIST_INIT_DUP;\n @@ transport.c: static enum protocol_allow_config get_protocol_config(const char *t\n -\tconst struct string_list *whitelist = protocol_whitelist();\n -\tif (whitelist)\n -\t\treturn string_list_has_string(whitelist, type);\n -+\tconst struct string_list *allowlist = protocol_allowlist();\n -+\tif (allowlist)\n -+\t\treturn string_list_has_string(allowlist, type);\n ++\tconst struct string_list *allow_list = protocol_allow_list();\n ++\tif (allow_list)\n ++\t\treturn string_list_has_string(allow_list, type);\n \n \tswitch (get_protocol_config(type)) {\n \tcase PROTOCOL_ALLOW_ALWAYS:\n\n-- \ngitgitgadget\n"},{"id":"459112","messageId":"3c3c8c20bcb4e570d25a676ad1f29877762adb82.1657852722.git.gitgitgadget@gmail.com","threadId":"58152","inReplyTo":"pull.1274.v2.git.1657852722.gitgitgadget@gmail.com","subject":"[PATCH v2 2/3] t/*: avoid \"whitelist\"","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-07-15T02:38:41Z","receivedAt":"2022-07-15T02:38:54Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThe word \"whitelist\" has cultural implications that are not inclusive.\nThankfully, it is not difficult to reword and avoid its use.\n\nFocus on changes in the test scripts, since most of the changes are in\ncomments and test names. The renamed test_allow_var helper is only used\nonce inside the widely-used test_proto helper.\n\nSigned-off-by: Derrick Stolee \n---\n t/README | 9 ++++-----\n t/lib-proto-disable.sh | 6 +++---\n t/t5812-proto-disable-http.sh | 2 +-\n t/t5815-submodule-protos.sh | 4 ++--\n t/t9400-git-cvsserver-server.sh | 2 +-\n t/test-lib-functions.sh | 2 +-\n t/test-lib.sh | 2 +-\n 7 files changed, 13 insertions(+), 14 deletions(-)\n\ndiff --git a/t/README b/t/README\nindex 309a31133c6..56d5ebb5798 100644\n--- a/t/README\n+++ b/t/README\n@@ -367,11 +367,10 @@ GIT_TEST_SPLIT_INDEX= forces split-index mode on the whole\n test suite. Accept any boolean values that are accepted by git-config.\n \n GIT_TEST_PASSING_SANITIZE_LEAK= when compiled with\n-SANITIZE=leak will run only those tests that have whitelisted\n-themselves as passing with no memory leaks. Tests can be whitelisted\n-by setting \"TEST_PASSES_SANITIZE_LEAK=true\" before sourcing\n-\"test-lib.sh\" itself at the top of the test script. This test mode is\n-used by the \"linux-leaks\" CI target.\n+SANITIZE=leak will run only those tests that have marked themselves as\n+passing with no memory leaks by setting \"TEST_PASSES_SANITIZE_LEAK=true\"\n+before sourcing \"test-lib.sh\" itself at the top of the test script. This\n+test mode is used by the \"linux-leaks\" CI target.\n \n GIT_TEST_PROTOCOL_VERSION=, when set, makes 'protocol.version'\n default to n.\ndiff --git a/t/lib-proto-disable.sh b/t/lib-proto-disable.sh\nindex 83babe57d95..890622be816 100644\n--- a/t/lib-proto-disable.sh\n+++ b/t/lib-proto-disable.sh\n@@ -1,7 +1,7 @@\n # Test routines for checking protocol disabling.\n \n-# Test clone/fetch/push with GIT_ALLOW_PROTOCOL whitelist\n-test_whitelist () {\n+# Test clone/fetch/push with GIT_ALLOW_PROTOCOL environment variable\n+test_allow_var () {\n \tdesc=$1\n \tproto=$2\n \turl=$3\n@@ -183,7 +183,7 @@ test_config () {\n # $2 - machine-readable name of the protocol\n # $3 - the URL to try cloning\n test_proto () {\n-\ttest_whitelist \"$@\"\n+\ttest_allow_var \"$@\"\n \n \ttest_config \"$@\"\n }\ndiff --git a/t/t5812-proto-disable-http.sh b/t/t5812-proto-disable-http.sh\nindex af8772fadaa..bbeebee02f1 100755\n--- a/t/t5812-proto-disable-http.sh\n+++ b/t/t5812-proto-disable-http.sh\n@@ -16,7 +16,7 @@ test_expect_success 'create git-accessible repo' '\n \n test_proto \"smart http\" http \"$HTTPD_URL/smart/repo.git\"\n \n-test_expect_success 'curl redirects respect whitelist' '\n+test_expect_success 'curl redirects respect allowed protocols' '\n \ttest_must_fail env GIT_ALLOW_PROTOCOL=http:https \\\n \t\t\t GIT_SMART_HTTP=0 \\\n \t\tgit clone \"$HTTPD_URL/ftp-redir/repo.git\" 2>stderr &&\ndiff --git a/t/t5815-submodule-protos.sh b/t/t5815-submodule-protos.sh\nindex 06f55a1b8a0..990f034149d 100755\n--- a/t/t5815-submodule-protos.sh\n+++ b/t/t5815-submodule-protos.sh\n@@ -1,6 +1,6 @@\n #!/bin/sh\n \n-test_description='test protocol whitelisting with submodules'\n+test_description='test protocol restrictions with submodules'\n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/lib-proto-disable.sh\n \n@@ -36,7 +36,7 @@ test_expect_success 'update of ext not allowed' '\n \ttest_must_fail git -C dst submodule update ext-module\n '\n \n-test_expect_success 'user can override whitelist' '\n+test_expect_success 'user can override with environment variable' '\n \tGIT_ALLOW_PROTOCOL=ext git -C dst submodule update ext-module\n '\n \ndiff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh\nindex 210ddf09e30..51b798cb493 100755\n--- a/t/t9400-git-cvsserver-server.sh\n+++ b/t/t9400-git-cvsserver-server.sh\n@@ -221,7 +221,7 @@ test_expect_success 'req_Root (export-all)' \\\n 'cat request-anonymous | git-cvsserver --export-all pserver \"$WORKDIR\" >log 2>&1 &&\n sed -ne \\$p log | grep \"^I LOVE YOU\\$\"'\n \n-test_expect_success 'req_Root failure (export-all w/o whitelist)' \\\n+test_expect_success 'req_Root failure (export-all w/o directory)' \\\n '! (cat request-anonymous | git-cvsserver --export-all pserver >log 2>&1 || false)'\n \n test_expect_success 'req_Root (everything together)' \\\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex 6da7273f1d5..6fe62329d8b 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -651,7 +651,7 @@ test_set_prereq () {\n \t\t# test_unset_prereq()\n \t\t!*)\n \t\t\t;;\n-\t\t# (Temporary?) whitelist of things we can't easily\n+\t\t# (Temporary?) list of things we can't easily\n \t\t# pretend not to support\n \t\tSYMLINKS)\n \t\t\t;;\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 55857af601b..fff85f4b425 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -1408,7 +1408,7 @@ then\n \ttest_done\n fi\n \n-# skip non-whitelisted tests when compiled with SANITIZE=leak\n+# skip unmarked tests when compiled with SANITIZE=leak\n if test -n \"$SANITIZE_LEAK\"\n then\n \tif test_bool_env GIT_TEST_PASSING_SANITIZE_LEAK false\n-- \ngitgitgadget\n\n"},{"id":"459114","messageId":"0d862cbbebe1a9f47f72255217faf734b1db4055.1657852722.git.gitgitgadget@gmail.com","threadId":"58152","inReplyTo":"pull.1274.v2.git.1657852722.gitgitgadget@gmail.com","subject":"[PATCH v2 3/3] *: avoid \"whitelist\"","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-07-15T02:38:42Z","receivedAt":"2022-07-15T02:38:58Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThe word \"whitelist\" has cultural implications that are not inclusive.\nThankfully, it is not difficult to reword and avoid its use.\n\nA previous change already modified the documentation for git-cvsserver\nand git-daemon to refer to a directory list. It is simple to update the\ncomments and error messages here to refer to that directory list.\n\nIn the case of transport.c, the GIT_ALLOW_PROTOCOL environment variable\nwas referred to as a \"whitelist\", but the word \"allow\" is already part\nof the variable. Replace \"whitelist\" with \"allow_list\" in these cases to\ndemonstrate that we are processing a list of allowed protocols.\n\nAfter this change, the only remaining uses of \"whitelist\" and its\ncompanion \"blacklist\" are in release notes for older versions of Git and\nin the sha1dc project, which is maintained independently.\n\nSigned-off-by: Derrick Stolee \n---\n daemon.c | 8 ++++----\n git-cvsserver.perl | 2 +-\n transport.c | 8 ++++----\n 3 files changed, 9 insertions(+), 9 deletions(-)\n\ndiff --git a/daemon.c b/daemon.c\nindex 58f1077885c..e0706efc652 100644\n--- a/daemon.c\n+++ b/daemon.c\n@@ -279,7 +279,7 @@ static const char *path_ok(const char *directory, struct hostinfo *hi)\n \t\t/* The validation is done on the paths after enter_repo\n \t\t * appends optional {.git,.git/.git} and friends, but\n \t\t * it does not use getcwd(). So if your /pub is\n-\t\t * a symlink to /mnt/pub, you can whitelist /pub and\n+\t\t * a symlink to /mnt/pub, you can include /pub and\n \t\t * do not have to say /mnt/pub.\n \t\t * Do not say /pub/.\n \t\t */\n@@ -298,7 +298,7 @@ static const char *path_ok(const char *directory, struct hostinfo *hi)\n \t\t\treturn path;\n \t}\n \n-\tlogerror(\"'%s': not in whitelist\", path);\n+\tlogerror(\"'%s': not in directory list\", path);\n \treturn NULL;\t\t/* Fallthrough. Deny by default */\n }\n \n@@ -403,7 +403,7 @@ static int run_service(const char *dir, struct daemon_service *service,\n \t * a \"git-daemon-export-ok\" flag that says that the other side\n \t * is ok with us doing this.\n \t *\n-\t * path_ok() uses enter_repo() and does whitelist checking.\n+\t * path_ok() uses enter_repo() and checks for included directories.\n \t * We only need to make sure the repository is exported.\n \t */\n \n@@ -1444,7 +1444,7 @@ int cmd_main(int argc, const char **argv)\n \t\tcred = prepare_credentials(user_name, group_name);\n \n \tif (strict_paths && (!ok_paths || !*ok_paths))\n-\t\tdie(\"option --strict-paths requires a whitelist\");\n+\t\tdie(\"option --strict-paths requires a directory list\");\n \n \tif (base_path && !is_directory(base_path))\n \t\tdie(\"base-path '%s' does not exist or is not a directory\",\ndiff --git a/git-cvsserver.perl b/git-cvsserver.perl\nindex 4c8118010a8..ec64f06af7c 100755\n--- a/git-cvsserver.perl\n+++ b/git-cvsserver.perl\n@@ -152,7 +152,7 @@ $state->{allowed_roots} = [ @ARGV ];\n \n # don't export the whole system unless the users requests it\n if ($state->{'export-all'} && !@{$state->{allowed_roots}}) {\n- die \"--export-all can only be used together with an explicit whitelist\\n\";\n+ die \"--export-all can only be used together with an explicit directory list\\n\";\n }\n \n # Environment handling for running under git-shell\ndiff --git a/transport.c b/transport.c\nindex 52db7a3cb09..b51e991e443 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -940,7 +940,7 @@ static int external_specification_len(const char *url)\n \treturn strchr(url, ':') - url;\n }\n \n-static const struct string_list *protocol_whitelist(void)\n+static const struct string_list *protocol_allow_list(void)\n {\n \tstatic int enabled = -1;\n \tstatic struct string_list allowed = STRING_LIST_INIT_DUP;\n@@ -1020,9 +1020,9 @@ static enum protocol_allow_config get_protocol_config(const char *type)\n \n int is_transport_allowed(const char *type, int from_user)\n {\n-\tconst struct string_list *whitelist = protocol_whitelist();\n-\tif (whitelist)\n-\t\treturn string_list_has_string(whitelist, type);\n+\tconst struct string_list *allow_list = protocol_allow_list();\n+\tif (allow_list)\n+\t\treturn string_list_has_string(allow_list, type);\n \n \tswitch (get_protocol_config(type)) {\n \tcase PROTOCOL_ALLOW_ALWAYS:\n-- \ngitgitgadget\n"},{"id":"459123","messageId":"xmqq5yjy3n2c.fsf@gitster.g","threadId":"58152","inReplyTo":"pull.1274.v2.git.1657852722.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 0/3] Remove use of \"whitelist\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-07-15T06:30:19Z","receivedAt":"2022-07-15T06:31:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Derrick Stolee via GitGitGadget\" writes:\n\n> The word \"whitelist\" has cultural implications that make its use\n> non-inclusive.\n>\n> A previous version of this series recommended the replacement of\n> \"allowlist\", but that term is still new and not accepted by some common\n> dictionaries.\n>\n> Instead, this version avoids the use of \"whitelist\" by rewording the\n> sentences that use it. In many cases, this improves readability since the\n> term is used suddenly without other context (and in some cases, is not\n> necessary at all).\n>\n> There is one case where \"whitelist\" is replaced with \"allow_list\" but that\n> is because we are operating on a string list parsed from the\n> GIT_ALLOW_PROTOCOL environment variable.\n>\n> Thanks, -Stolee\n\nMaybe I am biased, but for all the changes in these patches, I find\nthe updated text far easier to understand than a mere replacing of\nthe words s/white/allow/, even if I pretend that allowlist is\nconsidered by everybody a proper part of English vocabulary. After\nall, I think most of the places did not have to say \"whitelist\" in\nthe first place.\n\nWill queue.\n\nThanks.\n"},{"id":"459137","messageId":"220715.86sfn2zlkm.gmgdl@evledraar.gmail.com","threadId":"58152","inReplyTo":"19632a2d2459114484dcdaddd4db0d22dfd64fa7.1657852722.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 1/3] Documentation: remove use of whitelist","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-07-15T10:47:48Z","receivedAt":"2022-07-15T11:01:36Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Jul 15 2022, Derrick Stolee via GitGitGadget wrote:\n\n> From: Derrick Stolee \n> [...]\n> diff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt\n> index fdc28c041c7..7a0539cb411 100644\n> --- a/Documentation/git-daemon.txt\n> +++ b/Documentation/git-daemon.txt\n> @@ -32,8 +32,8 @@ that service if it is enabled.\n> It verifies that the directory has the magic file \"git-daemon-export-ok\", and\n> it will refuse to export any Git directory that hasn't explicitly been marked\n> for export this way (unless the `--export-all` parameter is specified). If you\n> -pass some directory paths as 'git daemon' arguments, you can further restrict\n> -the offers to a whitelist comprising of those.\n> +pass some directory paths as 'git daemon' arguments, the offers are limited to\n> +repositories within those directories.\n> \n> By default, only `upload-pack` service is enabled, which serves\n> 'git fetch-pack' and 'git ls-remote' clients, which are invoked\n> @@ -50,7 +50,7 @@ OPTIONS\n> \tMatch paths exactly (i.e. don't allow \"/foo/repo\" when the real path is\n> \t\"/foo/repo.git\" or \"/foo/repo/.git\") and don't do user-relative paths.\n> \t'git daemon' will refuse to start when this option is enabled and no\n> -\twhitelist is specified.\n> +\tspecific directories are specified.\n\nStructurally this series should be changed so that like changes are\ncoupled together, this would be much easier to review with the daemon.c\nchanges in 3/3.\n\nBut that also shows that this change is needed, but really lacking\ncompared to what we could do here, which is that both the the SYNOPSIS\nand the heading here should be, respectively:\n\n\n [--strict-paths=...]\n\nAnd:\n\n --strict-paths=...:\n\nI.e. all we're trying to get across here is \"this option has a mandatory\nargument\", so let's just say something like that explicitly? I think in\nthis case we don't need the prose at all, the synopsis + heading + error\nwould be enough.\n\nMore generally: As I noted on v1 I think the underlying motivation for\nthe series is mistaken, but I'm also happy for any excuse people can\nfind to improve our documentation.\n\nBut as I pointed out on your similar earlier series to expunge gendered\npronouns from the docs I think best practices in our docs just happen to\nalign with what you want.\n\nI.e. in that case we prefer a style that isn't introducing \"actors\"\nanyway, so even without that motivation the prose could be improved.\n\nAnd ditto here, I think the use of the term \"whitelist\" is fine in and\nof itself, but in this case we can clearly improve the docs anyway.\n\nI just think that the origin of the change really shows in this case, I\nhaven't yet reviewed the rest but suspect I'll find something\nsimilar. I.e. there's a clear improvement to be made, but since it was\nfirst made with s/whitelist/allowlist/g, and now an attempt to\ns/whitelist//g in some form we lose sight of the larger picture.\n\nWhich in thihs case is that perhaps the sentence isn't needed at all,\nand that the synopsis & title is the real thing worth fixing.\n\nSo, if what motivates doc improvements in your case is working on your\nlocal dictionary black^Hdenylist :): great, we can probably improve our\ndocs in any case. But please spend a bit of time eyeballing the change\nwithout an eye to that motivation, sometimes we can improve it much more\nwithout much effert...\n\n> diff --git a/Documentation/git.txt b/Documentation/git.txt\n> index 302607a4967..dd5061563eb 100644\n> --- a/Documentation/git.txt\n> +++ b/Documentation/git.txt\n> @@ -886,8 +886,7 @@ for full details.\n> \t`protocol.allow` is set to `never`, and each of the listed\n> \tprotocols has `protocol..allow` set to `always`\n> \t(overriding any existing configuration). In other words, any\n> -\tprotocol not mentioned will be disallowed (i.e., this is a\n> -\twhitelist, not a blacklist). See the description of\n> +\tprotocol not mentioned will be disallowed. See the description of\n> \t`protocol.allow` in linkgit:git-config[1] for more details.\n> \n> `GIT_PROTOCOL_FROM_USER`::\n\nI agree with Junio's earlier feedback about \"in other words\" being a\ntelltale sign of prose that needs improving.\n\nBut the point of the previous prose (such as it was) was to elaborate on\nth existing \"allow\" to say \"oh, allow means the same as whitelist\",\nsurely?\n\nSo I think we really could just delete this \"in other words\" entirely,\ni.e. it's basically saying \"you are allowed to eat ice cream (in other\nwords, you are not disallowed)\", it's not adding anything anymore. The\n\"(...)\" can just be removed.\n"},{"id":"459138","messageId":"220715.86o7xqzkt3.gmgdl@evledraar.gmail.com","threadId":"58152","inReplyTo":"3c3c8c20bcb4e570d25a676ad1f29877762adb82.1657852722.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 2/3] t/*: avoid \"whitelist\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-07-15T11:02:50Z","receivedAt":"2022-07-15T11:18:06Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Jul 15 2022, Derrick Stolee via GitGitGadget wrote:\n\n> From: Derrick Stolee \n>\n> The word \"whitelist\" has cultural implications that are not inclusive.\n> Thankfully, it is not difficult to reword and avoid its use.\n>\n> Focus on changes in the test scripts, since most of the changes are in\n> comments and test names. The renamed test_allow_var helper is only used\n> once inside the widely-used test_proto helper.\n>\n> Signed-off-by: Derrick Stolee \n> ---\n> t/README | 9 ++++-----\n> t/lib-proto-disable.sh | 6 +++---\n> t/t5812-proto-disable-http.sh | 2 +-\n> t/t5815-submodule-protos.sh | 4 ++--\n> t/t9400-git-cvsserver-server.sh | 2 +-\n> t/test-lib-functions.sh | 2 +-\n> t/test-lib.sh | 2 +-\n> 7 files changed, 13 insertions(+), 14 deletions(-)\n>\n> diff --git a/t/README b/t/README\n> index 309a31133c6..56d5ebb5798 100644\n> --- a/t/README\n> +++ b/t/README\n> @@ -367,11 +367,10 @@ GIT_TEST_SPLIT_INDEX= forces split-index mode on the whole\n> test suite. Accept any boolean values that are accepted by git-config.\n> \n> GIT_TEST_PASSING_SANITIZE_LEAK= when compiled with\n> -SANITIZE=leak will run only those tests that have whitelisted\n> -themselves as passing with no memory leaks. Tests can be whitelisted\n> -by setting \"TEST_PASSES_SANITIZE_LEAK=true\" before sourcing\n> -\"test-lib.sh\" itself at the top of the test script. This test mode is\n> -used by the \"linux-leaks\" CI target.\n> +SANITIZE=leak will run only those tests that have marked themselves as\n> +passing with no memory leaks by setting \"TEST_PASSES_SANITIZE_LEAK=true\"\n> +before sourcing \"test-lib.sh\" itself at the top of the test script. This\n> +test mode is used by the \"linux-leaks\" CI target.\n\nIt's hard to improve your own verbage, but I think in this case my\noriginal version can be improved still:\n\n\tGIT_TEST_PASSING_SANITIZE_LEAK= when compiled with\n\tSANITIZE=leak will, when true, only run those tests that declare\n\tthemselves leak-free by setting \"TEST_PASSES_SANITIZE_LEAK=true\"\n\tbefore sourcing \"test-lib.sh\". This test mode is used by the\n\t\"linux-leaks\" CI target.\n\n> -test_expect_success 'curl redirects respect whitelist' '\n> +test_expect_success 'curl redirects respect allowed protocols' '\n\nIsn't the real problem here that this is inaccurate with regards to\n\"curl\", i.e. AFAIK from browsing transport.c the whitelist of protocols\nhas nothing to do with curl, we parse that out and apply it before we\never get to the specific transport layer.\n\nSo this should just be \"http(s) transport respects GIT_ALLOW_PROTOCOL\",\nno?\n\n> -test_description='test protocol whitelisting with submodules'\n> +test_description='test protocol restrictions with submodules'\n\nMinor: I think this shows the awkwardness of using a word derived from\n\"allow\". Before we could use \"whitelist\" and \"whitelisting\"\nconsistentlry, but now you have \"allowed\", \"allowlist\", \"restrictions\"\netc.\n\nI guess you could say \"test protocol allowances...\" or something? Meh.\n\n> -test_expect_success 'user can override whitelist' '\n> +test_expect_success 'user can override with environment variable' '\n\nOverride what? This is a non-improvement. Saying \"how\" with \"environment\nvariable\" is good, but we dropped any mention of what's being tested.\n\n> -test_expect_success 'req_Root failure (export-all w/o whitelist)' \\\n> +test_expect_success 'req_Root failure (export-all w/o directory)' \\\n> '! (cat request-anonymous | git-cvsserver --export-all pserver >log 2>&1 || false)'\n\nIs it really that we don't have any directiory whatsoever, in that case\nI think this is a canditate for splitting up. I.e. the test didn't test\nwhat it was intending to test?\n\nBut if it's really \"without \" we should still say that, no?\n\n> -\t\t# (Temporary?) whitelist of things we can't easily\n> +\t\t# (Temporary?) list of things we can't easily\n> \t\t# pretend not to support\n\nWe've had this since my dfe1a17df9b (tests: add a special setup where\nprerequisites fail, 2019-05-13), so we can probably just drop\n\"(Temporary?)\" there while we're at it...\n\n> -# skip non-whitelisted tests when compiled with SANITIZE=leak\n> +# skip unmarked tests when compiled with SANITIZE=leak\n\nThis subtly changes the meaning in a way I didn't intend when writing\nthis. I.e. the existing wording could be improved, but some of what\nwe're skipping isn't \"unmarked\", it's explicitly marked as \"false\"\n(which we assume by default).\n\nI.e. I read this new wording as saying \"skip \", whereas the\nprevious can encompas \"\", just not \"\"...\n"},{"id":"459139","messageId":"220715.86k08ezkbl.gmgdl@evledraar.gmail.com","threadId":"58152","inReplyTo":"0d862cbbebe1a9f47f72255217faf734b1db4055.1657852722.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 3/3] *: avoid \"whitelist\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-07-15T11:19:58Z","receivedAt":"2022-07-15T11:30:03Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Jul 15 2022, Derrick Stolee via GitGitGadget wrote:\n\n> From: Derrick Stolee \n> [...]\n> -\tlogerror(\"'%s': not in whitelist\", path);\n> +\tlogerror(\"'%s': not in directory list\", path);\n> \treturn NULL;\t\t/* Fallthrough. Deny by default */\n> }\n> \tif (strict_paths && (!ok_paths || !*ok_paths))\n> -\t\tdie(\"option --strict-paths requires a whitelist\");\n> +\t\tdie(\"option --strict-paths requires a directory list\");\n> \n> \tif (base_path && !is_directory(base_path))\n> \t\tdie(\"base-path '%s' does not exist or is not a directory\",\n\nThe former here is worse than before, which relates to the latter.\n\nPut yourself in the shoes of a user who gets this error message. You\nthen do \"man git-daemon\", search for \"whitelist\", and the only thing\nyou'll find are references to --strict-paths and --interpolated-path,\nwhich puts you on the right path.\n\nIOW I don't care about the term \"whitelist\" here, I care that it it has\n(goes and counts...) 5 occurances in the manpage, directorty has 16.\n\nAnd SYNOPSIS of \"git-daemon\" says \"...[ \"Derrick Stolee via GitGitGadget\" writes:\n> \n>> The word \"whitelist\" has cultural implications that make its use\n>> non-inclusive.\n>>\n>> A previous version of this series recommended the replacement of\n>> \"allowlist\", but that term is still new and not accepted by some common\n>> dictionaries.\n>>\n>> Instead, this version avoids the use of \"whitelist\" by rewording the\n>> sentences that use it. In many cases, this improves readability since the\n>> term is used suddenly without other context (and in some cases, is not\n>> necessary at all).\n>>\n>> There is one case where \"whitelist\" is replaced with \"allow_list\" but that\n>> is because we are operating on a string list parsed from the\n>> GIT_ALLOW_PROTOCOL environment variable.\n>>\n>> Thanks, -Stolee\n> \n> Maybe I am biased, but for all the changes in these patches, I find\n> the updated text far easier to understand than a mere replacing of\n> the words s/white/allow/,\n\nI was struck by that as well, the new wording is definitely an \nimprovement, thanks for working on it Stolee\n\nBest Wishes\n\nPhillips.\n"},{"id":"459367","messageId":"0fe031b8-1eec-7407-4e5c-cae298ddeb8e@github.com","threadId":"58152","inReplyTo":"220715.86sfn2zlkm.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH v2 1/3] Documentation: remove use of whitelist","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-07-19T14:21:04Z","receivedAt":"2022-07-19T14:32:15Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 7/15/2022 6:47 AM, Ævar Arnfjörð Bjarmason wrote:\n> \n> On Fri, Jul 15 2022, Derrick Stolee via GitGitGadget wrote:\n> \n>> From: Derrick Stolee \n>> [...]\n>> diff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt\n>> index fdc28c041c7..7a0539cb411 100644\n>> --- a/Documentation/git-daemon.txt\n>> +++ b/Documentation/git-daemon.txt\n>> @@ -32,8 +32,8 @@ that service if it is enabled.\n>> It verifies that the directory has the magic file \"git-daemon-export-ok\", and\n>> it will refuse to export any Git directory that hasn't explicitly been marked\n>> for export this way (unless the `--export-all` parameter is specified). If you\n>> -pass some directory paths as 'git daemon' arguments, you can further restrict\n>> -the offers to a whitelist comprising of those.\n>> +pass some directory paths as 'git daemon' arguments, the offers are limited to\n>> +repositories within those directories.\n>> \n>> By default, only `upload-pack` service is enabled, which serves\n>> 'git fetch-pack' and 'git ls-remote' clients, which are invoked\n>> @@ -50,7 +50,7 @@ OPTIONS\n>> \tMatch paths exactly (i.e. don't allow \"/foo/repo\" when the real path is\n>> \t\"/foo/repo.git\" or \"/foo/repo/.git\") and don't do user-relative paths.\n>> \t'git daemon' will refuse to start when this option is enabled and no\n>> -\twhitelist is specified.\n>> +\tspecific directories are specified.\n> \n> Structurally this series should be changed so that like changes are\n> coupled together, this would be much easier to review with the daemon.c\n> changes in 3/3.\n\nSure. That makes sense. The point here is that git-daemon's documentation\nand error messages currently make the word \"whitelist\" a critical point to\nunderstanding how the feature works. Instead, we can explain it more\nclearly using other language. Since this is the biggest place where such\nimportant is placed on the word, then making the changes isolated to this\ncommand makes sense.\n \n> But that also shows that this change is needed, but really lacking\n> compared to what we could do here, which is that both the the SYNOPSIS\n> and the heading here should be, respectively:\n> \n> \n> [--strict-paths=...]\n> \n> And:\n> \n> --strict-paths=...:\n> \n> I.e. all we're trying to get across here is \"this option has a mandatory\n> argument\", so let's just say something like that explicitly? I think in\n> this case we don't need the prose at all, the synopsis + heading + error\n> would be enough.\n\nThis example is misunderstanding that --strict-paths is a boolean\noption and changes how the list of \"undecorated\" arguments at the end\nis interpreted.\n\nThe point is that there is an optional list of directories given as\narguments, and the --strict-paths mode changes those directories to\nnot include recursive subdirectories as repo roots.\n>> \t`protocol.allow` is set to `never`, and each of the listed\n>> \tprotocols has `protocol..allow` set to `always`\n>> \t(overriding any existing configuration). In other words, any\n>> -\tprotocol not mentioned will be disallowed (i.e., this is a\n>> -\twhitelist, not a blacklist). See the description of\n>> +\tprotocol not mentioned will be disallowed. See the description of\n>> \t`protocol.allow` in linkgit:git-config[1] for more details.\n>> \n>> `GIT_PROTOCOL_FROM_USER`::\n> \n> I agree with Junio's earlier feedback about \"in other words\" being a\n> telltale sign of prose that needs improving.\n> \n> But the point of the previous prose (such as it was) was to elaborate on\n> th existing \"allow\" to say \"oh, allow means the same as whitelist\",\n> surely?\n> \n> So I think we really could just delete this \"in other words\" entirely,\n> i.e. it's basically saying \"you are allowed to eat ice cream (in other\n> words, you are not disallowed)\", it's not adding anything anymore. The\n> \"(...)\" can just be removed.\n\nI guess I stopped at the first level of \"in other words\", that being the\n\"i.e.\" parenthetical. I didn't realize that this was already nested inside\nan aside that was unnecessary.\n\nThanks,\n-Stolee\n"},{"id":"459369","messageId":"75cc0f43-d9a5-45d3-5e36-dd20acfebf50@github.com","threadId":"58152","inReplyTo":"220715.86o7xqzkt3.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH v2 2/3] t/*: avoid \"whitelist\"","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-07-19T15:09:35Z","receivedAt":"2022-07-19T15:09:41Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 7/15/2022 7:02 AM, Ævar Arnfjörð Bjarmason wrote:\n>> GIT_TEST_PASSING_SANITIZE_LEAK= when compiled with\n>> -SANITIZE=leak will run only those tests that have whitelisted\n>> -themselves as passing with no memory leaks. Tests can be whitelisted\n>> -by setting \"TEST_PASSES_SANITIZE_LEAK=true\" before sourcing\n>> -\"test-lib.sh\" itself at the top of the test script. This test mode is\n>> -used by the \"linux-leaks\" CI target.\n>> +SANITIZE=leak will run only those tests that have marked themselves as\n>> +passing with no memory leaks by setting \"TEST_PASSES_SANITIZE_LEAK=true\"\n>> +before sourcing \"test-lib.sh\" itself at the top of the test script. This\n>> +test mode is used by the \"linux-leaks\" CI target.\n> \n> It's hard to improve your own verbage, but I think in this case my\n> original version can be improved still:\n> \n> \tGIT_TEST_PASSING_SANITIZE_LEAK= when compiled with\n> \tSANITIZE=leak will, when true, only run those tests that declare\n> \tthemselves leak-free by setting \"TEST_PASSES_SANITIZE_LEAK=true\"\n> \tbefore sourcing \"test-lib.sh\". This test mode is used by the\n> \t\"linux-leaks\" CI target.\n\nAnother iteration:\n\n GIT_TEST_PASSING_SANITIZE_LEAK= focuses the test suite on finding\n memory leaks. When the variable is true and Git is compiled with\n SANITIZE=leak, only run those tests that declare themselves leak-free by\n setting \"TEST_PASSES_SANITIZE_LEAK=true\" before sourcing \"test-lib.sh\".\n This test mode is used by the \"linux-leaks\" CI target.\n \n>> -test_expect_success 'curl redirects respect whitelist' '\n>> +test_expect_success 'curl redirects respect allowed protocols' '\n> \n> Isn't the real problem here that this is inaccurate with regards to\n> \"curl\", i.e. AFAIK from browsing transport.c the whitelist of protocols\n> has nothing to do with curl, we parse that out and apply it before we\n> ever get to the specific transport layer.\n> \n> So this should just be \"http(s) transport respects GIT_ALLOW_PROTOCOL\",\n> no?\n\nSounds good.\n\n>> -test_description='test protocol whitelisting with submodules'\n>> +test_description='test protocol restrictions with submodules'\n> \n> Minor: I think this shows the awkwardness of using a word derived from\n> \"allow\". Before we could use \"whitelist\" and \"whitelisting\"\n> consistentlry, but now you have \"allowed\", \"allowlist\", \"restrictions\"\n> etc.\n> \n> I guess you could say \"test protocol allowances...\" or something? Meh.\n\nPerhaps \"filtering\" is the best way to describe the higher-level\nfeature that these lists help to implement.\n\nThanks,\n-Stolee\n"},{"id":"459374","messageId":"220719.86lespun6l.gmgdl@evledraar.gmail.com","threadId":"58152","inReplyTo":"75cc0f43-d9a5-45d3-5e36-dd20acfebf50@github.com","subject":"Re: [PATCH v2 2/3] t/*: avoid \"whitelist\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-07-19T15:26:45Z","receivedAt":"2022-07-19T15:39:36Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Jul 19 2022, Derrick Stolee wrote:\n\n> On 7/15/2022 7:02 AM, Ævar Arnfjörð Bjarmason wrote:\n>>> GIT_TEST_PASSING_SANITIZE_LEAK= when compiled with\n>>> -SANITIZE=leak will run only those tests that have whitelisted\n>>> -themselves as passing with no memory leaks. Tests can be whitelisted\n>>> -by setting \"TEST_PASSES_SANITIZE_LEAK=true\" before sourcing\n>>> -\"test-lib.sh\" itself at the top of the test script. This test mode is\n>>> -used by the \"linux-leaks\" CI target.\n>>> +SANITIZE=leak will run only those tests that have marked themselves as\n>>> +passing with no memory leaks by setting \"TEST_PASSES_SANITIZE_LEAK=true\"\n>>> +before sourcing \"test-lib.sh\" itself at the top of the test script. This\n>>> +test mode is used by the \"linux-leaks\" CI target.\n>> \n>> It's hard to improve your own verbage, but I think in this case my\n>> original version can be improved still:\n>> \n>> \tGIT_TEST_PASSING_SANITIZE_LEAK= when compiled with\n>> \tSANITIZE=leak will, when true, only run those tests that declare\n>> \tthemselves leak-free by setting \"TEST_PASSES_SANITIZE_LEAK=true\"\n>> \tbefore sourcing \"test-lib.sh\". This test mode is used by the\n>> \t\"linux-leaks\" CI target.\n>\n> Another iteration:\n>\n> GIT_TEST_PASSING_SANITIZE_LEAK= focuses the test suite on finding\n> memory leaks.\n\nIn some ways it does the opposite of \"focusing on finding memory leaks\",\nwe're explicitly finding fewer memory leaks, what we're doing is\nasserting that the tests we've whitelisted still pass.\n\n> SANITIZE=leak, only run those tests that declare themselves leak-free by\n> setting \"TEST_PASSES_SANITIZE_LEAK=true\" before sourcing \"test-lib.sh\".\n> This test mode is used by the \"linux-leaks\" CI target.\n\nPerhaps:\n\n\tGIT_TEST_PASSING_SANITIZE_LEAK=true skips those tests that\n\thaven't declared themselves as leak-free by setting\n\t\"TEST_PASSES_SANITIZE_LEAK=true\" before sourcing\n\t\"test-lib.sh\". This test mode is used by the \"linux-leaks\" CI\n\ttarget.\n\nAnyway, do you mind if this part is dropped from this series?\n\nI have a set of patches I was meaning to submit to add a tri-state\n[false,true,check] support for this, which this conflicts with.\n\n\"check\" being a mode where we check that the tests in \"false\" aren't yet\npassing. So for that I'll need to re-word this whole thing anyway. I'll\nrephrase this while I'm at it to something I think you'll be OK with.\n\n>>> -test_description='test protocol whitelisting with submodules'\n>>> +test_description='test protocol restrictions with submodules'\n>> \n>> Minor: I think this shows the awkwardness of using a word derived from\n>> \"allow\". Before we could use \"whitelist\" and \"whitelisting\"\n>> consistentlry, but now you have \"allowed\", \"allowlist\", \"restrictions\"\n>> etc.\n>> \n>> I guess you could say \"test protocol allowances...\" or something? Meh.\n>\n> Perhaps \"filtering\" is the best way to describe the higher-level\n> feature that these lists help to implement.\n\n*Shrug*. I think \"filtering\" more naturally refers to the process,\ni.e. in Makefile syntax:\n\n\t$(filter-out $(A),$(LIST))\n\t$(filter $(B),$(LIST))\n\nI'd call $(B) a whitelist (or whatever), $(A) a blacklist (or whatever),\nbut \"filtering\" the process of producing them.\n"},{"id":"459375","messageId":"0c2fc2be-3d63-8df1-2113-bb2229f6d05e@github.com","threadId":"58152","inReplyTo":"220719.86lespun6l.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH v2 2/3] t/*: avoid \"whitelist\"","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-07-19T15:42:55Z","receivedAt":"2022-07-19T15:43:00Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 7/19/2022 11:26 AM, Ævar Arnfjörð Bjarmason wrote:\n> Anyway, do you mind if this part is dropped from this series?\n> \n> I have a set of patches I was meaning to submit to add a tri-state\n> [false,true,check] support for this, which this conflicts with.\n> \n> \"check\" being a mode where we check that the tests in \"false\" aren't yet\n> passing. So for that I'll need to re-word this whole thing anyway. I'll\n> rephrase this while I'm at it to something I think you'll be OK with.\n\nI had just split the changes regarding SANITIZE=leak into its own\npatch, so it will be easy for me to drop it from my series. Thanks\nfor working on it.\n\nThanks,\n-Stolee\n"},{"id":"459406","messageId":"pull.1274.v3.git.1658255537.gitgitgadget@gmail.com","threadId":"58152","inReplyTo":"pull.1274.v2.git.1657852722.gitgitgadget@gmail.com","subject":"[PATCH v3 0/5] Remove use of \"whitelist\"","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-07-19T18:32:12Z","receivedAt":"2022-07-19T18:32:26Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"The word \"whitelist\" has cultural implications that make its use\nnon-inclusive.\n\nA previous version of this series recommended the replacement of\n\"allowlist\", but that term is still new and not accepted by some common\ndictionaries.\n\nInstead, this version avoids the use of \"whitelist\" by rewording the\nsentences that use it. In many cases, this improves readability since the\nterm is used suddenly without other context (and in some cases, is not\nnecessary at all).\n\nThere is one case where \"whitelist\" is replaced with \"allow_list\" but that\nis because we are operating on a string list parsed from the\nGIT_ALLOW_PROTOCOL environment variable.\n\n\nUpdates in v3\n=============\n\n * Reorganized patches so changes to git-daemon and git-cvsserver are\n bunched together across code, documentation, and tests. Their\n documentation is more thoroughly modified to clarify the role of the\n directory arguments.\n * The changes surrounding SANITIZE=leak are delayed for Ævar's work that\n will update the documentation while changing the behavior of the test\n variables.\n * Several replacements are made to clarify meaning instead of doing a\n simple replacement. Thanks, Ævar for the suggestions.\n\nThanks, -Stolee\n\nDerrick Stolee (5):\n daemon: clarify directory arguments\n git-cvsserver: clarify directory list\n git.txt: remove redundant language\n t: avoid \"whitelist\"\n transport.c: avoid \"whitelist\"\n\n Documentation/git-cvsserver.txt | 19 ++++++++++---------\n Documentation/git-daemon.txt | 21 +++++++++++----------\n Documentation/git.txt | 4 +---\n daemon.c | 8 ++++----\n git-cvsserver.perl | 2 +-\n t/lib-proto-disable.sh | 6 +++---\n t/t5812-proto-disable-http.sh | 2 +-\n t/t5815-submodule-protos.sh | 4 ++--\n t/t9400-git-cvsserver-server.sh | 2 +-\n t/test-lib-functions.sh | 3 +--\n transport.c | 8 ++++----\n 11 files changed, 39 insertions(+), 40 deletions(-)\n\n\nbase-commit: e4a4b31577c7419497ac30cebe30d755b97752c5\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1274%2Fderrickstolee%2Fallow-deny-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1274/derrickstolee/allow-deny-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/1274\n\nRange-diff vs v2:\n\n 1: 19632a2d245 ! 1: d5ca7bffac0 Documentation: remove use of whitelist\n @@ Metadata\n Author: Derrick Stolee \n \n ## Commit message ##\n - Documentation: remove use of whitelist\n + daemon: clarify directory arguments\n \n - The word \"whitelist\" has cultural implications that are not inclusive.\n - Thankfully, it is not difficult to reword and avoid its use.\n + The undecorated arguments to the 'git-daemon' command provide a list of\n + directories. When at least one directory is specified, then 'git-daemon'\n + only serves requests that are within that directory list. The boolean\n + '--strict-paths' option makes the list more explicit in that\n + subdirectories are no longer included.\n \n - Remove uses of \"whitelist\" in the Documentation/ directory. In all\n - cases, we can rewrite the sentences to add clarity instead of relying on\n - the reader understanding this term.\n + The existing documentation and error messages around this directory list\n + refer to it and its behavior as a \"whitelist\". The word \"whitelist\" has\n + cultural implications that are not inclusive. Thankfully, it is not\n + difficult to reword and avoid its use. In the process, we can define the\n + purpose of this directory list directly.\n \n - The most substantial change is to git-daemon.txt which had several uses,\n - but we can refer to the input directories as a list of included\n - directories, making the descriptions slightly simpler.\n + In Documentation/git-daemon.txt, rewrite the OPTIONS section around the\n + '' option. Add additional clarity to the other options that\n + refer to these directories.\n \n - Helped-by: Jeff King \n - Helped-by: Junio C Hamano \n - Signed-off-by: Derrick Stolee \n + Some error messages can also be improved in daemon.c. The\n + '--strict-paths' option requires '' arguments, so refer to\n + that section of the documentation directly. A logerror() call points out\n + that a requested directory is not in the specified directory list. We\n + can use \"list\" here without any loss of information.\n \n - ## Documentation/git-cvsserver.txt ##\n -@@ Documentation/git-cvsserver.txt: circumstances, allowing easier restricted usage through git-shell.\n - \n - GIT_CVSSERVER_BASE_PATH takes the place of the argument to --base-path.\n - \n --GIT_CVSSERVER_ROOT specifies a single-directory whitelist. The\n -+GIT_CVSSERVER_ROOT specifies a single allowed directory. The\n - repository must still be configured to allow access through\n - git-cvsserver, as described above.\n - \n + Signed-off-by: Derrick Stolee \n \n ## Documentation/git-daemon.txt ##\n @@ Documentation/git-daemon.txt: that service if it is enabled.\n @@ Documentation/git-daemon.txt: OPTIONS\n \t\"/foo/repo.git\" or \"/foo/repo/.git\") and don't do user-relative paths.\n \t'git daemon' will refuse to start when this option is enabled and no\n -\twhitelist is specified.\n -+\tspecific directories are specified.\n ++\tdirectory arguments are provided.\n \n --base-path=::\n \tRemap all the path requests as relative to the given path.\n @@ Documentation/git-daemon.txt: standard output to be sent to the requestor as an\n \n ::\n -\tA directory to add to the whitelist of allowed directories. Unless\n -+\tA directory to add to the list of allowed directories. Unless\n - \t--strict-paths is specified this will also include subdirectories\n - \tof each named directory.\n +-\t--strict-paths is specified this will also include subdirectories\n +-\tof each named directory.\n ++\tThe remaining arguments provide a list of directories. If any\n ++\tdirectories are specified, then the `git-daemon` process will\n ++\tserve a requested directory only if it is contained in one of\n ++\tthese directories. If `--strict-paths` is specified, then the\n ++\trequested directory must match one of these directories exactly.\n \n + SERVICES\n + --------\n @@ Documentation/git-daemon.txt: git\t\t9418/tcp\t\t# Git Version Control System\n \n 'git daemon' as inetd server::\n @@ Documentation/git-daemon.txt: git\t\t9418/tcp\t\t# Git Version Control System\n -\trepository under the whitelisted set of directories, /pub/foo\n -\tand /pub/bar, place an entry like the following into\n -\t/etc/inetd all on one line:\n -+\trepository in the directory list of `/pub/foo` and `/pub/bar`,\n -+\tplace an entry like the following into `/etc/inetd` all on one line:\n ++\trepository within `/pub/foo` or `/pub/bar`, place an entry like\n ++\tthe following into `/etc/inetd` all on one line:\n +\n ------------------------------------------------\n \tgit stream tcp nowait nobody /usr/bin/git\n \n - ## Documentation/git.txt ##\n -@@ Documentation/git.txt: for full details.\n - \t`protocol.allow` is set to `never`, and each of the listed\n - \tprotocols has `protocol..allow` set to `always`\n - \t(overriding any existing configuration). In other words, any\n --\tprotocol not mentioned will be disallowed (i.e., this is a\n --\twhitelist, not a blacklist). See the description of\n -+\tprotocol not mentioned will be disallowed. See the description of\n - \t`protocol.allow` in linkgit:git-config[1] for more details.\n + ## daemon.c ##\n +@@ daemon.c: static const char *path_ok(const char *directory, struct hostinfo *hi)\n + \t\t/* The validation is done on the paths after enter_repo\n + \t\t * appends optional {.git,.git/.git} and friends, but\n + \t\t * it does not use getcwd(). So if your /pub is\n +-\t\t * a symlink to /mnt/pub, you can whitelist /pub and\n ++\t\t * a symlink to /mnt/pub, you can include /pub and\n + \t\t * do not have to say /mnt/pub.\n + \t\t * Do not say /pub/.\n + \t\t */\n +@@ daemon.c: static const char *path_ok(const char *directory, struct hostinfo *hi)\n + \t\t\treturn path;\n + \t}\n + \n +-\tlogerror(\"'%s': not in whitelist\", path);\n ++\tlogerror(\"'%s': not in directory list\", path);\n + \treturn NULL;\t\t/* Fallthrough. Deny by default */\n + }\n + \n +@@ daemon.c: static int run_service(const char *dir, struct daemon_service *service,\n + \t * a \"git-daemon-export-ok\" flag that says that the other side\n + \t * is ok with us doing this.\n + \t *\n +-\t * path_ok() uses enter_repo() and does whitelist checking.\n ++\t * path_ok() uses enter_repo() and checks for included directories.\n + \t * We only need to make sure the repository is exported.\n + \t */\n + \n +@@ daemon.c: int cmd_main(int argc, const char **argv)\n + \t\tcred = prepare_credentials(user_name, group_name);\n + \n + \tif (strict_paths && (!ok_paths || !*ok_paths))\n +-\t\tdie(\"option --strict-paths requires a whitelist\");\n ++\t\tdie(\"option --strict-paths requires '' arguments\");\n \n - `GIT_PROTOCOL_FROM_USER`::\n + \tif (base_path && !is_directory(base_path))\n + \t\tdie(\"base-path '%s' does not exist or is not a directory\",\n -: ----------- > 2: db32444b879 git-cvsserver: clarify directory list\n -: ----------- > 3: c1c83c4284b git.txt: remove redundant language\n 2: 3c3c8c20bcb ! 4: e08342f07d2 t/*: avoid \"whitelist\"\n @@ Metadata\n Author: Derrick Stolee \n \n ## Commit message ##\n - t/*: avoid \"whitelist\"\n + t: avoid \"whitelist\"\n \n The word \"whitelist\" has cultural implications that are not inclusive.\n Thankfully, it is not difficult to reword and avoid its use.\n @@ Commit message\n \n Signed-off-by: Derrick Stolee \n \n - ## t/README ##\n -@@ t/README: GIT_TEST_SPLIT_INDEX= forces split-index mode on the whole\n - test suite. Accept any boolean values that are accepted by git-config.\n - \n - GIT_TEST_PASSING_SANITIZE_LEAK= when compiled with\n --SANITIZE=leak will run only those tests that have whitelisted\n --themselves as passing with no memory leaks. Tests can be whitelisted\n --by setting \"TEST_PASSES_SANITIZE_LEAK=true\" before sourcing\n --\"test-lib.sh\" itself at the top of the test script. This test mode is\n --used by the \"linux-leaks\" CI target.\n -+SANITIZE=leak will run only those tests that have marked themselves as\n -+passing with no memory leaks by setting \"TEST_PASSES_SANITIZE_LEAK=true\"\n -+before sourcing \"test-lib.sh\" itself at the top of the test script. This\n -+test mode is used by the \"linux-leaks\" CI target.\n - \n - GIT_TEST_PROTOCOL_VERSION=, when set, makes 'protocol.version'\n - default to n.\n -\n ## t/lib-proto-disable.sh ##\n @@\n # Test routines for checking protocol disabling.\n @@ t/t5812-proto-disable-http.sh: test_expect_success 'create git-accessible repo'\n test_proto \"smart http\" http \"$HTTPD_URL/smart/repo.git\"\n \n -test_expect_success 'curl redirects respect whitelist' '\n -+test_expect_success 'curl redirects respect allowed protocols' '\n ++test_expect_success 'http(s) transport respects GIT_ALLOW_PROTOCOL' '\n \ttest_must_fail env GIT_ALLOW_PROTOCOL=http:https \\\n \t\t\t GIT_SMART_HTTP=0 \\\n \t\tgit clone \"$HTTPD_URL/ftp-redir/repo.git\" 2>stderr &&\n @@ t/t5815-submodule-protos.sh\n #!/bin/sh\n \n -test_description='test protocol whitelisting with submodules'\n -+test_description='test protocol restrictions with submodules'\n ++test_description='test protocol filtering with submodules'\n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/lib-proto-disable.sh\n \n @@ t/t5815-submodule-protos.sh: test_expect_success 'update of ext not allowed' '\n '\n \n -test_expect_success 'user can override whitelist' '\n -+test_expect_success 'user can override with environment variable' '\n ++test_expect_success 'user can filter protocols with GIT_ALLOW_PROTOCOL' '\n \tGIT_ALLOW_PROTOCOL=ext git -C dst submodule update ext-module\n '\n \n \n - ## t/t9400-git-cvsserver-server.sh ##\n -@@ t/t9400-git-cvsserver-server.sh: test_expect_success 'req_Root (export-all)' \\\n - 'cat request-anonymous | git-cvsserver --export-all pserver \"$WORKDIR\" >log 2>&1 &&\n - sed -ne \\$p log | grep \"^I LOVE YOU\\$\"'\n - \n --test_expect_success 'req_Root failure (export-all w/o whitelist)' \\\n -+test_expect_success 'req_Root failure (export-all w/o directory)' \\\n - '! (cat request-anonymous | git-cvsserver --export-all pserver >log 2>&1 || false)'\n - \n - test_expect_success 'req_Root (everything together)' \\\n -\n ## t/test-lib-functions.sh ##\n @@ t/test-lib-functions.sh: test_set_prereq () {\n \t\t# test_unset_prereq()\n \t\t!*)\n \t\t\t;;\n -\t\t# (Temporary?) whitelist of things we can't easily\n -+\t\t# (Temporary?) list of things we can't easily\n - \t\t# pretend not to support\n +-\t\t# pretend not to support\n ++\t\t# List of things we can't easily pretend to not support\n \t\tSYMLINKS)\n \t\t\t;;\n -\n - ## t/test-lib.sh ##\n -@@ t/test-lib.sh: then\n - \ttest_done\n - fi\n - \n --# skip non-whitelisted tests when compiled with SANITIZE=leak\n -+# skip unmarked tests when compiled with SANITIZE=leak\n - if test -n \"$SANITIZE_LEAK\"\n - then\n - \tif test_bool_env GIT_TEST_PASSING_SANITIZE_LEAK false\n + \t\t# Inspecting whether GIT_TEST_FAIL_PREREQS is on\n 3: 0d862cbbebe ! 5: a4f31877b15 *: avoid \"whitelist\"\n @@ Metadata\n Author: Derrick Stolee \n \n ## Commit message ##\n - *: avoid \"whitelist\"\n + transport.c: avoid \"whitelist\"\n \n The word \"whitelist\" has cultural implications that are not inclusive.\n Thankfully, it is not difficult to reword and avoid its use.\n \n - A previous change already modified the documentation for git-cvsserver\n - and git-daemon to refer to a directory list. It is simple to update the\n - comments and error messages here to refer to that directory list.\n -\n - In the case of transport.c, the GIT_ALLOW_PROTOCOL environment variable\n - was referred to as a \"whitelist\", but the word \"allow\" is already part\n - of the variable. Replace \"whitelist\" with \"allow_list\" in these cases to\n - demonstrate that we are processing a list of allowed protocols.\n -\n - After this change, the only remaining uses of \"whitelist\" and its\n - companion \"blacklist\" are in release notes for older versions of Git and\n - in the sha1dc project, which is maintained independently.\n + The GIT_ALLOW_PROTOCOL environment variable was referred to as a\n + \"whitelist\", but the word \"allow\" is already part of the variable.\n + Replace \"whitelist\" with \"allow_list\" in these cases to demonstrate that\n + we are processing a list of allowed protocols.\n \n Signed-off-by: Derrick Stolee \n \n - ## daemon.c ##\n -@@ daemon.c: static const char *path_ok(const char *directory, struct hostinfo *hi)\n - \t\t/* The validation is done on the paths after enter_repo\n - \t\t * appends optional {.git,.git/.git} and friends, but\n - \t\t * it does not use getcwd(). So if your /pub is\n --\t\t * a symlink to /mnt/pub, you can whitelist /pub and\n -+\t\t * a symlink to /mnt/pub, you can include /pub and\n - \t\t * do not have to say /mnt/pub.\n - \t\t * Do not say /pub/.\n - \t\t */\n -@@ daemon.c: static const char *path_ok(const char *directory, struct hostinfo *hi)\n - \t\t\treturn path;\n - \t}\n - \n --\tlogerror(\"'%s': not in whitelist\", path);\n -+\tlogerror(\"'%s': not in directory list\", path);\n - \treturn NULL;\t\t/* Fallthrough. Deny by default */\n - }\n - \n -@@ daemon.c: static int run_service(const char *dir, struct daemon_service *service,\n - \t * a \"git-daemon-export-ok\" flag that says that the other side\n - \t * is ok with us doing this.\n - \t *\n --\t * path_ok() uses enter_repo() and does whitelist checking.\n -+\t * path_ok() uses enter_repo() and checks for included directories.\n - \t * We only need to make sure the repository is exported.\n - \t */\n - \n -@@ daemon.c: int cmd_main(int argc, const char **argv)\n - \t\tcred = prepare_credentials(user_name, group_name);\n - \n - \tif (strict_paths && (!ok_paths || !*ok_paths))\n --\t\tdie(\"option --strict-paths requires a whitelist\");\n -+\t\tdie(\"option --strict-paths requires a directory list\");\n - \n - \tif (base_path && !is_directory(base_path))\n - \t\tdie(\"base-path '%s' does not exist or is not a directory\",\n -\n - ## git-cvsserver.perl ##\n -@@ git-cvsserver.perl: $state->{allowed_roots} = [ @ARGV ];\n - \n - # don't export the whole system unless the users requests it\n - if ($state->{'export-all'} && !@{$state->{allowed_roots}}) {\n -- die \"--export-all can only be used together with an explicit whitelist\\n\";\n -+ die \"--export-all can only be used together with an explicit directory list\\n\";\n - }\n - \n - # Environment handling for running under git-shell\n -\n ## transport.c ##\n @@ transport.c: static int external_specification_len(const char *url)\n \treturn strchr(url, ':') - url;\n\n-- \ngitgitgadget\n"},{"id":"459407","messageId":"d5ca7bffac0679823c78228e6d06ea9545dc4325.1658255537.git.gitgitgadget@gmail.com","threadId":"58152","inReplyTo":"pull.1274.v3.git.1658255537.gitgitgadget@gmail.com","subject":"[PATCH v3 1/5] daemon: clarify directory arguments","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-07-19T18:32:13Z","receivedAt":"2022-07-19T18:32:27Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThe undecorated arguments to the 'git-daemon' command provide a list of\ndirectories. When at least one directory is specified, then 'git-daemon'\nonly serves requests that are within that directory list. The boolean\n'--strict-paths' option makes the list more explicit in that\nsubdirectories are no longer included.\n\nThe existing documentation and error messages around this directory list\nrefer to it and its behavior as a \"whitelist\". The word \"whitelist\" has\ncultural implications that are not inclusive. Thankfully, it is not\ndifficult to reword and avoid its use. In the process, we can define the\npurpose of this directory list directly.\n\nIn Documentation/git-daemon.txt, rewrite the OPTIONS section around the\n'' option. Add additional clarity to the other options that\nrefer to these directories.\n\nSome error messages can also be improved in daemon.c. The\n'--strict-paths' option requires '' arguments, so refer to\nthat section of the documentation directly. A logerror() call points out\nthat a requested directory is not in the specified directory list. We\ncan use \"list\" here without any loss of information.\n\nSigned-off-by: Derrick Stolee \n---\n Documentation/git-daemon.txt | 21 +++++++++++----------\n daemon.c | 8 ++++----\n 2 files changed, 15 insertions(+), 14 deletions(-)\n\ndiff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt\nindex fdc28c041c7..236df516c73 100644\n--- a/Documentation/git-daemon.txt\n+++ b/Documentation/git-daemon.txt\n@@ -32,8 +32,8 @@ that service if it is enabled.\n It verifies that the directory has the magic file \"git-daemon-export-ok\", and\n it will refuse to export any Git directory that hasn't explicitly been marked\n for export this way (unless the `--export-all` parameter is specified). If you\n-pass some directory paths as 'git daemon' arguments, you can further restrict\n-the offers to a whitelist comprising of those.\n+pass some directory paths as 'git daemon' arguments, the offers are limited to\n+repositories within those directories.\n \n By default, only `upload-pack` service is enabled, which serves\n 'git fetch-pack' and 'git ls-remote' clients, which are invoked\n@@ -50,7 +50,7 @@ OPTIONS\n \tMatch paths exactly (i.e. don't allow \"/foo/repo\" when the real path is\n \t\"/foo/repo.git\" or \"/foo/repo/.git\") and don't do user-relative paths.\n \t'git daemon' will refuse to start when this option is enabled and no\n-\twhitelist is specified.\n+\tdirectory arguments are provided.\n \n --base-path=::\n \tRemap all the path requests as relative to the given path.\n@@ -73,7 +73,7 @@ OPTIONS\n \t%IP for the server's IP address, %P for the port number,\n \tand %D for the absolute path of the named repository.\n \tAfter interpolation, the path is validated against the directory\n-\twhitelist.\n+\tlist.\n \n --export-all::\n \tAllow pulling from all directories that look like Git repositories\n@@ -218,9 +218,11 @@ standard output to be sent to the requestor as an error message when\n it declines the service.\n \n ::\n-\tA directory to add to the whitelist of allowed directories. Unless\n-\t--strict-paths is specified this will also include subdirectories\n-\tof each named directory.\n+\tThe remaining arguments provide a list of directories. If any\n+\tdirectories are specified, then the `git-daemon` process will\n+\tserve a requested directory only if it is contained in one of\n+\tthese directories. If `--strict-paths` is specified, then the\n+\trequested directory must match one of these directories exactly.\n \n SERVICES\n --------\n@@ -264,9 +266,8 @@ git\t\t9418/tcp\t\t# Git Version Control System\n \n 'git daemon' as inetd server::\n \tTo set up 'git daemon' as an inetd service that handles any\n-\trepository under the whitelisted set of directories, /pub/foo\n-\tand /pub/bar, place an entry like the following into\n-\t/etc/inetd all on one line:\n+\trepository within `/pub/foo` or `/pub/bar`, place an entry like\n+\tthe following into `/etc/inetd` all on one line:\n +\n ------------------------------------------------\n \tgit stream tcp nowait nobody /usr/bin/git\ndiff --git a/daemon.c b/daemon.c\nindex 58f1077885c..0ae7d12b5c1 100644\n--- a/daemon.c\n+++ b/daemon.c\n@@ -279,7 +279,7 @@ static const char *path_ok(const char *directory, struct hostinfo *hi)\n \t\t/* The validation is done on the paths after enter_repo\n \t\t * appends optional {.git,.git/.git} and friends, but\n \t\t * it does not use getcwd(). So if your /pub is\n-\t\t * a symlink to /mnt/pub, you can whitelist /pub and\n+\t\t * a symlink to /mnt/pub, you can include /pub and\n \t\t * do not have to say /mnt/pub.\n \t\t * Do not say /pub/.\n \t\t */\n@@ -298,7 +298,7 @@ static const char *path_ok(const char *directory, struct hostinfo *hi)\n \t\t\treturn path;\n \t}\n \n-\tlogerror(\"'%s': not in whitelist\", path);\n+\tlogerror(\"'%s': not in directory list\", path);\n \treturn NULL;\t\t/* Fallthrough. Deny by default */\n }\n \n@@ -403,7 +403,7 @@ static int run_service(const char *dir, struct daemon_service *service,\n \t * a \"git-daemon-export-ok\" flag that says that the other side\n \t * is ok with us doing this.\n \t *\n-\t * path_ok() uses enter_repo() and does whitelist checking.\n+\t * path_ok() uses enter_repo() and checks for included directories.\n \t * We only need to make sure the repository is exported.\n \t */\n \n@@ -1444,7 +1444,7 @@ int cmd_main(int argc, const char **argv)\n \t\tcred = prepare_credentials(user_name, group_name);\n \n \tif (strict_paths && (!ok_paths || !*ok_paths))\n-\t\tdie(\"option --strict-paths requires a whitelist\");\n+\t\tdie(\"option --strict-paths requires '' arguments\");\n \n \tif (base_path && !is_directory(base_path))\n \t\tdie(\"base-path '%s' does not exist or is not a directory\",\n-- \ngitgitgadget\n\n"},{"id":"459408","messageId":"db32444b87917089aaef7d501cc0e2a7fa969d97.1658255537.git.gitgitgadget@gmail.com","threadId":"58152","inReplyTo":"pull.1274.v3.git.1658255537.gitgitgadget@gmail.com","subject":"[PATCH v3 2/5] git-cvsserver: clarify directory list","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-07-19T18:32:14Z","receivedAt":"2022-07-19T18:32:27Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThe documentation and error messages for git-cvsserver include some\nreferences to a \"whitelist\" that is not otherwise included in the\ndocumentation. When different parts of the documentation do not use\ncommon language, this can lead to confusion as to how things are meant\nto operate.\n\nFurther, the word \"whitelist\" has cultural implications that make its\nuse non-inclusive. Thankfully, we can remove it while increasing\nclarity.\n\nUpdate Documentation/git-cvsserver.txt in a similar way to the previous\nchange to Documentation/git-daemon.txt. The optional '...'\nlist can specify a list of allowed directories. We refer to that list\ndirectly inside of the documentation for the GIT_CVSSERVER_ROOT\nenvironment variable.\n\nWhile modifying this documentation, update the environment variables to\nuse a list format. We use the modern way of tabbing the description of\neach variable in this section. We do _not_ update the description of\n'...' to use tabs this way since the rest of the items in the\nOPTIONS list do not use this modern formatting.\n\nA single error message in the actual git-cvsserver.perl code refers to\nthe whitelist during argument parsing. Instead, refer to the directory\nlist that has been clarified in the documentation.\n\nSigned-off-by: Derrick Stolee \n---\n Documentation/git-cvsserver.txt | 19 ++++++++++---------\n git-cvsserver.perl | 2 +-\n t/t9400-git-cvsserver-server.sh | 2 +-\n 3 files changed, 12 insertions(+), 11 deletions(-)\n\ndiff --git a/Documentation/git-cvsserver.txt b/Documentation/git-cvsserver.txt\nindex 4dc57ed2547..53f111bc0ac 100644\n--- a/Documentation/git-cvsserver.txt\n+++ b/Documentation/git-cvsserver.txt\n@@ -63,11 +63,10 @@ Print version information and exit\n Print usage information and exit\n \n ::\n-You can specify a list of allowed directories. If no directories\n-are given, all are allowed. This is an additional restriction, gitcvs\n-access still needs to be enabled by the `gitcvs.enabled` config option\n-unless `--export-all` was given, too.\n-\n+The remaining arguments provide a list of directories. If no directories\n+are given, then all are allowed. Repositories within these directories\n+still require the `gitcvs.enabled` config option, unless `--export-all`\n+is specified.\n \n LIMITATIONS\n -----------\n@@ -311,11 +310,13 @@ ENVIRONMENT\n These variables obviate the need for command-line options in some\n circumstances, allowing easier restricted usage through git-shell.\n \n-GIT_CVSSERVER_BASE_PATH takes the place of the argument to --base-path.\n+GIT_CVSSERVER_BASE_PATH::\n+\tThis variable replaces the argument to --base-path.\n \n-GIT_CVSSERVER_ROOT specifies a single-directory whitelist. The\n-repository must still be configured to allow access through\n-git-cvsserver, as described above.\n+GIT_CVSSERVER_ROOT::\n+\tThis variable specifies a single directory, replacing the\n+\t`...` argument list. The repository still requires the\n+\t`gitcvs.enabled` config option, unless `--export-all` is specified.\n \n When these environment variables are set, the corresponding\n command-line arguments may not be used.\ndiff --git a/git-cvsserver.perl b/git-cvsserver.perl\nindex 4c8118010a8..7b757360e28 100755\n--- a/git-cvsserver.perl\n+++ b/git-cvsserver.perl\n@@ -152,7 +152,7 @@ $state->{allowed_roots} = [ @ARGV ];\n \n # don't export the whole system unless the users requests it\n if ($state->{'export-all'} && !@{$state->{allowed_roots}}) {\n- die \"--export-all can only be used together with an explicit whitelist\\n\";\n+ die \"--export-all can only be used together with an explicit '...' list\\n\";\n }\n \n # Environment handling for running under git-shell\ndiff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh\nindex 210ddf09e30..379b19f2f85 100755\n--- a/t/t9400-git-cvsserver-server.sh\n+++ b/t/t9400-git-cvsserver-server.sh\n@@ -221,7 +221,7 @@ test_expect_success 'req_Root (export-all)' \\\n 'cat request-anonymous | git-cvsserver --export-all pserver \"$WORKDIR\" >log 2>&1 &&\n sed -ne \\$p log | grep \"^I LOVE YOU\\$\"'\n \n-test_expect_success 'req_Root failure (export-all w/o whitelist)' \\\n+test_expect_success 'req_Root failure (export-all w/o directory list)' \\\n '! (cat request-anonymous | git-cvsserver --export-all pserver >log 2>&1 || false)'\n \n test_expect_success 'req_Root (everything together)' \\\n-- \ngitgitgadget\n\n"},{"id":"459409","messageId":"c1c83c4284ba4b041694a521c3639f33561ac5e3.1658255537.git.gitgitgadget@gmail.com","threadId":"58152","inReplyTo":"pull.1274.v3.git.1658255537.gitgitgadget@gmail.com","subject":"[PATCH v3 3/5] git.txt: remove redundant language","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-07-19T18:32:15Z","receivedAt":"2022-07-19T18:32:28Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThe documentation for GIT_ALLOW_PROTOCOL has a sentence that adds no\nvalue, since it repeats the meaning from the previous sentence (twice!).\n\nThe word \"whitelist\" has cultural implications that are not inclusive,\nwhich brought attention to this sentence.\n\nHelped-by: Jeff King \nHelped-by: Junio C Hamano \nSigned-off-by: Derrick Stolee \n---\n Documentation/git.txt | 4 +---\n 1 file changed, 1 insertion(+), 3 deletions(-)\n\ndiff --git a/Documentation/git.txt b/Documentation/git.txt\nindex 302607a4967..47a6095ff40 100644\n--- a/Documentation/git.txt\n+++ b/Documentation/git.txt\n@@ -885,9 +885,7 @@ for full details.\n \tIf set to a colon-separated list of protocols, behave as if\n \t`protocol.allow` is set to `never`, and each of the listed\n \tprotocols has `protocol..allow` set to `always`\n-\t(overriding any existing configuration). In other words, any\n-\tprotocol not mentioned will be disallowed (i.e., this is a\n-\twhitelist, not a blacklist). See the description of\n+\t(overriding any existing configuration). See the description of\n \t`protocol.allow` in linkgit:git-config[1] for more details.\n \n `GIT_PROTOCOL_FROM_USER`::\n-- \ngitgitgadget\n\n"},{"id":"459410","messageId":"e08342f07d2d5268208f637ce229210a1d99f83b.1658255537.git.gitgitgadget@gmail.com","threadId":"58152","inReplyTo":"pull.1274.v3.git.1658255537.gitgitgadget@gmail.com","subject":"[PATCH v3 4/5] t: avoid \"whitelist\"","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-07-19T18:32:16Z","receivedAt":"2022-07-19T18:32:30Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThe word \"whitelist\" has cultural implications that are not inclusive.\nThankfully, it is not difficult to reword and avoid its use.\n\nFocus on changes in the test scripts, since most of the changes are in\ncomments and test names. The renamed test_allow_var helper is only used\nonce inside the widely-used test_proto helper.\n\nSigned-off-by: Derrick Stolee \n---\n t/lib-proto-disable.sh | 6 +++---\n t/t5812-proto-disable-http.sh | 2 +-\n t/t5815-submodule-protos.sh | 4 ++--\n t/test-lib-functions.sh | 3 +--\n 4 files changed, 7 insertions(+), 8 deletions(-)\n\ndiff --git a/t/lib-proto-disable.sh b/t/lib-proto-disable.sh\nindex 83babe57d95..890622be816 100644\n--- a/t/lib-proto-disable.sh\n+++ b/t/lib-proto-disable.sh\n@@ -1,7 +1,7 @@\n # Test routines for checking protocol disabling.\n \n-# Test clone/fetch/push with GIT_ALLOW_PROTOCOL whitelist\n-test_whitelist () {\n+# Test clone/fetch/push with GIT_ALLOW_PROTOCOL environment variable\n+test_allow_var () {\n \tdesc=$1\n \tproto=$2\n \turl=$3\n@@ -183,7 +183,7 @@ test_config () {\n # $2 - machine-readable name of the protocol\n # $3 - the URL to try cloning\n test_proto () {\n-\ttest_whitelist \"$@\"\n+\ttest_allow_var \"$@\"\n \n \ttest_config \"$@\"\n }\ndiff --git a/t/t5812-proto-disable-http.sh b/t/t5812-proto-disable-http.sh\nindex af8772fadaa..d8da5f58d16 100755\n--- a/t/t5812-proto-disable-http.sh\n+++ b/t/t5812-proto-disable-http.sh\n@@ -16,7 +16,7 @@ test_expect_success 'create git-accessible repo' '\n \n test_proto \"smart http\" http \"$HTTPD_URL/smart/repo.git\"\n \n-test_expect_success 'curl redirects respect whitelist' '\n+test_expect_success 'http(s) transport respects GIT_ALLOW_PROTOCOL' '\n \ttest_must_fail env GIT_ALLOW_PROTOCOL=http:https \\\n \t\t\t GIT_SMART_HTTP=0 \\\n \t\tgit clone \"$HTTPD_URL/ftp-redir/repo.git\" 2>stderr &&\ndiff --git a/t/t5815-submodule-protos.sh b/t/t5815-submodule-protos.sh\nindex 06f55a1b8a0..4d5956cc18f 100755\n--- a/t/t5815-submodule-protos.sh\n+++ b/t/t5815-submodule-protos.sh\n@@ -1,6 +1,6 @@\n #!/bin/sh\n \n-test_description='test protocol whitelisting with submodules'\n+test_description='test protocol filtering with submodules'\n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/lib-proto-disable.sh\n \n@@ -36,7 +36,7 @@ test_expect_success 'update of ext not allowed' '\n \ttest_must_fail git -C dst submodule update ext-module\n '\n \n-test_expect_success 'user can override whitelist' '\n+test_expect_success 'user can filter protocols with GIT_ALLOW_PROTOCOL' '\n \tGIT_ALLOW_PROTOCOL=ext git -C dst submodule update ext-module\n '\n \ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex 6da7273f1d5..8c44856eaec 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -651,8 +651,7 @@ test_set_prereq () {\n \t\t# test_unset_prereq()\n \t\t!*)\n \t\t\t;;\n-\t\t# (Temporary?) whitelist of things we can't easily\n-\t\t# pretend not to support\n+\t\t# List of things we can't easily pretend to not support\n \t\tSYMLINKS)\n \t\t\t;;\n \t\t# Inspecting whether GIT_TEST_FAIL_PREREQS is on\n-- \ngitgitgadget\n\n"},{"id":"459411","messageId":"a4f31877b15e46413cb62378577d95886720b020.1658255537.git.gitgitgadget@gmail.com","threadId":"58152","inReplyTo":"pull.1274.v3.git.1658255537.gitgitgadget@gmail.com","subject":"[PATCH v3 5/5] transport.c: avoid \"whitelist\"","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-07-19T18:32:17Z","receivedAt":"2022-07-19T18:32:45Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThe word \"whitelist\" has cultural implications that are not inclusive.\nThankfully, it is not difficult to reword and avoid its use.\n\nThe GIT_ALLOW_PROTOCOL environment variable was referred to as a\n\"whitelist\", but the word \"allow\" is already part of the variable.\nReplace \"whitelist\" with \"allow_list\" in these cases to demonstrate that\nwe are processing a list of allowed protocols.\n\nSigned-off-by: Derrick Stolee \n---\n transport.c | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/transport.c b/transport.c\nindex 52db7a3cb09..b51e991e443 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -940,7 +940,7 @@ static int external_specification_len(const char *url)\n \treturn strchr(url, ':') - url;\n }\n \n-static const struct string_list *protocol_whitelist(void)\n+static const struct string_list *protocol_allow_list(void)\n {\n \tstatic int enabled = -1;\n \tstatic struct string_list allowed = STRING_LIST_INIT_DUP;\n@@ -1020,9 +1020,9 @@ static enum protocol_allow_config get_protocol_config(const char *type)\n \n int is_transport_allowed(const char *type, int from_user)\n {\n-\tconst struct string_list *whitelist = protocol_whitelist();\n-\tif (whitelist)\n-\t\treturn string_list_has_string(whitelist, type);\n+\tconst struct string_list *allow_list = protocol_allow_list();\n+\tif (allow_list)\n+\t\treturn string_list_has_string(allow_list, type);\n \n \tswitch (get_protocol_config(type)) {\n \tcase PROTOCOL_ALLOW_ALWAYS:\n-- \ngitgitgadget\n"},{"id":"459428","messageId":"xmqqwnc8anvh.fsf@gitster.g","threadId":"58152","inReplyTo":"75cc0f43-d9a5-45d3-5e36-dd20acfebf50@github.com","subject":"Re: [PATCH v2 2/3] t/*: avoid \"whitelist\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-07-19T19:44:34Z","receivedAt":"2022-07-19T19:44:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Derrick Stolee writes:\n\n>> \tGIT_TEST_PASSING_SANITIZE_LEAK= when compiled with\n>> \tSANITIZE=leak will, when true, only run those tests that declare\n>> \tthemselves leak-free by setting \"TEST_PASSES_SANITIZE_LEAK=true\"\n>> \tbefore sourcing \"test-lib.sh\". This test mode is used by the\n>> \t\"linux-leaks\" CI target.\n>\n> Another iteration:\n>\n> GIT_TEST_PASSING_SANITIZE_LEAK= focuses the test suite on finding\n> memory leaks. When the variable is true and Git is compiled with\n> SANITIZE=leak, only run those tests that declare themselves leak-free by\n> setting \"TEST_PASSES_SANITIZE_LEAK=true\" before sourcing \"test-lib.sh\".\n> This test mode is used by the \"linux-leaks\" CI target.\n\nSounds good. These scripts opt into the sanitize-leak tests by\ndeclaring themselves to be leak-free and that is captured very well\nwithout using the *list word ;-)\n"},{"id":"460320","messageId":"YuXOPpww39EsOna4@coredump.intra.peff.net","threadId":"58152","inReplyTo":"c1c83c4284ba4b041694a521c3639f33561ac5e3.1658255537.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v3 3/5] git.txt: remove redundant language","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-07-31T00:35:10Z","receivedAt":"2022-07-31T00:35:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jul 19, 2022 at 06:32:15PM +0000, Derrick Stolee via GitGitGadget wrote:\n\n> From: Derrick Stolee \n> \n> The documentation for GIT_ALLOW_PROTOCOL has a sentence that adds no\n> value, since it repeats the meaning from the previous sentence (twice!).\n\nI think the point the original was trying to make was emphasizing that\nwhen the variable is set at all, then we operate in this new mode.\n\nMaybe that's sufficiently conveyed in the first sentence, but perhaps\nanother way of expanding would be:\n\n If the variable is unset, it has no effect, and the normal\n configuration is respected.\n\nBut perhaps that is obvious enough from the previous sentence. Since I\nwas involved in drafting the original, I'm not sure I'm a good judge of\nwhat is obvious and what is not. :)\n\n-Peff\n"}]}