Skip to content

upload-pack.c: fix partial clone allowed filter regression

Taylor Blau git 1 version In review

Get series with b4
b4 am [email protected]

Versions newest first

  1. v1 Latest2 patches 1 replyIn review

Patches in v1 No applies recorded yet

#Subject and filesSizeRepliesApplied
1/2builtin/clone.c: don't ignore transport_fetch_refs() errorsbuiltin/clone.c+11 -40 replies
2/2upload-pack.c: don't free allowed_filters util pointerst/t5616-partial-clone.sh upload-pack.c+10 -21 reply

Reviews trailers per patch

None of the 2 patches has a Reviewed-by or Acked-by.

PatchJeff King
1/2Co-authored-by
2/2

Trailers given in replies are filled; those carried in the patches from earlier versions are outlined with dashes.

Read the v1 thread

Cover letter [PATCH 0/2] upload-pack.c: fix partial clone allowed filter regression

Hi,

Here's a regression fix that I noticed this morning while deploying 2.28 out to GitHub. The gist is that we accidentally call 'string_list_clear(..., 1)' when freeing the list of allowed/banned object filters, but the ->util pointer either contains '0', or '1'. So, the free() of the util pointer is bogus, and we end up crashing.

The fix in the second patch is straightforward, but it doesn't work without the first patch, which teaches 'git clone' to match 'git fetch' and not ignore an error from 'transport_fetch_refs()'.

Unfortunately, this regression has been in Git since 2.28. Fortunately, nobody has seemed to notice, so I doubt that it's bitten anybody out in the wild. But, it's still worth fixing, since it is so obviously broken.

Thanks,
Taylor

Taylor Blau (2):
builtin/clone.c: don't ignore transport_fetch_refs() errors upload-pack.c: don't free allowed_filters util pointers

 builtin/clone.c          | 15 +++++++++++----
 t/t5616-partial-clone.sh | 10 +++++++++-
 upload-pack.c            |  2 +-
 3 files changed, 21 insertions(+), 6 deletions(-)

--
2.29.2.533.g07db1f5344