git/upload-pack.c, branch v2.30.2

Merge branch 'tb/partial-clone-filters-fix'

2020-12-17T23:06:40Z

Fix potential server side resource deallocation issues when responding to a partial clone request. * tb/partial-clone-filters-fix: upload-pack.c: don't free allowed_filters util pointers builtin/clone.c: don't ignore transport_fetch_refs() errors

Merge branch 'jk/multi-line-indent-style-fix'

2020-12-14T18:21:38Z

Style fix. * jk/multi-line-indent-style-fix: style: indent multiline "if" conditions to align

Merge branch 'jk/check-config-parsing-error-in-upload-pack'

2020-12-14T18:21:37Z

Tighten error checking in the codepath that responds to "git fetch". * jk/check-config-parsing-error-in-upload-pack: upload-pack: propagate return value from object filter config callback

Merge branch 'js/trace2-session-id'

2020-12-08T23:11:20Z

The transport layer was taught to optionally exchange the session ID assigned by the trace2 subsystem during fetch/push transactions. * js/trace2-session-id: receive-pack: log received client session ID send-pack: advertise session ID in capabilities upload-pack, serve: log received client session ID fetch-pack: advertise session ID in capabilities transport: log received server session ID serve: advertise session ID in v2 capabilities receive-pack: advertise session ID in v0 capabilities upload-pack: advertise session ID in v0 capabilities trace2: add a public function for getting the SID docs: new transfer.advertiseSID option docs: new capability to advertise session IDs

upload-pack.c: don't free allowed_filters util pointers

2020-12-03T20:42:33Z

To keep track of which object filters are allowed or not, 'git upload-pack' stores the name of each filter in a string_list, and sets it ->util pointer to be either 0 or 1, indicating whether it is banned or allowed. Later on, we attempt to clear that list, but we incorrectly ask for the util pointers to be free()'d, too. This behavior (introduced back in 6dd3456a8c (upload-pack.c: allow banning certain object filter(s), 2020-08-03)) leads to an invalid free, and causes us to crash. In order to trigger this, one needs to fetch from a server that (a) has at least one object filter allowed, and (b) issue a fetch that contains a subset of the allowed filters (i.e., we cannot ask for a banned filter, since this causes us to die() before we hit the bogus string_list_clear()). In that case, whatever banned filters exist will cause a noop free() (since those ->util pointers are set to 0), but the first allowed filter we try to free will crash us. We never noticed this in the tests because we didn't have an example of setting 'uploadPackFilter' configuration variables and then following up with a valid fetch. The first new 'git clone' prevents further regression here. For good measure on top, add a test which checks the same behavior at a tree depth greater than 0. Signed-off-by: Taylor Blau Signed-off-by: Junio C Hamano

style: indent multiline "if" conditions to align

2020-12-03T18:32:32Z

Commit 6dc905d974 (config: split repo scope to local and worktree, 2020-02-10) made some "if" statements multiline, but didn't indent the second lines in our usual way. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

upload-pack: propagate return value from object filter config callback

2020-12-03T18:25:13Z

If we encounter an error in parse_filter_object_config(), we'll complain to stderr but won't actually propagate the return value up the stack. This is unlike most of our config callbacks, which return the error to git_config() so it can die (this includes the call just below us to parse_hide_refs_config(), which can also produce errors). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

Merge branch 'jk/stop-pack-objects-when-fetch-is-killed'

2020-12-03T08:18:07Z

"git fetch" that is killed may leave a pack-objects process behind, still computing to find a good compression, wasting cycles. This has been corrected. * jk/stop-pack-objects-when-fetch-is-killed: upload-pack: kill pack-objects helper on signal or exit

upload-pack: kill pack-objects helper on signal or exit

2020-12-01T20:05:58Z

We spawn an external pack-objects process to actually send objects to the remote side. If we are killed by a signal during this process, then pack-objects may continue to run. As soon as it starts producing output for the pack, it will see a failure writing to upload-pack and exit itself. But before then, it may do significant work traversing the object graph, compressing deltas, etc, which will all be pointless. So let's make sure to kill as soon as we know that the caller will not read the result. There's no test here, since it's inherently racy, but here's an easy reproduction is on a large-ish repo like linux.git: - make sure you don't have pack bitmaps (since they make the enumerating phase go quickly). For linux.git it takes ~30s or so to walk the whole graph on my machine. - run "git clone --no-local -q . dst"; the "-q" is important because if pack-objects is writing progress to upload-pack (to get multiplexed over the sideband to the client), then it will notice pretty quickly the failure to write to stderr - kill the client-side clone process in another terminal (don't use ^C, as that will send SIGINT to all of the processes) - run "ps au | grep git" or similar to observe upload-pack dying within 5 seconds (it will send a keepalive that will notice the client has gone away) - but you'll still see pack-objects consuming 100% CPU (and 1GB+ of RAM) during the traversal and delta compression phases. It will exit as soon as it starts to write the pack (when it will notice that upload-pack went away). With this patch, pack-objects exits as soon as upload-pack does. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

upload-pack, serve: log received client session ID

2020-11-12T02:26:53Z

When upload-pack (protocol v0/v1) or a protocol v2 server receives a session-id capability from a client, log the received session ID via a trace2 data event. Signed-off-by: Josh Steadmon Signed-off-by: Junio C Hamano