Re: [PATCH] wincred: fix get credential if username has @

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxx> wrote:
> Aleksey Vasenev <margtu-fivt@xxxxx> writes:
>
>>> To: git@xxxxxxxxxxxxxxx
>>> Cc: Junio C Hamano <gitster@xxxxxxxxx>, Aleksey Vasenev <margtu-fivt@xxxxx>
>
> 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.

>> Signed-off-by: Aleksey Vasenev <margtu-fivt@xxxxx>
>> ---
>>  .../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...

>> +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@xxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]