{"thread":{"id":"4586","subject":"upload-pack is unsafe in corrupt repository","startedAt":"2006-06-20T12:12:25Z","lastAt":"2006-06-20T12:12:25Z","messageCount":1,"participants":["Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"22133","messageId":"7vac8881qu.fsf@assigned-by-dhcp.cox.net","threadId":"4586","inReplyTo":null,"subject":"upload-pack is unsafe in corrupt repository","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-06-20T12:12:25Z","receivedAt":"2006-06-20T12:12:25Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"I was helping Andrew fetching from maintainer trees.  One of the\ntrees were corrupt (i.e. git-fsck-objects unclean) and there was\na rather grave consequences we found out the hard way (and it is\n5am but Tuesday is not my git day X-<).\n\nupload-pack forks a pipeline, whose upstream execs rev-list and\ndownstream execs pack-objects.  rev-list correctly notices that\nit cannot reach some objects and dies, but pack-objects just\nsays \"Ah, the list of object input just ended -- let's send them\nall\".  This results in git-fetch-pack to be fed an incomplete\npack.  However, git-unpack-objects is happy (the pack is not\ncorrupt, just that it does not contain what the protocol\nexchange expected to send) and git-fetch-pack ends up going all\nthe way through updating the ref.\n\nEek.\n\nAt this moment, I am just reporting this because I am too tired\nto code it myself right now, but I think the fix is to make\nupload-pack to fork once more, waitpid() for rev-list and\ninject/append garbage in the stream pack-objects produces, in\norder to cause unpack-objects (or index-pack) that runs on the\nthe fetching end to notice, when rev-list or pack-objects barf.\n\nOf course a nicer fix is to add the sideband we have been\ndiscussing about, but that relies on protocol extension and\nwould not solve the problem for older clients.\n\nSide note: in my head, the sideband would work by the \"forked\nonce more\" upload-pack process to listen to both stdout and\nstderr of pack-objects, and feed them packetized (instead of the\ncurrent \"go-straight-stream\") by perhaps prefixing them with one\nbyte (is this a payload, or is this a progress-bar/keepalive type\npacket).  Of course, this would change the way the payload is\ntransferred, so the initial handshake needs to negotiate if the\nsideband should be used just like we do other new features like\nthin pack transfers.\n"}]}