Re: [PATCH] wincred: fix get credential if username has @
Sorry for the extremely delayed reply, I had a bug in my mail-filters. Hopefully fixed now.
On Wed, Nov 19, 2014 at 11:41 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 17 quoted lines
> Aleksey Vasenev <margtu-fivt@ya.ru> writes:
>
>>> To: git@vger.kernel.org
>>> Cc: Junio C Hamano <gitster@pobox.com>, Aleksey Vasenev <margtu-fivt@ya.ru>
>
> Sorry, but I am hardly qualified to review this one, especially
> without any log message that explains what breaks and how it breaks
> with the current code, which may lead the reader to understand how
> the updated code fixes the issue. Cc'ing me does not help us very
> much.
>
> $ git shortlog --no-merges -n contrib/credential/wincred/
>
> gives me a few names who may be able to give us some inputs, so I'll
> Cc them.
>
> Thanks.
I noticed the breakage myself around the same time, and posted about it here:
https://groups.google.com/d/msg/msysgit/YVuCqmwwRyY/HULHj5OoE88J
Unfortunately, it stopped there.
Show 23 quoted lines
>> Signed-off-by: Aleksey Vasenev <margtu-fivt@ya.ru>
>> ---
>> .../credential/wincred/git-credential-wincred.c | 25 +++++++++++++++++++---
>> 1 file changed, 22 insertions(+), 3 deletions(-)
>>
>> diff --git a/contrib/credential/wincred/git-credential-wincred.c b/contrib/credential/wincred/git-credential-wincred.c
>> index a1d38f0..0061340 100644
>> --- a/contrib/credential/wincred/git-credential-wincred.c
>> +++ b/contrib/credential/wincred/git-credential-wincred.c
>> @@ -111,14 +111,23 @@ static void write_item(const char *what, LPCWSTR wbuf, int wlen)
>> * Match an (optional) expected string and a delimiter in the target string,
>> * consuming the matched text by updating the target pointer.
>> */
>> -static int match_part(LPCWSTR *ptarget, LPCWSTR want, LPCWSTR delim)
>> +
>> +static LPCWSTR wcsstr_last(LPCWSTR str, LPCWSTR find)
>> +{
>> + LPCWSTR res = NULL, pos;
>> + for (pos = wcsstr(str, find); pos; pos = wcsstr(pos + 1, find))
>> + res = pos;
>> + return res;
>> +}
>> +Ugh, there's no wcsrstr? I guess this is a reasonable way to emulate it...
Show 38 quoted lines
>> +static int match_part_with_last(LPCWSTR *ptarget, LPCWSTR want, LPCWSTR delim, int last)
>> {
>> LPCWSTR delim_pos, start = *ptarget;
>> int len;
>>
>> /* find start of delimiter (or end-of-string if delim is empty) */
>> if (*delim)
>> - delim_pos = wcsstr(start, delim);
>> + delim_pos = last ? wcsstr_last(start, delim) : wcsstr(start, delim);
>> else
>> delim_pos = start + wcslen(start);
>>
>> @@ -138,6 +147,16 @@ static int match_part(LPCWSTR *ptarget, LPCWSTR want, LPCWSTR delim)
>> return !want || (!wcsncmp(want, start, len) && !want[len]);
>> }
>>
>> +static int match_part(LPCWSTR *ptarget, LPCWSTR want, LPCWSTR delim)
>> +{
>> + return match_part_with_last(ptarget, want, delim, 0);
>> +}
>> +
>> +static int match_part_last(LPCWSTR *ptarget, LPCWSTR want, LPCWSTR delim)
>> +{
>> + return match_part_with_last(ptarget, want, delim, 1);
>> +}
>> +
>> static int match_cred(const CREDENTIALW *cred)
>> {
>> LPCWSTR target = cred->TargetName;
>> @@ -146,7 +165,7 @@ static int match_cred(const CREDENTIALW *cred)
>>
>> return match_part(&target, L"git", L":") &&
>> match_part(&target, protocol, L"://") &&
>> - match_part(&target, wusername, L"@") &&
>> + match_part_last(&target, wusername, L"@") &&
>> match_part(&target, host, L"/") &&
>> match_part(&target, path, L"");
>> }Looks reasonable enough to me.
Acked-by: Erik Faye-Lund <kusmabite@gmail.com>
Aleksey Vasenev
