{"thread":{"id":"28567","subject":"Git ksshaskpass to play nice with https and kwallet","startedAt":"2011-10-04T10:19:59Z","lastAt":"2011-10-14T18:59:04Z","messageCount":30,"participants":["Michael J Gruber","Jeff King","Junio C Hamano","Tay Ray Chuan"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"176829","messageId":"4E8ADDCF.6090406@drmicha.warpmail.net","threadId":"28567","inReplyTo":null,"subject":"Git ksshaskpass to play nice with https and kwallet","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-10-04T10:19:59Z","receivedAt":"2011-10-04T10:19:59Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Checking out bitbucket and https, I came across a stupid issue between\nthe default KDE setup (SSH_ASKPASS=/usr/bin/ksshakspass and KDE wallet)\nand the way we call the askpass helper, which makes the combo not work\nout of the box.\n\nFor those interested in getting it to work (or learning why it does not\nwork out of the box), you can read the details formatted on my sparse\nblog (brain dump)\n\nhttp://grubix.blogspot.com/2011/10/git-ksshaskpass-to-play-nice-with-https.html\n\nor below if you don't feel like clicking through.\n\nCheers\nMichael\n\nBy default (with GIT_ASKPASS not set etc.), Git invokes $SSH_ASKPASS\nwhen it needs to ask you for credential info. In a KDE environment,\nSSH_ASKPASS is set to /usr/bin/ksshaskpass. So far so good.\n\nBut Git calls the askpass helper with a command line like\n/usr/bin/ksshaskpass Username for 'bitbucket.org':\nand once again with\n/usr/bin/ksshaskpass Password for 'bitbucket.org':\nSo far so good.\n\nBut when asked to store the credentials in the KDE wallet, ksshaskpass\ntries (too) hard to guess a good key from that line. And for both\ninvocations, it comes up with the same key (the URL), so that when the\npassword info is needed, the username info from the wallet is returned.\nAuthentication fails.\nFar from good.\n\nIf you still want to use the KDE helpers, you can save the little snippet\n#!/bin/bash\nset $*\nexec ksshaskpass $1\\@$3\ninto ~/bin/git-ksshaskpass (and make it executable) and set\n\ngit config --global core.askpass ~/bin/git-ksshaskpass\nGitcha!\n\nNB:\n\n    tested and works with https\n    not tested with other methods (use ssh-agent)\n    new credential helper interface coming in Git after 1.7.7\n"},{"id":"176835","messageId":"20111004105008.GA11789@sigill.intra.peff.net","threadId":"28567","inReplyTo":"4E8ADDCF.6090406@drmicha.warpmail.net","subject":"Re: Git ksshaskpass to play nice with https and kwallet","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-10-04T10:50:08Z","receivedAt":"2011-10-04T10:50:08Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Oct 04, 2011 at 12:19:59PM +0200, Michael J Gruber wrote:\n\n> But Git calls the askpass helper with a command line like\n> /usr/bin/ksshaskpass Username for 'bitbucket.org':\n> and once again with\n> /usr/bin/ksshaskpass Password for 'bitbucket.org':\n> So far so good.\n> \n> But when asked to store the credentials in the KDE wallet, ksshaskpass\n> tries (too) hard to guess a good key from that line. And for both\n> invocations, it comes up with the same key (the URL), so that when the\n> password info is needed, the username info from the wallet is returned.\n> Authentication fails.\n> Far from good.\n\nNeat. I didn't know ksshaskpass would do that. I wondered for a minute\nif all of the credential helper stuff could have gone through the\naskpass interface. But I don't think so.\n\nOne problem is that the askpass interface only lets us ask for one thing\nat a time. So even with your clever hack, it will end up storing two\nseparate keys: Username@host and Password@host. But it has no idea\nthey're connected. So if you store \"user1 / pass1\", then try to push to\n\"user2@host\", we would silently use the password for user1.\n\nOn top of that, there isn't much contextual information. I guess they\nassumed the guessing would be used for \"ssh\". But it means that a stored\nssh password could potentially be used for git, and vice versa. I guess\nyou could get around that by making the host field longer and more\ndescriptive (i.e., a full url).\n\n-Peff\n"},{"id":"176839","messageId":"4E8AEDBD.4070404@drmicha.warpmail.net","threadId":"28567","inReplyTo":"20111004105008.GA11789@sigill.intra.peff.net","subject":"Re: Git ksshaskpass to play nice with https and kwallet","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-10-04T11:27:57Z","receivedAt":"2011-10-04T11:27:57Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Jeff King venit, vidit, dixit 04.10.2011 12:50:\n> On Tue, Oct 04, 2011 at 12:19:59PM +0200, Michael J Gruber wrote:\n> \n>> But Git calls the askpass helper with a command line like\n>> /usr/bin/ksshaskpass Username for 'bitbucket.org':\n>> and once again with\n>> /usr/bin/ksshaskpass Password for 'bitbucket.org':\n>> So far so good.\n>>\n>> But when asked to store the credentials in the KDE wallet, ksshaskpass\n>> tries (too) hard to guess a good key from that line. And for both\n>> invocations, it comes up with the same key (the URL), so that when the\n>> password info is needed, the username info from the wallet is returned.\n>> Authentication fails.\n>> Far from good.\n> \n> Neat. I didn't know ksshaskpass would do that. I wondered for a minute\n> if all of the credential helper stuff could have gone through the\n> askpass interface. But I don't think so.\n\nDon't worry ;)\n\n> One problem is that the askpass interface only lets us ask for one thing\n> at a time. So even with your clever hack, it will end up storing two\n> separate keys: Username@host and Password@host. But it has no idea\n> they're connected. So if you store \"user1 / pass1\", then try to push to\n> \"user2@host\", we would silently use the password for user1.\n> \n> On top of that, there isn't much contextual information. I guess they\n> assumed the guessing would be used for \"ssh\". But it means that a stored\n> ssh password could potentially be used for git, and vice versa. I guess\n> you could get around that by making the host field longer and more\n> descriptive (i.e., a full url).\n\nI think it's really meant for ssh keys only, where the keyid identifies\nthe key uniquely.\n\nStill, ksshaskpass's trying to guess a unique key from the prompt text\nseems quite hackish to me. But many people will have a Git without\ncredential-helpers, and a KDE default setup, so hope my post helps\nsomeone besides myself.\n\nNote that git-credentials-askpass would have a fair chance of doing\nbetter: credential_askpass() knows the username and could pass it to\ncredential_ask_one(), e.g. by amending the description field, or setting\nthe first field to \"Password for user %(user)\". Do you think that would\nbe worth deviating from the default behavior (i.e. compared to no helper)?\n\nCheers,\nMichael\n"},{"id":"176841","messageId":"20111004113713.GA19171@sigill.intra.peff.net","threadId":"28567","inReplyTo":"4E8AEDBD.4070404@drmicha.warpmail.net","subject":"Re: Git ksshaskpass to play nice with https and kwallet","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-10-04T11:37:13Z","receivedAt":"2011-10-04T11:37:13Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Oct 04, 2011 at 01:27:57PM +0200, Michael J Gruber wrote:\n\n> Still, ksshaskpass's trying to guess a unique key from the prompt text\n> seems quite hackish to me. But many people will have a Git without\n> credential-helpers, and a KDE default setup, so hope my post helps\n> someone besides myself.\n\nHmm. I don't think that pre-credential-helper git actually puts the\nhostname in the prompt, though. It just says \"Username:\". So your trick\nwouldn't work then, would it?\n\n> Note that git-credentials-askpass would have a fair chance of doing\n> better: credential_askpass() knows the username and could pass it to\n> credential_ask_one(), e.g. by amending the description field, or setting\n> the first field to \"Password for user %(user)\". Do you think that would\n> be worth deviating from the default behavior (i.e. compared to no helper)?\n\nI think that git should do that by default. v1.7.7 (and earlier) does:\n\n  $ git push https://example.com/foo.git\n  Username:\n  Password:\n\nWith my patches in 'next', it does:\n\n  $ git push https://example.com/foo.git\n  Username for 'example.com':\n  Password for 'example.com':\n\nBut it would probably be better to say:\n\n  $ git push https://example.com/foo.git\n  Username for 'example.com':\n  Password for 'user@example.com':\n\nThe latter is especially useful if you have put a username in your\n~/.gitconfig, in which case you get:\n\n  $ git push https://example.com/foo.git\n  Password for 'user@example.com':\n\nwhich is a nice reminder. And it would happen to work with your askpass\nmagic (I also wonder if it should mention the protocol and the repo, but\nmost of the time that isn't relevant, and it does make the prompt harder\nto read).\n\n-Peff\n"},{"id":"176842","messageId":"4E8AF812.5090906@drmicha.warpmail.net","threadId":"28567","inReplyTo":"20111004113713.GA19171@sigill.intra.peff.net","subject":"Re: Git ksshaskpass to play nice with https and kwallet","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-10-04T12:12:02Z","receivedAt":"2011-10-04T12:12:02Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Jeff King venit, vidit, dixit 04.10.2011 13:37:\n> On Tue, Oct 04, 2011 at 01:27:57PM +0200, Michael J Gruber wrote:\n> \n>> Still, ksshaskpass's trying to guess a unique key from the prompt text\n>> seems quite hackish to me. But many people will have a Git without\n>> credential-helpers, and a KDE default setup, so hope my post helps\n>> someone besides myself.\n> \n> Hmm. I don't think that pre-credential-helper git actually puts the\n> hostname in the prompt, though. It just says \"Username:\". So your trick\n> wouldn't work then, would it?\n> \n>> Note that git-credentials-askpass would have a fair chance of doing\n>> better: credential_askpass() knows the username and could pass it to\n>> credential_ask_one(), e.g. by amending the description field, or setting\n>> the first field to \"Password for user %(user)\". Do you think that would\n>> be worth deviating from the default behavior (i.e. compared to no helper)?\n> \n> I think that git should do that by default. v1.7.7 (and earlier) does:\n> \n>   $ git push https://example.com/foo.git\n>   Username:\n>   Password:\n> \n> With my patches in 'next', it does:\n> \n>   $ git push https://example.com/foo.git\n>   Username for 'example.com':\n>   Password for 'example.com':\n>\n\nSheesh. I'm too used to using next(+) to even think of that! You're\ncompletely right: My trick only works with next's additions.\n\n> But it would probably be better to say:\n> \n>   $ git push https://example.com/foo.git\n>   Username for 'example.com':\n>   Password for 'user@example.com':\n\nYes, exactly. credential_askpass() knows what it needs for that.\n\n> The latter is especially useful if you have put a username in your\n> ~/.gitconfig, in which case you get:\n\nI'm actually wondering why git can't infer the user from\n\nhttps://user@host.com\n\nwith last week's next, at least.\n> \n>   $ git push https://example.com/foo.git\n>   Password for 'user@example.com':\n> \n> which is a nice reminder. And it would happen to work with your askpass\n> magic (I also wonder if it should mention the protocol and the repo, but\n> most of the time that isn't relevant, and it does make the prompt harder\n> to read).\n\nWith the above, I can probably do without any magic: 'example.com' would\nbe the wallet key for the username (if I let the wallet store it) and\n'user@example.com' the key for the password, whether the username comes\nfrom the wallet or from the config. (Again, why not from the URL?)\n\nMichael\n"},{"id":"176844","messageId":"20111004124344.GA30162@sigill.intra.peff.net","threadId":"28567","inReplyTo":"4E8AF812.5090906@drmicha.warpmail.net","subject":"Re: Git ksshaskpass to play nice with https and kwallet","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-10-04T12:43:44Z","receivedAt":"2011-10-04T12:43:44Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Oct 04, 2011 at 02:12:02PM +0200, Michael J Gruber wrote:\n\n> > The latter is especially useful if you have put a username in your\n> > ~/.gitconfig, in which case you get:\n> \n> I'm actually wondering why git can't infer the user from\n> \n> https://user@host.com\n> \n> with last week's next, at least.\n\nIt can, and it has for some time. Part of the configurable-username\nthing was that it would be way nicer to just use a user-agnostic URL,\nbecause it means it's easier to share with other people.\n\n> >   $ git push https://example.com/foo.git\n> >   Password for 'user@example.com':\n> > \n> > which is a nice reminder. And it would happen to work with your askpass\n> > magic (I also wonder if it should mention the protocol and the repo, but\n> > most of the time that isn't relevant, and it does make the prompt harder\n> > to read).\n> \n> With the above, I can probably do without any magic: 'example.com' would\n> be the wallet key for the username (if I let the wallet store it) and\n> 'user@example.com' the key for the password, whether the username comes\n> from the wallet or from the config. (Again, why not from the URL?)\n\nYeah, sorry, I should have said \"ksshaskpass's magic\". :)\n\nAnd yes, it can come from the URL. Mentioning the user in the password\nprompt is not as useful a reminder if it comes from:\n\n  $ git push https://user@example.com/foo.git\n\nbut, if it's something like:\n\n  $ git clone https://user@example.com/foo.git\n  [months pass]\n\n  $ git push\n  Password for 'user@example.com':\n\nthen it's a nice reminder.\n\n-Peff\n"},{"id":"176862","messageId":"4E8B5553.2080706@drmicha.warpmail.net","threadId":"28567","inReplyTo":"20111004124344.GA30162@sigill.intra.peff.net","subject":"Re: Git ksshaskpass to play nice with https and kwallet","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-10-04T18:49:55Z","receivedAt":"2011-10-04T18:49:55Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Jeff King venit, vidit, dixit 04.10.2011 14:43:\n> On Tue, Oct 04, 2011 at 02:12:02PM +0200, Michael J Gruber wrote:\n> \n>>> The latter is especially useful if you have put a username in your\n>>> ~/.gitconfig, in which case you get:\n>>\n>> I'm actually wondering why git can't infer the user from\n>>\n>> https://user@host.com\n>>\n>> with last week's next, at least.\n> \n> It can, and it has for some time. Part of the configurable-username\n> thing was that it would be way nicer to just use a user-agnostic URL,\n> because it means it's easier to share with other people.\n> \n\nWe seem to mean something different:\n\ngit config --get remote.bitbucket.pushurl\nhttps://grubix@bitbucket.org/grubix/git.git\nSSH_ASKPASS= git push -n bitbucket\nUsername for 'bitbucket.org':\n\nI mean that git should not need to ask for the username here.\n\nMichael\n"},{"id":"176946","messageId":"20111005175536.GB22962@sigill.intra.peff.net","threadId":"28567","inReplyTo":"4E8B5553.2080706@drmicha.warpmail.net","subject":"Re: Git ksshaskpass to play nice with https and kwallet","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-10-05T17:55:36Z","receivedAt":"2011-10-05T17:55:36Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Oct 04, 2011 at 08:49:55PM +0200, Michael J Gruber wrote:\n\n> We seem to mean something different:\n> \n> git config --get remote.bitbucket.pushurl\n> https://grubix@bitbucket.org/grubix/git.git\n> SSH_ASKPASS= git push -n bitbucket\n> Username for 'bitbucket.org':\n> \n> I mean that git should not need to ask for the username here.\n\nNo, we are in agreement about the intended behavior. I think you are\nseeing a bug. What version of git produced it?\n\nWith my http-auth series, I get:\n\n  $ git push https://github.com/peff/git.git\n  Username for 'github.com':\n\n  $ git push https://peff@github.com/peff/git.git\n  Password for 'github.com':\n\nUsing v1.7.7 produces similar results.\n\n-Peff\n"},{"id":"176948","messageId":"20111005180125.GC22962@sigill.intra.peff.net","threadId":"28567","inReplyTo":"20111005175536.GB22962@sigill.intra.peff.net","subject":"Re: Git ksshaskpass to play nice with https and kwallet","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-10-05T18:01:25Z","receivedAt":"2011-10-05T18:01:25Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Oct 05, 2011 at 01:55:36PM -0400, Jeff King wrote:\n\n> On Tue, Oct 04, 2011 at 08:49:55PM +0200, Michael J Gruber wrote:\n> \n> > We seem to mean something different:\n> > \n> > git config --get remote.bitbucket.pushurl\n> > https://grubix@bitbucket.org/grubix/git.git\n> > SSH_ASKPASS= git push -n bitbucket\n> > Username for 'bitbucket.org':\n> > \n> > I mean that git should not need to ask for the username here.\n> \n> No, we are in agreement about the intended behavior. I think you are\n> seeing a bug. What version of git produced it?\n> \n> With my http-auth series, I get:\n> \n>   $ git push https://github.com/peff/git.git\n>   Username for 'github.com':\n> \n>   $ git push https://peff@github.com/peff/git.git\n>   Password for 'github.com':\n> \n> Using v1.7.7 produces similar results.\n\nHrm. I do get this, with the same version of git:\n\n  $ git config remote.foo.url https://github.com/peff/git.git\n  $ git push foo\n  Username for 'github.com':\n\n  $ git config remote.foo.url https://peff@github.com/peff/git.git\n  $ git push foo\n  Password for 'github.com':\n\nSo far so good. Now how about this:\n\n  $ git config remote.foo.url https://github.com/peff/git.git\n  $ git config remote.foo.pushurl https://peff@github.com/peff/git.git\n  $ git push foo\n  Username for 'github.com':\n\nSo I think the problem is with pushurl, not with the auth code. Oddly,\nthough, running GIT_TRACE reveals:\n\n  $ GIT_TRACE=1 git push foo\n  trace: built-in: git 'push' 'foo'\n  trace: run_command: 'git-remote-https' 'foo' 'https://peff@github.com/peff/git.git'\n\nwhich is the right URL. So it's almost as if we are throwing away the\npassed URL in favor of the configuration, and then looking up the\nconfiguration wrong. I'm about to go get on a plane, so I don't have\nmore time to look at it now, but I suspect it's something simple and\nstupid.\n\n-Peff\n"},{"id":"177005","messageId":"4E8D4BD5.2090202@drmicha.warpmail.net","threadId":"28567","inReplyTo":"20111005180125.GC22962@sigill.intra.peff.net","subject":"Re: Git ksshaskpass to play nice with https and kwallet","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-10-06T06:33:57Z","receivedAt":"2011-10-06T06:33:57Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Jeff King venit, vidit, dixit 05.10.2011 20:01:\n> On Wed, Oct 05, 2011 at 01:55:36PM -0400, Jeff King wrote:\n> \n>> On Tue, Oct 04, 2011 at 08:49:55PM +0200, Michael J Gruber wrote:\n>>\n>>> We seem to mean something different:\n>>>\n>>> git config --get remote.bitbucket.pushurl\n>>> https://grubix@bitbucket.org/grubix/git.git\n>>> SSH_ASKPASS= git push -n bitbucket\n>>> Username for 'bitbucket.org':\n>>>\n>>> I mean that git should not need to ask for the username here.\n>>\n>> No, we are in agreement about the intended behavior. I think you are\n>> seeing a bug. What version of git produced it?\n>>\n>> With my http-auth series, I get:\n>>\n>>   $ git push https://github.com/peff/git.git\n>>   Username for 'github.com':\n>>\n>>   $ git push https://peff@github.com/peff/git.git\n>>   Password for 'github.com':\n>>\n>> Using v1.7.7 produces similar results.\n> \n> Hrm. I do get this, with the same version of git:\n> \n>   $ git config remote.foo.url https://github.com/peff/git.git\n>   $ git push foo\n>   Username for 'github.com':\n> \n>   $ git config remote.foo.url https://peff@github.com/peff/git.git\n>   $ git push foo\n>   Password for 'github.com':\n> \n> So far so good. Now how about this:\n> \n>   $ git config remote.foo.url https://github.com/peff/git.git\n>   $ git config remote.foo.pushurl https://peff@github.com/peff/git.git\n>   $ git push foo\n>   Username for 'github.com':\n> \n> So I think the problem is with pushurl, not with the auth code. Oddly,\n> though, running GIT_TRACE reveals:\n\nYep, I have a pushurl in config.\n\n> \n>   $ GIT_TRACE=1 git push foo\n>   trace: built-in: git 'push' 'foo'\n>   trace: run_command: 'git-remote-https' 'foo' 'https://peff@github.com/peff/git.git'\n> \n> which is the right URL. So it's almost as if we are throwing away the\n> passed URL in favor of the configuration, and then looking up the\n> configuration wrong. I'm about to go get on a plane, so I don't have\n> more time to look at it now, but I suspect it's something simple and\n> stupid.\n\nThanks for confirming what it should be like. I never expected pushurl\nand url to behave differently (I had used GIT_TRACE myself).\n\nGood flight :)\n\nMichael\n"},{"id":"177020","messageId":"2f1eccfa3fa9e732e9bea344fd69dfd9b16697a9.1317906388.git.git@drmicha.warpmail.net","threadId":"28567","inReplyTo":"4E8D4BD5.2090202@drmicha.warpmail.net","subject":"[RFC/PATCH] remote-curl: Obey passed URL","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-10-06T13:15:59Z","receivedAt":"2011-10-06T13:15:59Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"When the curl remote helper is called, e.g., as\n\ngit-remote-https foo https://john@doe.com\n\nit looks up remote.foo.url rather than taking the provided url, at least\nas far as http_init() is concerned (authentication). (It does use the\nprovided url at later stages.)\n\nThe problem is that for pushing, \"git push\" looks up the pushurl, which\nmay be different, and passes that down to the remote helper, so that the\nremote helper should take that when provided.\n\nNote that at the init stage, the remote helper does not know what kind of\ntranscation is going to be requested, but the caller does.\n\nChange it so that the remote helper obeys the passed url rather than\ntrying to look it up.\n\nSigned-off-by: Michael J Gruber <git@drmicha.warpmail.net>\n---\nWhile this passes all tests and fixes the described problem, I don't\nknow about any side effects. Also, I always feel a bit insecure about\nsimply dropping pointers to allocated memory (remote->url[0]). Should\nI free() it? Neither remote-helpers nor memory management are exactly\nhomeground for me...\n---\n remote-curl.c |    1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\ndiff --git a/remote-curl.c b/remote-curl.c\nindex b8cf45a..1fa396a 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -846,6 +846,7 @@ int main(int argc, const char **argv)\n \n \tif (argc > 2) {\n \t\tend_url_with_slash(&buf, argv[2]);\n+\t\tremote->url[0] = xstrdup(argv[2]);\n \t} else {\n \t\tend_url_with_slash(&buf, remote->url[0]);\n \t}\n-- \n1.7.7.rc2.451.g15e150\n"},{"id":"177022","messageId":"20111006132500.GA1792@sigill.intra.peff.net","threadId":"28567","inReplyTo":"2f1eccfa3fa9e732e9bea344fd69dfd9b16697a9.1317906388.git.git@drmicha.warpmail.net","subject":"Re: [RFC/PATCH] remote-curl: Obey passed URL","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-10-06T13:25:00Z","receivedAt":"2011-10-06T13:25:00Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 06, 2011 at 03:15:59PM +0200, Michael J Gruber wrote:\n\n> When the curl remote helper is called, e.g., as\n> \n> git-remote-https foo https://john@doe.com\n> \n> it looks up remote.foo.url rather than taking the provided url, at least\n> as far as http_init() is concerned (authentication). (It does use the\n> provided url at later stages.)\n> [...]\n> --- a/remote-curl.c\n> +++ b/remote-curl.c\n> @@ -846,6 +846,7 @@ int main(int argc, const char **argv)\n>  \n>  \tif (argc > 2) {\n>  \t\tend_url_with_slash(&buf, argv[2]);\n> +\t\tremote->url[0] = xstrdup(argv[2]);\n>  \t} else {\n>  \t\tend_url_with_slash(&buf, remote->url[0]);\n>  \t}\n\nYour analysis is correct, but tweaking the remote object seems kind\nof ugly. I think a nicer solution would be to pass the URL in\nseparately to http_init. Of the three existing callers:\n\n  1. http-fetch.c just passes a NULL remote. Which means we don't even\n     look at the URL at all for grabbing the auth information. Passing\n     the URL would be an improvement.\n\n  2. http-push.c creates a fake remote just to stick the URL in. That\n     ugly code could just go away.\n\n  3. remote-curl.c needs to pass in the alternate URL anyway, as you\n     described.\n\nSo it seems like all callsites would benefit.\n\nThat being said, I wonder if http_init is the right place to pull the\nauth information out. It's where we've always done it, and it makes\nsense if you are hitting one base URL. But what happens if we get a\nredirect to some other site? Shouldn't we be deciding at the time of\nmaking the request what the context of the http request is?\n\nAnd then http_init can stop caring entirely what the URL is.\n\n-Peff\n"},{"id":"177026","messageId":"20111006133758.GA18033@sigill.intra.peff.net","threadId":"28567","inReplyTo":"20111006132500.GA1792@sigill.intra.peff.net","subject":"Re: [RFC/PATCH] remote-curl: Obey passed URL","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-10-06T13:37:58Z","receivedAt":"2011-10-06T13:37:58Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 06, 2011 at 09:25:00AM -0400, Jeff King wrote:\n\n> Your analysis is correct, but tweaking the remote object seems kind\n> of ugly. I think a nicer solution would be to pass the URL in\n> separately to http_init. Of the three existing callers:\n\nHere's what that patch looks like. It's definitely an improvement and\nfixes a real bug, so it may be worth applying. But I'm still going to\nlook into pushing the url examination closer to the point of use.\n\n-- >8 --\nSubject: [PATCH] http_init: accept separate URL parameter\n\nThe http_init function takes a \"struct remote\". Part of its\ninitialization procedure is to look at the remote's url and\ngrab some auth-related parameters. However, using the url\nincluded in the remote is:\n\n  - wrong; the remote-curl helper may have a separate,\n    unrelated URL (e.g., from remote.*.pushurl). Looking at\n    the remote's configured url is incorrect.\n\n  - incomplete; http-fetch doesn't have a remote, so passes\n    NULL. So http_init never gets to see the URL we are\n    actually going to use.\n\n  - cumbersome; http-push has a similar problem to\n    http-fetch, but actually builds a fake remote just to\n    pass in the URL.\n\nInstead, let's just add a separate URL parameter to\nhttp_init, and all three callsites can pass in the\nappropriate information.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n http-fetch.c  |    2 +-\n http-push.c   |   10 +---------\n http.c        |    8 ++++----\n http.h        |    2 +-\n remote-curl.c |    2 +-\n 5 files changed, 8 insertions(+), 16 deletions(-)\n\ndiff --git a/http-fetch.c b/http-fetch.c\nindex 3af4c71..e341872 100644\n--- a/http-fetch.c\n+++ b/http-fetch.c\n@@ -63,7 +63,7 @@ int main(int argc, const char **argv)\n \n \tgit_config(git_default_config, NULL);\n \n-\thttp_init(NULL);\n+\thttp_init(NULL, url);\n \twalker = get_http_walker(url);\n \twalker->get_tree = get_tree;\n \twalker->get_history = get_history;\ndiff --git a/http-push.c b/http-push.c\nindex 376331a..215b640 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -1747,7 +1747,6 @@ int main(int argc, char **argv)\n \tint i;\n \tint new_refs;\n \tstruct ref *ref, *local_refs;\n-\tstruct remote *remote;\n \n \tgit_extract_argv0_path(argv[0]);\n \n@@ -1821,14 +1820,7 @@ int main(int argc, char **argv)\n \n \tmemset(remote_dir_exists, -1, 256);\n \n-\t/*\n-\t * Create a minimum remote by hand to give to http_init(),\n-\t * primarily to allow it to look at the URL.\n-\t */\n-\tremote = xcalloc(sizeof(*remote), 1);\n-\tALLOC_GROW(remote->url, remote->url_nr + 1, remote->url_alloc);\n-\tremote->url[remote->url_nr++] = repo->url;\n-\thttp_init(remote);\n+\thttp_init(NULL, repo->url);\n \n #ifdef USE_CURL_MULTI\n \tis_running_queue = 0;\ndiff --git a/http.c b/http.c\nindex b2ae8de..d9f9938 100644\n--- a/http.c\n+++ b/http.c\n@@ -357,7 +357,7 @@ static void set_from_env(const char **var, const char *envname)\n \t\t*var = val;\n }\n \n-void http_init(struct remote *remote)\n+void http_init(struct remote *remote, const char *url)\n {\n \tchar *low_speed_limit;\n \tchar *low_speed_time;\n@@ -421,11 +421,11 @@ void http_init(struct remote *remote)\n \tif (getenv(\"GIT_CURL_FTP_NO_EPSV\"))\n \t\tcurl_ftp_no_epsv = 1;\n \n-\tif (remote && remote->url && remote->url[0]) {\n-\t\thttp_auth_init(remote->url[0]);\n+\tif (url) {\n+\t\thttp_auth_init(url);\n \t\tif (!ssl_cert_password_required &&\n \t\t    getenv(\"GIT_SSL_CERT_PASSWORD_PROTECTED\") &&\n-\t\t    !prefixcmp(remote->url[0], \"https://\"))\n+\t\t    !prefixcmp(url, \"https://\"))\n \t\t\tssl_cert_password_required = 1;\n \t}\n \ndiff --git a/http.h b/http.h\nindex 0bf8592..3c332a9 100644\n--- a/http.h\n+++ b/http.h\n@@ -86,7 +86,7 @@ struct buffer {\n extern void step_active_slots(void);\n #endif\n \n-extern void http_init(struct remote *remote);\n+extern void http_init(struct remote *remote, const char *url);\n extern void http_cleanup(void);\n \n extern int data_received;\ndiff --git a/remote-curl.c b/remote-curl.c\nindex 69831e9..33d3d8c 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -852,7 +852,7 @@ int main(int argc, const char **argv)\n \n \turl = strbuf_detach(&buf, NULL);\n \n-\thttp_init(remote);\n+\thttp_init(remote, url);\n \n \tdo {\n \t\tif (strbuf_getline(&buf, stdin, '\\n') == EOF)\n-- \n1.7.7.37.g0e376\n"},{"id":"177485","messageId":"4E95FDC8.5030009@drmicha.warpmail.net","threadId":"28567","inReplyTo":"20111006133758.GA18033@sigill.intra.peff.net","subject":"Re: [RFC/PATCH] remote-curl: Obey passed URL","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-10-12T20:51:20Z","receivedAt":"2011-10-12T20:51:20Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Jeff King venit, vidit, dixit 06.10.2011 15:37:\n> On Thu, Oct 06, 2011 at 09:25:00AM -0400, Jeff King wrote:\n> \n>> Your analysis is correct, but tweaking the remote object seems kind\n>> of ugly. I think a nicer solution would be to pass the URL in\n>> separately to http_init. Of the three existing callers:\n> \n> Here's what that patch looks like. It's definitely an improvement and\n> fixes a real bug, so it may be worth applying. But I'm still going to\n> look into pushing the url examination closer to the point of use.\n\nIt definitely is an improvement. I've been running happily with this\n(and without my askpass helper/workaround). Are you going forward with\nthis one?\n\n> \n> -- >8 --\n> Subject: [PATCH] http_init: accept separate URL parameter\n> \n> The http_init function takes a \"struct remote\". Part of its\n> initialization procedure is to look at the remote's url and\n> grab some auth-related parameters. However, using the url\n> included in the remote is:\n> \n>   - wrong; the remote-curl helper may have a separate,\n>     unrelated URL (e.g., from remote.*.pushurl). Looking at\n>     the remote's configured url is incorrect.\n> \n>   - incomplete; http-fetch doesn't have a remote, so passes\n>     NULL. So http_init never gets to see the URL we are\n>     actually going to use.\n> \n>   - cumbersome; http-push has a similar problem to\n>     http-fetch, but actually builds a fake remote just to\n>     pass in the URL.\n> \n> Instead, let's just add a separate URL parameter to\n> http_init, and all three callsites can pass in the\n> appropriate information.\n> \n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n>  http-fetch.c  |    2 +-\n>  http-push.c   |   10 +---------\n>  http.c        |    8 ++++----\n>  http.h        |    2 +-\n>  remote-curl.c |    2 +-\n>  5 files changed, 8 insertions(+), 16 deletions(-)\n> \n> diff --git a/http-fetch.c b/http-fetch.c\n> index 3af4c71..e341872 100644\n> --- a/http-fetch.c\n> +++ b/http-fetch.c\n> @@ -63,7 +63,7 @@ int main(int argc, const char **argv)\n>  \n>  \tgit_config(git_default_config, NULL);\n>  \n> -\thttp_init(NULL);\n> +\thttp_init(NULL, url);\n>  \twalker = get_http_walker(url);\n>  \twalker->get_tree = get_tree;\n>  \twalker->get_history = get_history;\n> diff --git a/http-push.c b/http-push.c\n> index 376331a..215b640 100644\n> --- a/http-push.c\n> +++ b/http-push.c\n> @@ -1747,7 +1747,6 @@ int main(int argc, char **argv)\n>  \tint i;\n>  \tint new_refs;\n>  \tstruct ref *ref, *local_refs;\n> -\tstruct remote *remote;\n>  \n>  \tgit_extract_argv0_path(argv[0]);\n>  \n> @@ -1821,14 +1820,7 @@ int main(int argc, char **argv)\n>  \n>  \tmemset(remote_dir_exists, -1, 256);\n>  \n> -\t/*\n> -\t * Create a minimum remote by hand to give to http_init(),\n> -\t * primarily to allow it to look at the URL.\n> -\t */\n> -\tremote = xcalloc(sizeof(*remote), 1);\n> -\tALLOC_GROW(remote->url, remote->url_nr + 1, remote->url_alloc);\n> -\tremote->url[remote->url_nr++] = repo->url;\n> -\thttp_init(remote);\n> +\thttp_init(NULL, repo->url);\n>  \n>  #ifdef USE_CURL_MULTI\n>  \tis_running_queue = 0;\n> diff --git a/http.c b/http.c\n> index b2ae8de..d9f9938 100644\n> --- a/http.c\n> +++ b/http.c\n> @@ -357,7 +357,7 @@ static void set_from_env(const char **var, const char *envname)\n>  \t\t*var = val;\n>  }\n>  \n> -void http_init(struct remote *remote)\n> +void http_init(struct remote *remote, const char *url)\n>  {\n>  \tchar *low_speed_limit;\n>  \tchar *low_speed_time;\n> @@ -421,11 +421,11 @@ void http_init(struct remote *remote)\n>  \tif (getenv(\"GIT_CURL_FTP_NO_EPSV\"))\n>  \t\tcurl_ftp_no_epsv = 1;\n>  \n> -\tif (remote && remote->url && remote->url[0]) {\n> -\t\thttp_auth_init(remote->url[0]);\n> +\tif (url) {\n> +\t\thttp_auth_init(url);\n>  \t\tif (!ssl_cert_password_required &&\n>  \t\t    getenv(\"GIT_SSL_CERT_PASSWORD_PROTECTED\") &&\n> -\t\t    !prefixcmp(remote->url[0], \"https://\"))\n> +\t\t    !prefixcmp(url, \"https://\"))\n>  \t\t\tssl_cert_password_required = 1;\n>  \t}\n>  \n> diff --git a/http.h b/http.h\n> index 0bf8592..3c332a9 100644\n> --- a/http.h\n> +++ b/http.h\n> @@ -86,7 +86,7 @@ struct buffer {\n>  extern void step_active_slots(void);\n>  #endif\n>  \n> -extern void http_init(struct remote *remote);\n> +extern void http_init(struct remote *remote, const char *url);\n>  extern void http_cleanup(void);\n>  \n>  extern int data_received;\n> diff --git a/remote-curl.c b/remote-curl.c\n> index 69831e9..33d3d8c 100644\n> --- a/remote-curl.c\n> +++ b/remote-curl.c\n> @@ -852,7 +852,7 @@ int main(int argc, const char **argv)\n>  \n>  \turl = strbuf_detach(&buf, NULL);\n>  \n> -\thttp_init(remote);\n> +\thttp_init(remote, url);\n>  \n>  \tdo {\n>  \t\tif (strbuf_getline(&buf, stdin, '\\n') == EOF)\n"},{"id":"177489","messageId":"20111012214316.GA4393@sigill.intra.peff.net","threadId":"28567","inReplyTo":"4E95FDC8.5030009@drmicha.warpmail.net","subject":"[PATCH] http_init: accept separate URL parameter","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-10-12T21:43:16Z","receivedAt":"2011-10-12T21:43:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The http_init function takes a \"struct remote\". Part of its\ninitialization procedure is to look at the remote's url and\ngrab some auth-related parameters. However, using the url\nincluded in the remote is:\n\n  - wrong; the remote-curl helper may have a separate,\n    unrelated URL (e.g., from remote.*.pushurl). Looking at\n    the remote's configured url is incorrect.\n\n  - incomplete; http-fetch doesn't have a remote, so passes\n    NULL. So http_init never gets to see the URL we are\n    actually going to use.\n\n  - cumbersome; http-push has a similar problem to\n    http-fetch, but actually builds a fake remote just to\n    pass in the URL.\n\nInstead, let's just add a separate URL parameter to\nhttp_init, and all three callsites can pass in the\nappropriate information.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nOn Wed, Oct 12, 2011 at 10:51:20PM +0200, Michael J Gruber wrote:\n\n> > Here's what that patch looks like. It's definitely an improvement and\n> > fixes a real bug, so it may be worth applying. But I'm still going to\n> > look into pushing the url examination closer to the point of use.\n> \n> It definitely is an improvement. I've been running happily with this\n> (and without my askpass helper/workaround). Are you going forward with\n> this one?\n\nI think we should go ahead with this one. I gave some thought to\ntweaking the http code to figure out authentication closer to the point\nof use, so we could be adaptive to things like redirects. But it's quite\nan invasive change, since we now have to start possibly keeping a string\nof credentials, each mapped from their context.\n\nBut more importantly, it changes the user-visible behavior. If I do\nsomething like:\n\n  git fetch https://user@git.foo.com/repo.git\n\nand give it a password, and then it redirects me to \"git2.foo.com\" or\nsomething, then right now we will retry the same credential. I'm not\nsure if people rely on that or not.\n\nArguably, it's wrong to do so in the general case. If I redirect to\n\"git.someotherdomain.com\", you probably _do_ want to re-ask the\ncredential. So maybe it should be changed, and there should be some\nmagic with comparing the old and new contexts. I dunno.\n\nAt any rate, this is certainly an improvement in the meantime. If the\nurl parameter to http_init eventually goes away, it is easy enough to do\non top of this.\n\n http-fetch.c  |    2 +-\n http-push.c   |   10 +---------\n http.c        |    8 ++++----\n http.h        |    2 +-\n remote-curl.c |    2 +-\n 5 files changed, 8 insertions(+), 16 deletions(-)\n\ndiff --git a/http-fetch.c b/http-fetch.c\nindex 3af4c71..e341872 100644\n--- a/http-fetch.c\n+++ b/http-fetch.c\n@@ -63,7 +63,7 @@ int main(int argc, const char **argv)\n \n \tgit_config(git_default_config, NULL);\n \n-\thttp_init(NULL);\n+\thttp_init(NULL, url);\n \twalker = get_http_walker(url);\n \twalker->get_tree = get_tree;\n \twalker->get_history = get_history;\ndiff --git a/http-push.c b/http-push.c\nindex 6e8f6d0..ecbfae5 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -1747,7 +1747,6 @@ int main(int argc, char **argv)\n \tint i;\n \tint new_refs;\n \tstruct ref *ref, *local_refs;\n-\tstruct remote *remote;\n \n \tgit_extract_argv0_path(argv[0]);\n \n@@ -1821,14 +1820,7 @@ int main(int argc, char **argv)\n \n \tmemset(remote_dir_exists, -1, 256);\n \n-\t/*\n-\t * Create a minimum remote by hand to give to http_init(),\n-\t * primarily to allow it to look at the URL.\n-\t */\n-\tremote = xcalloc(sizeof(*remote), 1);\n-\tALLOC_GROW(remote->url, remote->url_nr + 1, remote->url_alloc);\n-\tremote->url[remote->url_nr++] = repo->url;\n-\thttp_init(remote);\n+\thttp_init(NULL, repo->url);\n \n #ifdef USE_CURL_MULTI\n \tis_running_queue = 0;\ndiff --git a/http.c b/http.c\nindex d6b2d78..65d3aa7 100644\n--- a/http.c\n+++ b/http.c\n@@ -356,7 +356,7 @@ static void set_from_env(const char **var, const char *envname)\n \t\t*var = val;\n }\n \n-void http_init(struct remote *remote)\n+void http_init(struct remote *remote, const char *url)\n {\n \tchar *low_speed_limit;\n \tchar *low_speed_time;\n@@ -420,11 +420,11 @@ void http_init(struct remote *remote)\n \tif (getenv(\"GIT_CURL_FTP_NO_EPSV\"))\n \t\tcurl_ftp_no_epsv = 1;\n \n-\tif (remote && remote->url && remote->url[0]) {\n-\t\thttp_auth_init(remote->url[0]);\n+\tif (url) {\n+\t\thttp_auth_init(url);\n \t\tif (!ssl_cert_password_required &&\n \t\t    getenv(\"GIT_SSL_CERT_PASSWORD_PROTECTED\") &&\n-\t\t    !prefixcmp(remote->url[0], \"https://\"))\n+\t\t    !prefixcmp(url, \"https://\"))\n \t\t\tssl_cert_password_required = 1;\n \t}\n \ndiff --git a/http.h b/http.h\nindex 0bf8592..3c332a9 100644\n--- a/http.h\n+++ b/http.h\n@@ -86,7 +86,7 @@ struct buffer {\n extern void step_active_slots(void);\n #endif\n \n-extern void http_init(struct remote *remote);\n+extern void http_init(struct remote *remote, const char *url);\n extern void http_cleanup(void);\n \n extern int data_received;\ndiff --git a/remote-curl.c b/remote-curl.c\nindex 6c24ab1..d4d0910 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -850,7 +850,7 @@ int main(int argc, const char **argv)\n \n \turl = strbuf_detach(&buf, NULL);\n \n-\thttp_init(remote);\n+\thttp_init(remote, url);\n \n \tdo {\n \t\tif (strbuf_getline(&buf, stdin, '\\n') == EOF)\n-- \n1.7.7.rc2.21.gb9948\n"},{"id":"177491","messageId":"20111012214610.GA4578@sigill.intra.peff.net","threadId":"28567","inReplyTo":"20111012214316.GA4393@sigill.intra.peff.net","subject":"Re: [PATCH] http_init: accept separate URL parameter","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-10-12T21:46:10Z","receivedAt":"2011-10-12T21:46:10Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Oct 12, 2011 at 05:43:16PM -0400, Jeff King wrote:\n\n> The http_init function takes a \"struct remote\". Part of its\n> initialization procedure is to look at the remote's url and\n> grab some auth-related parameters. However, using the url\n> included in the remote is:\n> \n>   - wrong; the remote-curl helper may have a separate,\n>     unrelated URL (e.g., from remote.*.pushurl). Looking at\n>     the remote's configured url is incorrect.\n> \n>   - incomplete; http-fetch doesn't have a remote, so passes\n>     NULL. So http_init never gets to see the URL we are\n>     actually going to use.\n> \n>   - cumbersome; http-push has a similar problem to\n>     http-fetch, but actually builds a fake remote just to\n>     pass in the URL.\n> \n> Instead, let's just add a separate URL parameter to\n> http_init, and all three callsites can pass in the\n> appropriate information.\n> \n> Signed-off-by: Jeff King <peff@peff.net>\n\nSorry, I forgot to mention: this is meant to go on top of the\nhttp-auth-keyring topic.\n\n-Peff\n"},{"id":"177498","messageId":"7vk4897s4c.fsf@alter.siamese.dyndns.org","threadId":"28567","inReplyTo":"20111012214610.GA4578@sigill.intra.peff.net","subject":"Re: [PATCH] http_init: accept separate URL parameter","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-12T22:38:27Z","receivedAt":"2011-10-12T22:38:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Wed, Oct 12, 2011 at 05:43:16PM -0400, Jeff King wrote:\n> ...\n>> Instead, let's just add a separate URL parameter to\n>> http_init, and all three callsites can pass in the\n>> appropriate information.\n>> \n>> Signed-off-by: Jeff King <peff@peff.net>\n>\n> Sorry, I forgot to mention: this is meant to go on top of the\n> http-auth-keyring topic.\n\nHmm, of course the patch was written to help http-auth-keyring topic, but\nwouldn't this be an improvement that is general enough?  I.e. it could\neven go to the bottom of the topic, no?\n"},{"id":"177499","messageId":"20111012224625.GA11408@sigill.intra.peff.net","threadId":"28567","inReplyTo":"7vk4897s4c.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] http_init: accept separate URL parameter","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-10-12T22:46:25Z","receivedAt":"2011-10-12T22:46:25Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Oct 12, 2011 at 03:38:27PM -0700, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > On Wed, Oct 12, 2011 at 05:43:16PM -0400, Jeff King wrote:\n> > ...\n> >> Instead, let's just add a separate URL parameter to\n> >> http_init, and all three callsites can pass in the\n> >> appropriate information.\n> >> \n> >> Signed-off-by: Jeff King <peff@peff.net>\n> >\n> > Sorry, I forgot to mention: this is meant to go on top of the\n> > http-auth-keyring topic.\n> \n> Hmm, of course the patch was written to help http-auth-keyring topic, but\n> wouldn't this be an improvement that is general enough?  I.e. it could\n> even go to the bottom of the topic, no?\n\nYes, it could, and probably should. I suspect it might need some\nrebasing to do that.\n\nI'm going to float some other possible designs for the topic as soon as\nI put enough polish on them. So I'll try to move this down when I\nre-roll.  In the meantime, if you want to throw it on top, great. If you\nwant to ignore it until then, no problem. :)\n\n-Peff\n"},{"id":"177504","messageId":"CALUzUxp-Qe05+VF_EDg3+iHBeqWcEfcSVp3fyV_fQjAUaCVjOA@mail.gmail.com","threadId":"28567","inReplyTo":"20111012214316.GA4393@sigill.intra.peff.net","subject":"Re: [PATCH] http_init: accept separate URL parameter","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2011-10-13T02:06:40Z","receivedAt":"2011-10-13T02:06:40Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"On Thu, Oct 13, 2011 at 5:43 AM, Jeff King <peff@peff.net> wrote:\n> The http_init function takes a \"struct remote\". Part of its\n> initialization procedure is to look at the remote's url and\n> grab some auth-related parameters. However, using the url\n> included in the remote is:\n>\n>  - wrong; the remote-curl helper may have a separate,\n>    unrelated URL (e.g., from remote.*.pushurl). Looking at\n>    the remote's configured url is incorrect.\n>\n>  - incomplete; http-fetch doesn't have a remote, so passes\n>    NULL. So http_init never gets to see the URL we are\n>    actually going to use.\n>\n>  - cumbersome; http-push has a similar problem to\n>    http-fetch, but actually builds a fake remote just to\n>    pass in the URL.\n>\n> Instead, let's just add a separate URL parameter to\n> http_init, and all three callsites can pass in the\n> appropriate information.\n>\n> Signed-off-by: Jeff King <peff@peff.net>\n\nThis is excellent.\n\n  Acked-by: Tay Ray Chuan <rctay89@gmail.com>\n\n-- \nCheers,\nRay Chuan\n"},{"id":"177516","messageId":"4E9692BF.8040705@drmicha.warpmail.net","threadId":"28567","inReplyTo":"20111012224625.GA11408@sigill.intra.peff.net","subject":"Re: [PATCH] http_init: accept separate URL parameter","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-10-13T07:26:55Z","receivedAt":"2011-10-13T07:26:55Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Jeff King venit, vidit, dixit 13.10.2011 00:46:\n> On Wed, Oct 12, 2011 at 03:38:27PM -0700, Junio C Hamano wrote:\n> \n>> Jeff King <peff@peff.net> writes:\n>>\n>>> On Wed, Oct 12, 2011 at 05:43:16PM -0400, Jeff King wrote:\n>>> ...\n>>>> Instead, let's just add a separate URL parameter to\n>>>> http_init, and all three callsites can pass in the\n>>>> appropriate information.\n>>>>\n>>>> Signed-off-by: Jeff King <peff@peff.net>\n>>>\n>>> Sorry, I forgot to mention: this is meant to go on top of the\n>>> http-auth-keyring topic.\n>>\n>> Hmm, of course the patch was written to help http-auth-keyring topic, but\n>> wouldn't this be an improvement that is general enough?  I.e. it could\n>> even go to the bottom of the topic, no?\n> \n> Yes, it could, and probably should. I suspect it might need some\n> rebasing to do that.\n> \n> I'm going to float some other possible designs for the topic as soon as\n> I put enough polish on them. So I'll try to move this down when I\n> re-roll.  In the meantime, if you want to throw it on top, great. If you\n> want to ignore it until then, no problem. :)\n> \n> -Peff\n\nThanks, Jeff.\n\nTo clarify:\n\nWithout http-auth-keyring, this helps in the sense that git reads the\nusername from a user@host URL and asks for the password only. When using\nGIT_ASKPASS or such, the askpass helper is called with \"Password:\" only.\n\nWith (parts of) http-auth-keyring, the askpass helper is called with\n\"Password for:user@host\", which helps the user identify the request, and\nwhich helps helpers such as ksshaskpass to store the password with a\nmeaningful key in a wallet.\n\nI'm not sure whether it's feasible/worth taking these bits of\nhttp-auth-keyring (improved prompt) out and apply them early. That is,\nI'm sure it's worth it (it would alleviate the need for credential\nhelpers for some users at least), I haven't looked at feasibility ;)\n\nMichael\n"},{"id":"177616","messageId":"cover.1318577792.git.git@drmicha.warpmail.net","threadId":"28567","inReplyTo":"4E9692BF.8040705@drmicha.warpmail.net","subject":"[PATCH 0/6] http-auth-early","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-10-14T07:40:34Z","receivedAt":"2011-10-14T07:40:34Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Here are the early parts of Jeff's http-auth-keyring series.\nIt contains only parts which are not using the credential API (which\nis still under discussion), so that this can go in (and help users)\nand alleviates the pressure on the credential discussion:\n\nEarly bits with cleanups to http.c.\nCherry-picked bit for improved prompts (\"Username for ...\" etc.)\nCherry-pickes bit for using configured pushurls.\n\nI tried to pick/resolve in a way which should help rebasing Jeff's series\non top of this.\n\nJeff King (5):\n  url: decode buffers that are not NUL-terminated\n  improve httpd auth tests\n  remote-curl: don't retry auth failures with dumb protocol\n  http: retry authentication failures for all http requests\n  http_init: accept separate URL parameter\n\nMichael J Gruber (1):\n  http: use hostname in credential description\n\n http-fetch.c          |    2 +-\n http-push.c           |   10 +-----\n http.c                |   91 +++++++++++++++++++++++++++---------------------\n http.h                |    2 +-\n remote-curl.c         |    4 +-\n t/lib-httpd.sh        |   10 +++--\n t/t5550-http-fetch.sh |   51 +++++++++++++++++++++++++--\n url.c                 |   26 ++++++++++----\n url.h                 |    1 +\n 9 files changed, 128 insertions(+), 69 deletions(-)\n\n-- \n1.7.7.338.g0156b\n"},{"id":"177617","messageId":"66c8448543432308e8fce5e3e04076e875410f67.1318577792.git.git@drmicha.warpmail.net","threadId":"28567","inReplyTo":"4E9692BF.8040705@drmicha.warpmail.net","subject":"[PATCH 1/6] url: decode buffers that are not NUL-terminated","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-10-14T07:40:35Z","receivedAt":"2011-10-14T07:40:35Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"From: Jeff King <peff@peff.net>\n\nThe url_decode function needs only minor tweaks to handle\narbitrary buffers. Let's do those tweaks, which cleans up an\nunreadable mess of temporary strings in http.c.\n\nSigned-off-by: Jeff King <peff@peff.net>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n http.c |   27 ++++-----------------------\n url.c  |   26 ++++++++++++++++++--------\n url.h  |    1 +\n 3 files changed, 23 insertions(+), 31 deletions(-)\n\ndiff --git a/http.c b/http.c\nindex a1ea3db..c93716c 100644\n--- a/http.c\n+++ b/http.c\n@@ -307,8 +307,7 @@ static CURL *get_curl_handle(void)\n \n static void http_auth_init(const char *url)\n {\n-\tchar *at, *colon, *cp, *slash, *decoded;\n-\tint len;\n+\tchar *at, *colon, *cp, *slash;\n \n \tcp = strstr(url, \"://\");\n \tif (!cp)\n@@ -328,29 +327,11 @@ static void http_auth_init(const char *url)\n \t\treturn; /* No credentials */\n \tif (!colon || at <= colon) {\n \t\t/* Only username */\n-\t\tlen = at - cp;\n-\t\tuser_name = xmalloc(len + 1);\n-\t\tmemcpy(user_name, cp, len);\n-\t\tuser_name[len] = '\\0';\n-\t\tdecoded = url_decode(user_name);\n-\t\tfree(user_name);\n-\t\tuser_name = decoded;\n+\t\tuser_name = url_decode_mem(cp, at - cp);\n \t\tuser_pass = NULL;\n \t} else {\n-\t\tlen = colon - cp;\n-\t\tuser_name = xmalloc(len + 1);\n-\t\tmemcpy(user_name, cp, len);\n-\t\tuser_name[len] = '\\0';\n-\t\tdecoded = url_decode(user_name);\n-\t\tfree(user_name);\n-\t\tuser_name = decoded;\n-\t\tlen = at - (colon + 1);\n-\t\tuser_pass = xmalloc(len + 1);\n-\t\tmemcpy(user_pass, colon + 1, len);\n-\t\tuser_pass[len] = '\\0';\n-\t\tdecoded = url_decode(user_pass);\n-\t\tfree(user_pass);\n-\t\tuser_pass = decoded;\n+\t\tuser_name = url_decode_mem(cp, colon - cp);\n+\t\tuser_pass = url_decode_mem(colon + 1, at - (colon + 1));\n \t}\n }\n \ndiff --git a/url.c b/url.c\nindex 3e06fd3..389d9da 100644\n--- a/url.c\n+++ b/url.c\n@@ -68,18 +68,20 @@ static int url_decode_char(const char *q)\n \treturn val;\n }\n \n-static char *url_decode_internal(const char **query, const char *stop_at,\n-\t\t\t\t struct strbuf *out, int decode_plus)\n+static char *url_decode_internal(const char **query, int len,\n+\t\t\t\t const char *stop_at, struct strbuf *out,\n+\t\t\t\t int decode_plus)\n {\n \tconst char *q = *query;\n \n-\tdo {\n+\twhile (len) {\n \t\tunsigned char c = *q;\n \n \t\tif (!c)\n \t\t\tbreak;\n \t\tif (stop_at && strchr(stop_at, c)) {\n \t\t\tq++;\n+\t\t\tlen--;\n \t\t\tbreak;\n \t\t}\n \n@@ -88,6 +90,7 @@ static char *url_decode_internal(const char **query, const char *stop_at,\n \t\t\tif (0 <= val) {\n \t\t\t\tstrbuf_addch(out, val);\n \t\t\t\tq += 3;\n+\t\t\t\tlen -= 3;\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t}\n@@ -97,34 +100,41 @@ static char *url_decode_internal(const char **query, const char *stop_at,\n \t\telse\n \t\t\tstrbuf_addch(out, c);\n \t\tq++;\n-\t} while (1);\n+\t\tlen--;\n+\t}\n \t*query = q;\n \treturn strbuf_detach(out, NULL);\n }\n \n char *url_decode(const char *url)\n {\n+\treturn url_decode_mem(url, strlen(url));\n+}\n+\n+char *url_decode_mem(const char *url, int len)\n+{\n \tstruct strbuf out = STRBUF_INIT;\n-\tconst char *colon = strchr(url, ':');\n+\tconst char *colon = memchr(url, ':', len);\n \n \t/* Skip protocol part if present */\n \tif (colon && url < colon) {\n \t\tstrbuf_add(&out, url, colon - url);\n+\t\tlen -= colon - url;\n \t\turl = colon;\n \t}\n-\treturn url_decode_internal(&url, NULL, &out, 0);\n+\treturn url_decode_internal(&url, len, NULL, &out, 0);\n }\n \n char *url_decode_parameter_name(const char **query)\n {\n \tstruct strbuf out = STRBUF_INIT;\n-\treturn url_decode_internal(query, \"&=\", &out, 1);\n+\treturn url_decode_internal(query, -1, \"&=\", &out, 1);\n }\n \n char *url_decode_parameter_value(const char **query)\n {\n \tstruct strbuf out = STRBUF_INIT;\n-\treturn url_decode_internal(query, \"&\", &out, 1);\n+\treturn url_decode_internal(query, -1, \"&\", &out, 1);\n }\n \n void end_url_with_slash(struct strbuf *buf, const char *url)\ndiff --git a/url.h b/url.h\nindex 7100e32..abdaf6f 100644\n--- a/url.h\n+++ b/url.h\n@@ -4,6 +4,7 @@\n extern int is_url(const char *url);\n extern int is_urlschemechar(int first_flag, int ch);\n extern char *url_decode(const char *url);\n+extern char *url_decode_mem(const char *url, int len);\n extern char *url_decode_parameter_name(const char **query);\n extern char *url_decode_parameter_value(const char **query);\n \n-- \n1.7.7.338.g0156b\n"},{"id":"177618","messageId":"5232586c7985e6a420ee741e19e7fd6d040d43f6.1318577792.git.git@drmicha.warpmail.net","threadId":"28567","inReplyTo":"4E9692BF.8040705@drmicha.warpmail.net","subject":"[PATCH 2/6] improve httpd auth tests","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-10-14T07:40:36Z","receivedAt":"2011-10-14T07:40:36Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"From: Jeff King <peff@peff.net>\n\nThese just checked that we could clone a repository when the\nusername and password were given in the URL; we should also\ncheck that git will prompt when no or partial credentials\nare given.\n\nSigned-off-by: Jeff King <peff@peff.net>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n t/lib-httpd.sh        |   10 +++++---\n t/t5550-http-fetch.sh |   51 +++++++++++++++++++++++++++++++++++++++++++++---\n 2 files changed, 53 insertions(+), 8 deletions(-)\n\ndiff --git a/t/lib-httpd.sh b/t/lib-httpd.sh\nindex b8996a3..f7dc078 100644\n--- a/t/lib-httpd.sh\n+++ b/t/lib-httpd.sh\n@@ -81,8 +81,7 @@ prepare_httpd() {\n \n \tif test -n \"$LIB_HTTPD_SSL\"\n \tthen\n-\t\tHTTPD_URL=https://127.0.0.1:$LIB_HTTPD_PORT\n-\t\tAUTH_HTTPD_URL=https://user%40host:user%40host@127.0.0.1:$LIB_HTTPD_PORT\n+\t\tHTTPD_PROTO=https\n \n \t\tRANDFILE_PATH=\"$HTTPD_ROOT_PATH\"/.rnd openssl req \\\n \t\t\t-config \"$TEST_PATH/ssl.cnf\" \\\n@@ -93,9 +92,12 @@ prepare_httpd() {\n \t\texport GIT_SSL_NO_VERIFY\n \t\tHTTPD_PARA=\"$HTTPD_PARA -DSSL\"\n \telse\n-\t\tHTTPD_URL=http://127.0.0.1:$LIB_HTTPD_PORT\n-\t\tAUTH_HTTPD_URL=http://user%40host:user%40host@127.0.0.1:$LIB_HTTPD_PORT\n+\t\tHTTPD_PROTO=http\n \tfi\n+\tHTTPD_DEST=127.0.0.1:$LIB_HTTPD_PORT\n+\tHTTPD_URL=$HTTPD_PROTO://$HTTPD_DEST\n+\tHTTPD_URL_USER=$HTTPD_PROTO://user%40host@$HTTPD_DEST\n+\tHTTPD_URL_USER_PASS=$HTTPD_PROTO://user%40host:user%40host@$HTTPD_DEST\n \n \tif test -n \"$LIB_HTTPD_DAV\" -o -n \"$LIB_HTTPD_SVN\"\n \tthen\ndiff --git a/t/t5550-http-fetch.sh b/t/t5550-http-fetch.sh\nindex a1883ca..ed4db09 100755\n--- a/t/t5550-http-fetch.sh\n+++ b/t/t5550-http-fetch.sh\n@@ -35,11 +35,54 @@ test_expect_success 'clone http repository' '\n \ttest_cmp file clone/file\n '\n \n-test_expect_success 'clone http repository with authentication' '\n+test_expect_success 'create password-protected repository' '\n \tmkdir \"$HTTPD_DOCUMENT_ROOT_PATH/auth/\" &&\n-\tcp -Rf \"$HTTPD_DOCUMENT_ROOT_PATH/repo.git\" \"$HTTPD_DOCUMENT_ROOT_PATH/auth/repo.git\" &&\n-\tgit clone $AUTH_HTTPD_URL/auth/repo.git clone-auth &&\n-\ttest_cmp file clone-auth/file\n+\tcp -Rf \"$HTTPD_DOCUMENT_ROOT_PATH/repo.git\" \\\n+\t       \"$HTTPD_DOCUMENT_ROOT_PATH/auth/repo.git\"\n+'\n+\n+test_expect_success 'setup askpass helpers' '\n+\tcat >askpass <<-EOF &&\n+\t#!/bin/sh\n+\techo >>\"$PWD/askpass-query\" \"askpass: \\$*\" &&\n+\tcat \"$PWD/askpass-response\"\n+\tEOF\n+\tchmod +x askpass &&\n+\tGIT_ASKPASS=\"$PWD/askpass\" &&\n+\texport GIT_ASKPASS &&\n+\t>askpass-expect-none &&\n+\techo \"askpass: Password: \" >askpass-expect-pass &&\n+\t{ echo \"askpass: Username: \" &&\n+\t  cat askpass-expect-pass\n+\t} >askpass-expect-both\n+'\n+\n+test_expect_success 'cloning password-protected repository can fail' '\n+\t>askpass-query &&\n+\techo wrong >askpass-response &&\n+\ttest_must_fail git clone \"$HTTPD_URL/auth/repo.git\" clone-auth-fail &&\n+\ttest_cmp askpass-expect-both askpass-query\n+'\n+\n+test_expect_success 'http auth can use user/pass in URL' '\n+\t>askpass-query &&\n+\techo wrong >askpass-reponse &&\n+\tgit clone \"$HTTPD_URL_USER_PASS/auth/repo.git\" clone-auth-none &&\n+\ttest_cmp askpass-expect-none askpass-query\n+'\n+\n+test_expect_success 'http auth can use just user in URL' '\n+\t>askpass-query &&\n+\techo user@host >askpass-response &&\n+\tgit clone \"$HTTPD_URL_USER/auth/repo.git\" clone-auth-pass &&\n+\ttest_cmp askpass-expect-pass askpass-query\n+'\n+\n+test_expect_success 'http auth can request both user and pass' '\n+\t>askpass-query &&\n+\techo user@host >askpass-response &&\n+\tgit clone \"$HTTPD_URL/auth/repo.git\" clone-auth-both &&\n+\ttest_cmp askpass-expect-both askpass-query\n '\n \n test_expect_success 'fetch changes via http' '\n-- \n1.7.7.338.g0156b\n"},{"id":"177619","messageId":"28d0c1017a10a93ce165a2d4e9fb6a691a933bd3.1318577792.git.git@drmicha.warpmail.net","threadId":"28567","inReplyTo":"4E9692BF.8040705@drmicha.warpmail.net","subject":"[PATCH 3/6] remote-curl: don't retry auth failures with dumb protocol","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-10-14T07:40:37Z","receivedAt":"2011-10-14T07:40:37Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"From: Jeff King <peff@peff.net>\n\nWhen fetching an http URL, we first try fetching info/refs\nwith an extra \"service\" parameter. This will work for a\nsmart-http server, or a dumb server which ignores extra\nparameters when fetching files. If that fails, we retry\nwithout the extra parameter to remain compatible with dumb\nservers which didn't like our first request.\n\nIf the server returned a \"401 Unauthorized\", indicating that\nthe credentials we provided were not good, there is not much\npoint in retrying. With the current code, we just waste an\nextra round trip to the HTTP server before failing.\n\nBut as the http code becomes smarter about throwing away\nrejected credentials and re-prompting the user for new ones\n(which it will later in this series), this will become more\nconfusing. At some point we will stop asking for credentials\nto retry smart http, and will be asking for credentials to\nretry dumb http. So now we're not only wasting an extra HTTP\nround trip for something that is unlikely to work, but we're\nmaking the user re-type their password for it.\n\nSigned-off-by: Jeff King <peff@peff.net>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n remote-curl.c |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/remote-curl.c b/remote-curl.c\nindex faaeda4..6c24ab1 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -115,7 +115,7 @@ static struct discovery* discover_refs(const char *service)\n \thttp_ret = http_get_strbuf(refs_url, &buffer, HTTP_NO_CACHE);\n \n \t/* try again with \"plain\" url (no ? or & appended) */\n-\tif (http_ret != HTTP_OK) {\n+\tif (http_ret != HTTP_OK && http_ret != HTTP_NOAUTH) {\n \t\tfree(refs_url);\n \t\tstrbuf_reset(&buffer);\n \n-- \n1.7.7.338.g0156b\n"},{"id":"177621","messageId":"8d677edc4fa3fd1fe12b49bf279aaad5be89b81c.1318577792.git.git@drmicha.warpmail.net","threadId":"28567","inReplyTo":"4E9692BF.8040705@drmicha.warpmail.net","subject":"[PATCH 4/6] http: retry authentication failures for all http requests","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-10-14T07:40:38Z","receivedAt":"2011-10-14T07:40:38Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"From: Jeff King <peff@peff.net>\n\nCommit 42653c0 (Prompt for a username when an HTTP request\n401s, 2010-04-01) changed http_get_strbuf to prompt for\ncredentials when we receive a 401, but didn't touch\nhttp_get_file. The latter is called only for dumb http;\nwhile it's usually the case that people don't use\nauthentication on top of dumb http, there is no reason not\nto allow both types of requests to use this feature.\n\nSigned-off-by: Jeff King <peff@peff.net>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n http.c |   17 +++++++++++------\n 1 files changed, 11 insertions(+), 6 deletions(-)\n\ndiff --git a/http.c b/http.c\nindex c93716c..89e3cf4 100644\n--- a/http.c\n+++ b/http.c\n@@ -846,13 +846,18 @@ static int http_request(const char *url, void *result, int target, int options)\n \treturn ret;\n }\n \n+static int http_request_reauth(const char *url, void *result, int target,\n+\t\t\t       int options)\n+{\n+\tint ret = http_request(url, result, target, options);\n+\tif (ret != HTTP_REAUTH)\n+\t\treturn ret;\n+\treturn http_request(url, result, target, options);\n+}\n+\n int http_get_strbuf(const char *url, struct strbuf *result, int options)\n {\n-\tint http_ret = http_request(url, result, HTTP_REQUEST_STRBUF, options);\n-\tif (http_ret == HTTP_REAUTH) {\n-\t\thttp_ret = http_request(url, result, HTTP_REQUEST_STRBUF, options);\n-\t}\n-\treturn http_ret;\n+\treturn http_request_reauth(url, result, HTTP_REQUEST_STRBUF, options);\n }\n \n /*\n@@ -875,7 +880,7 @@ static int http_get_file(const char *url, const char *filename, int options)\n \t\tgoto cleanup;\n \t}\n \n-\tret = http_request(url, result, HTTP_REQUEST_FILE, options);\n+\tret = http_request_reauth(url, result, HTTP_REQUEST_FILE, options);\n \tfclose(result);\n \n \tif ((ret == HTTP_OK) && move_temp_to_file(tmpfile.buf, filename))\n-- \n1.7.7.338.g0156b\n"},{"id":"177622","messageId":"f3365a7c54c17c2c53e5204578a3406057e62b5f.1318577792.git.git@drmicha.warpmail.net","threadId":"28567","inReplyTo":"4E9692BF.8040705@drmicha.warpmail.net","subject":"[PATCH 5/6] http: use hostname in credential description","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-10-14T07:40:39Z","receivedAt":"2011-10-14T07:40:39Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Until now, a request for an http password looked like:\n\n  Username:\n  Password:\n\nNow it will look like:\n\n  Username for 'example.com':\n  Password for 'example.com':\n\nPicked-from: Jeff King <peff@peff.net>\nSigned-off-by: Michael J Gruber <git@drmicha.warpmail.net>\n---\n http.c                |   41 +++++++++++++++++++++++++++++++++--------\n t/t5550-http-fetch.sh |    4 ++--\n 2 files changed, 35 insertions(+), 10 deletions(-)\n\ndiff --git a/http.c b/http.c\nindex 89e3cf4..149e116 100644\n--- a/http.c\n+++ b/http.c\n@@ -42,7 +42,7 @@ static long curl_low_speed_time = -1;\n static int curl_ftp_no_epsv;\n static const char *curl_http_proxy;\n static const char *curl_cookie_file;\n-static char *user_name, *user_pass;\n+static char *user_name, *user_pass, *description;\n static const char *user_agent;\n \n #if LIBCURL_VERSION_NUM >= 0x071700\n@@ -139,6 +139,25 @@ static void process_curl_messages(void)\n }\n #endif\n \n+static char *git_getpass_one(const char *what, const char *desc)\n+{\n+\tstruct strbuf prompt = STRBUF_INIT;\n+\tchar *r;\n+\n+\tif (desc)\n+\t\tstrbuf_addf(&prompt, \"%s for '%s': \", what, desc);\n+\telse\n+\t\tstrbuf_addf(&prompt, \"%s: \", what);\n+\t/* FIXME: for usernames, we should do something less magical that\n+\t* actually echoes the characters. However, we need to read from\n+\t* /dev/tty and not stdio, which is not portable (but getpass will do\n+\t* it for us). http.c uses the same workaround. */\n+\tr = git_getpass(prompt.buf);\n+\n+\tstrbuf_release(&prompt);\n+\treturn xstrdup(r);\n+}\n+\n static int http_options(const char *var, const char *value, void *cb)\n {\n \tif (!strcmp(\"http.sslverify\", var)) {\n@@ -214,7 +233,7 @@ static void init_curl_http_auth(CURL *result)\n \tif (user_name) {\n \t\tstruct strbuf up = STRBUF_INIT;\n \t\tif (!user_pass)\n-\t\t\tuser_pass = xstrdup(git_getpass(\"Password: \"));\n+\t\t\tuser_pass = xstrdup(git_getpass_one(\"Password\", description));\n \t\tstrbuf_addf(&up, \"%s:%s\", user_name, user_pass);\n \t\tcurl_easy_setopt(result, CURLOPT_USERPWD,\n \t\t\t\t strbuf_detach(&up, NULL));\n@@ -229,7 +248,7 @@ static int has_cert_password(void)\n \t\treturn 0;\n \t/* Only prompt the user once. */\n \tssl_cert_password_required = -1;\n-\tssl_cert_password = git_getpass(\"Certificate Password: \");\n+\tssl_cert_password = git_getpass_one(\"Certificate Password\", description);\n \tif (ssl_cert_password != NULL) {\n \t\tssl_cert_password = xstrdup(ssl_cert_password);\n \t\treturn 1;\n@@ -307,7 +326,7 @@ static CURL *get_curl_handle(void)\n \n static void http_auth_init(const char *url)\n {\n-\tchar *at, *colon, *cp, *slash;\n+\tconst char *at, *colon, *cp, *slash, *host;\n \n \tcp = strstr(url, \"://\");\n \tif (!cp)\n@@ -323,16 +342,22 @@ static void http_auth_init(const char *url)\n \tat = strchr(cp, '@');\n \tcolon = strchr(cp, ':');\n \tslash = strchrnul(cp, '/');\n-\tif (!at || slash <= at)\n-\t\treturn; /* No credentials */\n-\tif (!colon || at <= colon) {\n+\tif (!at || slash <= at) {\n+\t\t/* No credentials, but we may have to ask for some later */\n+\t\thost = cp;\n+\t}\n+\telse if (!colon || at <= colon) {\n \t\t/* Only username */\n \t\tuser_name = url_decode_mem(cp, at - cp);\n \t\tuser_pass = NULL;\n+\t\thost = at + 1;\n \t} else {\n \t\tuser_name = url_decode_mem(cp, colon - cp);\n \t\tuser_pass = url_decode_mem(colon + 1, at - (colon + 1));\n+\t\thost = at + 1;\n \t}\n+\n+\tdescription = url_decode_mem(host, slash - host);\n }\n \n static void set_from_env(const char **var, const char *envname)\n@@ -828,7 +853,7 @@ static int http_request(const char *url, void *result, int target, int options)\n \t\t\t\t * but that is non-portable.  Using git_getpass() can at least be stubbed\n \t\t\t\t * on other platforms with a different implementation if/when necessary.\n \t\t\t\t */\n-\t\t\t\tuser_name = xstrdup(git_getpass(\"Username: \"));\n+\t\t\t\tuser_name = xstrdup(git_getpass_one(\"Username\", description));\n \t\t\t\tinit_curl_http_auth(slot->curl);\n \t\t\t\tret = HTTP_REAUTH;\n \t\t\t}\ndiff --git a/t/t5550-http-fetch.sh b/t/t5550-http-fetch.sh\nindex ed4db09..d1ab4d0 100755\n--- a/t/t5550-http-fetch.sh\n+++ b/t/t5550-http-fetch.sh\n@@ -51,8 +51,8 @@ test_expect_success 'setup askpass helpers' '\n \tGIT_ASKPASS=\"$PWD/askpass\" &&\n \texport GIT_ASKPASS &&\n \t>askpass-expect-none &&\n-\techo \"askpass: Password: \" >askpass-expect-pass &&\n-\t{ echo \"askpass: Username: \" &&\n+\techo \"askpass: Password for '\\''$HTTPD_DEST'\\'': \" >askpass-expect-pass &&\n+\t{ echo \"askpass: Username for '\\''$HTTPD_DEST'\\'': \" &&\n \t  cat askpass-expect-pass\n \t} >askpass-expect-both\n '\n-- \n1.7.7.338.g0156b\n"},{"id":"177620","messageId":"57f0b2e5fedc91f22962517af620da8c209642ec.1318577792.git.git@drmicha.warpmail.net","threadId":"28567","inReplyTo":"4E9692BF.8040705@drmicha.warpmail.net","subject":"[PATCH 6/6] http_init: accept separate URL parameter","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-10-14T07:40:40Z","receivedAt":"2011-10-14T07:40:40Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"From: Jeff King <peff@peff.net>\n\nThe http_init function takes a \"struct remote\". Part of its\ninitialization procedure is to look at the remote's url and\ngrab some auth-related parameters. However, using the url\nincluded in the remote is:\n\n  - wrong; the remote-curl helper may have a separate,\n    unrelated URL (e.g., from remote.*.pushurl). Looking at\n    the remote's configured url is incorrect.\n\n  - incomplete; http-fetch doesn't have a remote, so passes\n    NULL. So http_init never gets to see the URL we are\n    actually going to use.\n\n  - cumbersome; http-push has a similar problem to\n    http-fetch, but actually builds a fake remote just to\n    pass in the URL.\n\nInstead, let's just add a separate URL parameter to\nhttp_init, and all three callsites can pass in the\nappropriate information.\n\nSigned-off-by: Jeff King <peff@peff.net>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n http-fetch.c  |    2 +-\n http-push.c   |   10 +---------\n http.c        |    8 ++++----\n http.h        |    2 +-\n remote-curl.c |    2 +-\n 5 files changed, 8 insertions(+), 16 deletions(-)\n\ndiff --git a/http-fetch.c b/http-fetch.c\nindex 3af4c71..e341872 100644\n--- a/http-fetch.c\n+++ b/http-fetch.c\n@@ -63,7 +63,7 @@ int main(int argc, const char **argv)\n \n \tgit_config(git_default_config, NULL);\n \n-\thttp_init(NULL);\n+\thttp_init(NULL, url);\n \twalker = get_http_walker(url);\n \twalker->get_tree = get_tree;\n \twalker->get_history = get_history;\ndiff --git a/http-push.c b/http-push.c\nindex 6e8f6d0..ecbfae5 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -1747,7 +1747,6 @@ int main(int argc, char **argv)\n \tint i;\n \tint new_refs;\n \tstruct ref *ref, *local_refs;\n-\tstruct remote *remote;\n \n \tgit_extract_argv0_path(argv[0]);\n \n@@ -1821,14 +1820,7 @@ int main(int argc, char **argv)\n \n \tmemset(remote_dir_exists, -1, 256);\n \n-\t/*\n-\t * Create a minimum remote by hand to give to http_init(),\n-\t * primarily to allow it to look at the URL.\n-\t */\n-\tremote = xcalloc(sizeof(*remote), 1);\n-\tALLOC_GROW(remote->url, remote->url_nr + 1, remote->url_alloc);\n-\tremote->url[remote->url_nr++] = repo->url;\n-\thttp_init(remote);\n+\thttp_init(NULL, repo->url);\n \n #ifdef USE_CURL_MULTI\n \tis_running_queue = 0;\ndiff --git a/http.c b/http.c\nindex 149e116..b181c8a 100644\n--- a/http.c\n+++ b/http.c\n@@ -367,7 +367,7 @@ static void set_from_env(const char **var, const char *envname)\n \t\t*var = val;\n }\n \n-void http_init(struct remote *remote)\n+void http_init(struct remote *remote, const char *url)\n {\n \tchar *low_speed_limit;\n \tchar *low_speed_time;\n@@ -431,11 +431,11 @@ void http_init(struct remote *remote)\n \tif (getenv(\"GIT_CURL_FTP_NO_EPSV\"))\n \t\tcurl_ftp_no_epsv = 1;\n \n-\tif (remote && remote->url && remote->url[0]) {\n-\t\thttp_auth_init(remote->url[0]);\n+\tif (url) {\n+\t\thttp_auth_init(url);\n \t\tif (!ssl_cert_password_required &&\n \t\t    getenv(\"GIT_SSL_CERT_PASSWORD_PROTECTED\") &&\n-\t\t    !prefixcmp(remote->url[0], \"https://\"))\n+\t\t    !prefixcmp(url, \"https://\"))\n \t\t\tssl_cert_password_required = 1;\n \t}\n \ndiff --git a/http.h b/http.h\nindex 0bf8592..3c332a9 100644\n--- a/http.h\n+++ b/http.h\n@@ -86,7 +86,7 @@ extern void add_fill_function(void *data, int (*fill)(void *));\n extern void step_active_slots(void);\n #endif\n \n-extern void http_init(struct remote *remote);\n+extern void http_init(struct remote *remote, const char *url);\n extern void http_cleanup(void);\n \n extern int data_received;\ndiff --git a/remote-curl.c b/remote-curl.c\nindex 6c24ab1..d4d0910 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -850,7 +850,7 @@ int main(int argc, const char **argv)\n \n \turl = strbuf_detach(&buf, NULL);\n \n-\thttp_init(remote);\n+\thttp_init(remote, url);\n \n \tdo {\n \t\tif (strbuf_getline(&buf, stdin, '\\n') == EOF)\n-- \n1.7.7.338.g0156b\n"},{"id":"177643","messageId":"20111014131932.GE7808@sigill.intra.peff.net","threadId":"28567","inReplyTo":"cover.1318577792.git.git@drmicha.warpmail.net","subject":"Re: [PATCH 0/6] http-auth-early","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-10-14T13:19:32Z","receivedAt":"2011-10-14T13:19:32Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Oct 14, 2011 at 09:40:34AM +0200, Michael J Gruber wrote:\n\n> Here are the early parts of Jeff's http-auth-keyring series.\n> It contains only parts which are not using the credential API (which\n> is still under discussion), so that this can go in (and help users)\n> and alleviates the pressure on the credential discussion:\n> \n> Early bits with cleanups to http.c.\n> Cherry-picked bit for improved prompts (\"Username for ...\" etc.)\n> Cherry-pickes bit for using configured pushurls.\n> \n> I tried to pick/resolve in a way which should help rebasing Jeff's series\n> on top of this.\n\nThanks for working on this. One of my intended tasks for today is to\nrebase my series, so it is nice to wake up to half of the work done. :)\n\n> Jeff King (5):\n>   url: decode buffers that are not NUL-terminated\n>   improve httpd auth tests\n>   remote-curl: don't retry auth failures with dumb protocol\n>   http: retry authentication failures for all http requests\n>   http_init: accept separate URL parameter\n> \n> Michael J Gruber (1):\n>   http: use hostname in credential description\n\nYour changes all look right. The naming of git_getpass_one in the\ncherry-picked commit is a little odd without the rest of the series as\ncontext. I would maybe have called it \"git_getpass_with_description\" or\nsomething.\n\n-Peff\n"},{"id":"177644","messageId":"4E9837F2.4050404@drmicha.warpmail.net","threadId":"28567","inReplyTo":"20111014131932.GE7808@sigill.intra.peff.net","subject":"Re: [PATCH 0/6] http-auth-early","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-10-14T13:24:02Z","receivedAt":"2011-10-14T13:24:02Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Jeff King venit, vidit, dixit 14.10.2011 15:19:\n> On Fri, Oct 14, 2011 at 09:40:34AM +0200, Michael J Gruber wrote:\n> \n>> Here are the early parts of Jeff's http-auth-keyring series.\n>> It contains only parts which are not using the credential API (which\n>> is still under discussion), so that this can go in (and help users)\n>> and alleviates the pressure on the credential discussion:\n>>\n>> Early bits with cleanups to http.c.\n>> Cherry-picked bit for improved prompts (\"Username for ...\" etc.)\n>> Cherry-pickes bit for using configured pushurls.\n>>\n>> I tried to pick/resolve in a way which should help rebasing Jeff's series\n>> on top of this.\n> \n> Thanks for working on this. One of my intended tasks for today is to\n> rebase my series, so it is nice to wake up to half of the work done. :)\n\nGood morning :)\n\n>> Jeff King (5):\n>>   url: decode buffers that are not NUL-terminated\n>>   improve httpd auth tests\n>>   remote-curl: don't retry auth failures with dumb protocol\n>>   http: retry authentication failures for all http requests\n>>   http_init: accept separate URL parameter\n>>\n>> Michael J Gruber (1):\n>>   http: use hostname in credential description\n> \n> Your changes all look right. The naming of git_getpass_one in the\n> cherry-picked commit is a little odd without the rest of the series as\n> context. I would maybe have called it \"git_getpass_with_description\" or\n> something.\n\ngit_getpass_my_life_will_be_short_and_ended_by_credentials\n\nI don't care. In fact, I wasn't sure whether I should I even change the\nauthor on this one. It's not a straight resolution and does involve\nchoices, but the meat is from your series.\n\nMichael\n"},{"id":"177665","messageId":"7vk487z9fr.fsf@alter.siamese.dyndns.org","threadId":"28567","inReplyTo":"20111014131932.GE7808@sigill.intra.peff.net","subject":"Re: [PATCH 0/6] http-auth-early","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-14T18:59:04Z","receivedAt":"2011-10-14T18:59:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> Your changes all look right. The naming of git_getpass_one in the\n> cherry-picked commit is a little odd without the rest of the series as\n> context. I would maybe have called it \"git_getpass_with_description\" or\n> something.\n\nThanks both; will queue with that updated function name.\n"}]}