Re: [PATCH 3/3] fetch: die on invalid --negotiation-tip hash
flat view
From: Emily Shaffer <hidden>
Date: 2021-07-13 22:36:42
On Wed, Jun 23, 2021 at 03:30:53PM -0700, Jonathan Tan wrote:
If a full hexadecimal hash is given as a --negotiation-tip to "git fetch", and that hash does not correspond to an object, "git fetch" will segfault if --negotiate-only is given and will silently ignore that hash otherwise. Make these cases fatal errors, just like the case when an invalid ref name or abbreviated hash is given. While at it, mark the error messages as translatable.
I don't usually like this kind of "while we're at it" change, but in
this case it's very very small, so it doesn't bother me much.quoted hunk ↗ jump to hunk
Signed-off-by: Jonathan Tan <redacted> --- builtin/fetch.c | 4 +++- t/t5510-fetch.sh | 9 +++++++++ 2 files changed, 12 insertions(+), 1 deletion(-)diff --git a/builtin/fetch.c b/builtin/fetch.c index 9191620e50..2c50465cff 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c@@ -1428,7 +1428,9 @@ static void add_negotiation_tips(struct git_transport_options *smart_options) if (!has_glob_specials(s)) { struct object_id oid; if (get_oid(s, &oid)) - die("%s is not a valid object", s); + die(_("%s is not a valid object"), s); + if (!has_object(the_repository, &oid, 0)) + die(_("%s is not a valid object"), s);
Any reason not to consolidate these, e.g. if (get_oid || !has_object)? Then we wouldn't dup the err string.
quoted hunk ↗ jump to hunk
oid_array_append(oids, &oid); continue; }diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh index e83b2a6506..5fc5750d8d 100755 --- a/t/t5510-fetch.sh +++ b/t/t5510-fetch.sh@@ -1214,6 +1214,15 @@ test_expect_success '--negotiation-tip understands abbreviated SHA-1' ' check_negotiation_tip ' +test_expect_success '--negotiation-tip rejects missing OIDs' ' + setup_negotiation_tip server server 0 && + test_must_fail git -C client fetch \ + --negotiation-tip=alpha_1 \
This one's okay...
+ --negotiation-tip=$(test_oid zero) \
...and this one's junk. Ok.
+ origin alpha_s beta_s 2>err && + test_i18ngrep "is not a valid object" err +' + . "$TEST_DIRECTORY"/lib-httpd.sh start_httpd -- 2.32.0.288.g62a8d224e6-goog
Other than the little nit, this seems OK to me. - Emily