On 2020-04-22 at 01:28:17, Jonathan Nieder wrote: > brian m. carlson wrote: > > On 2020-04-21 at 22:58:37, Jeff King wrote: > > >> This is unrelated to the recent helper fixes in v2.26.x. Here's a simple > >> reproduction: > >> > >> url=https://git.example.com/my-proj/my-repo.git > >> echo url=$url | > >> GIT_TERMINAL_PROMPT=0 \ > >> ./git \ > >> -c credential.helper= \ > >> -c credential.$url.helper='!echo username=foo; echo password=bar;:' \ > >> credential fill > >> > >> which should print a filled credential (with "foo/bar"), but will fail > >> with recent versions. It bisects to brian's 46fd7b3900 (credential: > >> allow wildcard patterns when matching config, 2020-02-20). > > > > Yeah, I can reproduce this. It looks like what's happening is that > > we're percent-encoding the slash in the paths as %2f, which of course > > isn't going to match in the urlmatch code. We probably need to tell the > > percent encoding function not to encode slashes in this case. > > > > I'm testing a patch now and hope to have it on the list a little later > > this evening. Thanks for reporting and bisecting, and sorry for the > > breakage. > > Thanks. Here's another (though I haven't tried bisecting yet): > > echo url='https://github.com/git/git' | > GIT_TERMINAL_PROMPT=0 \ > git -c credential.helper= \ > -c credential.github.com.helper='!echo username=foo; echo password=bar;:' \ > credential fill gitcredentials(7) says the following: Git considers each credential to have a context defined by a URL. This context is used to look up context-specific configuration, and is passed to any helpers, which may use it as an index into secure storage. I'm not sure a hostname qualifies as a URL in this case. So while my patch did break this, I don't believe it's ever been documented to actually work and was an artifact of our implementation (along with "credential./git/git.helper" and "credential.https://.helper";). I've also never seen this syntax used in the wild, but maybe I'm not looking in the right places. I don't think we can shoehorn it into urlmatch, since that would break compatibility with the `http.*` config options, so I think we'd have to revert the entire feature if we want to preserve it. I think I'd prefer to leave things as it is since it seems uncommon and there are easy alternatives, but if folks prefer, I can send a patch to revert the urlmatch feature. I will likely not be online tomorrow (Wednesday), so if folks decide they want a revert, I can send that out Thursday afternoon (GMT-05:00). -- brian m. carlson: Houston, Texas, US OpenPGP: https://keybase.io/bk2204
Attachment:
signature.asc
Description: PGP signature
