From: Junio C Hamano <gitster@pobox.com>
To: git@vger.kernel.org
Subject: [PATCH v4 1/4] safe.directory: preliminary clean-up
Date: Tue, 30 Jul 2024 11:43:49 -0700 [thread overview]
Message-ID: <20240730184352.2503276-2-gitster@pobox.com> (raw)
In-Reply-To: <20240730184352.2503276-1-gitster@pobox.com>
The paths given in the safe.directory configuration variable are
allowed to contain "~user" (which interpolates to user's home
directory) and "%(prefix)" (which interpolates to the installation
location in RUNTIME_PREFIX-enabled builds, and a call to the
git_config_pathname() function is tasked to obtain a copy of the
path with these constructs interpolated.
The function, when it succeeds, always yields an allocated string in
the location given as the out-parameter; even when there is nothing
to interpolate in the original, a literal copy is made. The code
path that contains this caller somehow made two contradicting and
incorrect assumptions of the behaviour when there is no need for
interpolation, and was written with extra defensiveness against
two phantom risks that do not exist.
One wrong assumption was that the function might yield NULL when
there is no interpolation. This led to the use of an extra "check"
variable, conditionally holding either the interpolated or the
original string. The assumption was with us since 8959555c
(setup_git_directory(): add an owner check for the top-level
directory, 2022-03-02) originally introduced the safe.directory
feature.
Another wrong assumption was that the function might yield the same
pointer as the input when there is no interpolation. This led to a
conditional free'ing of the interpolated copy, that the conditional
never skipped, as we always received an allocated string.
Simplify the code by removing the extra defensiveness.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
setup.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/setup.c b/setup.c
index d458edcc02..3177d010d1 100644
--- a/setup.c
+++ b/setup.c
@@ -1235,17 +1235,15 @@ static int safe_directory_cb(const char *key, const char *value,
char *allowed = NULL;
if (!git_config_pathname(&allowed, key, value)) {
- const char *check = allowed ? allowed : value;
- if (ends_with(check, "/*")) {
- size_t len = strlen(check);
- if (!fspathncmp(check, data->path, len - 1))
+ if (ends_with(allowed, "/*")) {
+ size_t len = strlen(allowed);
+ if (!fspathncmp(allowed, data->path, len - 1))
data->is_safe = 1;
- } else if (!fspathcmp(data->path, check)) {
+ } else if (!fspathcmp(data->path, allowed)) {
data->is_safe = 1;
}
- }
- if (allowed != value)
free(allowed);
+ }
}
return 0;
--
2.46.0-77-g633c50689c
next prev parent reply other threads:[~2024-07-30 18:44 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-07-20 22:09 [PATCH 0/2] safe.directory clean-up Junio C Hamano
2024-07-20 22:09 ` [PATCH 1/2] safe.directory: normalize the checked path Junio C Hamano
2024-07-20 22:09 ` [PATCH 2/2] safe.directory: normalize the configured path Junio C Hamano
2024-07-20 22:09 ` [PATCH 3/2] setup: use a single return path in setup_git_directory*() Junio C Hamano
2024-07-20 22:09 ` [PATCH 4/2] setup: cache normalized safe.directory configuration Junio C Hamano
2024-07-23 2:18 ` [PATCH v2 0/3] safe.directory clean-up Junio C Hamano
2024-07-23 2:18 ` [PATCH v2 1/3] safe.directory: normalize the checked path Junio C Hamano
2024-07-23 2:18 ` [PATCH v2 2/3] safe.directory: normalize the configured path Junio C Hamano
2024-07-25 9:45 ` Phillip Wood
2024-07-25 16:11 ` Junio C Hamano
2024-08-14 13:20 ` Phillip Wood
2024-08-14 17:15 ` Junio C Hamano
2024-08-15 9:51 ` Phillip Wood
2024-08-15 14:43 ` Junio C Hamano
2024-07-26 5:02 ` Jeff King
2024-07-26 15:02 ` Junio C Hamano
2024-07-27 22:05 ` Jeff King
2024-07-23 2:19 ` [PATCH v2 3/3] safe.directory: setting safe.directory="." allows the "current" directory Junio C Hamano
2024-07-25 9:45 ` Phillip Wood
2024-07-25 16:12 ` Junio C Hamano
2024-07-25 9:45 ` [PATCH v2 0/3] safe.directory clean-up Phillip Wood
2024-07-25 16:14 ` Junio C Hamano
2024-07-30 1:10 ` [PATCH v3 " Junio C Hamano
2024-07-30 1:10 ` [PATCH v3 1/3] safe.directory: normalize the checked path Junio C Hamano
2024-07-30 1:10 ` [PATCH v3 2/3] safe.directory: normalize the configured path Junio C Hamano
2024-07-30 7:31 ` Jeff King
2024-07-30 16:03 ` Junio C Hamano
2024-07-30 20:08 ` Jeff King
2024-07-30 7:43 ` Jeff King
2024-07-30 16:22 ` Junio C Hamano
2024-07-30 17:56 ` safe.directory: preliminary clean-up Junio C Hamano
2024-07-30 20:13 ` Jeff King
2024-07-30 20:10 ` [PATCH v3 2/3] safe.directory: normalize the configured path Jeff King
2024-07-30 1:10 ` [PATCH v3 3/3] safe.directory: setting safe.directory="." allows the "current" directory Junio C Hamano
2024-07-30 18:43 ` [PATCH v4 0/4] safe.directory clean-up Junio C Hamano
2024-07-30 18:43 ` Junio C Hamano [this message]
2024-07-30 18:43 ` [PATCH v4 2/4] safe.directory: normalize the checked path Junio C Hamano
2024-07-30 18:43 ` [PATCH v4 3/4] safe.directory: normalize the configured path Junio C Hamano
2024-07-30 18:43 ` [PATCH v4 4/4] safe.directory: setting safe.directory="." allows the "current" directory Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240730184352.2503276-2-gitster@pobox.com \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).