{"thread":{"id":"50521","subject":"2.21: Change in ref advertisement handling?","startedAt":"2019-02-16T02:45:55Z","lastAt":"2019-02-16T06:45:56Z","messageCount":2,"participants":["Bryan Turner","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"369426","messageId":"CAGyf7-FC1q1f4F36_hS53sdP3wCxd7fTgbegSj+tPS4idvF+Hg@mail.gmail.com","threadId":"50521","inReplyTo":null,"subject":"2.21: Change in ref advertisement handling?","fromName":"Bryan Turner","fromEmail":"bturner@atlassian.com","sentAt":"2019-02-16T02:45:42Z","receivedAt":"2019-02-16T02:45:55Z","isPatch":false,"sender":{"key":"bturner@atlassian.com","avatar":"https://gravatar.com/avatar/16bcf3167981c1ef7c804e502642366d888a35b0d0b0a4ca01fdc442aa1acb1e?d=mp&s=160"},"body":"In Bitbucket Server's Git test suite, we have some lazy tests that use\na stub HTTP server with mocked payloads to feed a Git client, to test\nits behavior.\n\nFor some of those tests, we have the server configured to send a\nresponse body like this for a ref advertisement:\n76d28b849be7fb3301e4bd14fb28328947b4f5a1<tab>HEAD<newline>\n76d28b849be7fb3301e4bd14fb28328947b4f5a1<tab>refs/heads/master<newline>\n\nThere are headers we set, like \"Content-Type:\napplication/x-git-upload-pack-advertisement\", but that's the response.\nIt's not packeted or anything.\n\nIf I use (for example) Git 2.17.1 and run \"git ls-remote <url>\" with\nthat response, Git happily displays essentially identical output, HEAD\nand master at 76d28b849be7fb3301e4bd14fb28328947b4f5a1.\n\nIf I run that \"git ls-remote\" with 2.21.0-rc1, Git displays \"fatal:\nthe remote end hung up unexpectedly\" and dies.\n\nIf I change the mocked response to:\n001e# service=git-upload-pack<newline>\n0000<no newline; wrapping just for display>\n003276d28b849be7fb3301e4bd14fb28328947b4f5a1<sp>HEAD<newline>\n003f76d28b849be7fb3301e4bd14fb28328947b4f5a1<sp>refs/heads/master<newline>\n0000\n\nThis works in both 2.17.1 and 2.21.0-rc1. (I was a little surprised\nGit didn't care about the lack of a NUL byte after \"HEAD\" with the\ncapabilities after it, before the newline, but even 2.21 doesn't seem\nto mind.)\n\nFirst let me say, I had no idea these tests of ours existed, and I'm\nsurprised that they worked on Git 2.20 and older, given that the\nresponse isn't in proper pkt-line format.\n\nI'm not looking for anything to change here, let me hasten to add; I'm\njust interested in some clarification. Why did our invalid ref\nadvertisement response work in older Git versions? Why has it stopped\nworking in 2.21?\n\nBest regards,\nBryan Turner\n"},{"id":"369431","messageId":"20190216064553.GA27641@sigill.intra.peff.net","threadId":"50521","inReplyTo":"CAGyf7-FC1q1f4F36_hS53sdP3wCxd7fTgbegSj+tPS4idvF+Hg@mail.gmail.com","subject":"Re: 2.21: Change in ref advertisement handling?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-02-16T06:45:53Z","receivedAt":"2019-02-16T06:45:56Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 15, 2019 at 06:45:42PM -0800, Bryan Turner wrote:\n\n> In Bitbucket Server's Git test suite, we have some lazy tests that use\n> a stub HTTP server with mocked payloads to feed a Git client, to test\n> its behavior.\n> \n> For some of those tests, we have the server configured to send a\n> response body like this for a ref advertisement:\n> 76d28b849be7fb3301e4bd14fb28328947b4f5a1<tab>HEAD<newline>\n> 76d28b849be7fb3301e4bd14fb28328947b4f5a1<tab>refs/heads/master<newline>\n> \n> There are headers we set, like \"Content-Type:\n> application/x-git-upload-pack-advertisement\", but that's the response.\n> It's not packeted or anything.\n>\n> [...]\n>\n> First let me say, I had no idea these tests of ours existed, and I'm\n> surprised that they worked on Git 2.20 and older, given that the\n> response isn't in proper pkt-line format.\n\nI believe older versions of Git would have fallen back to dumb-http\nhere. Do your tests actually follow-up with a real object fetch, or are\nthey just listing the refs? If the latter, I think it would \"work\" even\nif your server does not otherwise implement dumb-http.\n\n> I'm not looking for anything to change here, let me hasten to add; I'm\n> just interested in some clarification. Why did our invalid ref\n> advertisement response work in older Git versions? Why has it stopped\n> working in 2.21?\n\nThis is almost certainly due to my 8ee3e120cd (remote-curl: refactor\nsmart-http discovery, 2019-02-06), which intentionally tightened a few\ncorner cases to match the spec. In particular, once you respond with\n\"application/x-git-upload-pack-advertisement\", we fully commit to\nsmart-http and complain if you further violate the spec, rather than\nfalling back to dumb-http.\n\n-Peff\n"}]}