Re: [PATCH 3/3] fetch: die on invalid --negotiation-tip hash
- From
Emily Shaffer <emilyshaffer@google.com>
- Date
- Jul 13, 2021, 22:36 UTC
- Message-ID
- <YO4Vc58rl2Ble31u@google.com>
- In-Reply-To
- <8df607f4cfb3064200bf4effc91726c92804e637.1624486920.git.jonathantanmy@google.com>
On Wed, Jun 23, 2021 at 03:30:53PM -0700, Jonathan Tan wrote:
Show 8 quoted lines
> > 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.
Show 19 quoted lines
>
> Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
> ---
> 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.
Show 15 quoted lines
> 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.
Show 9 quoted lines
> + 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