On Thu, Aug 30, 2012 at 8:27 PM, Erik Faye-Lund <kusmabite@xxxxxxxxx> wrote:
> On Mon, Aug 27, 2012 at 12:04 AM, Philipp A. Hartmann <pah@xxxxx> wrote:
>> From: "Philipp A. Hartmann" <pah@xxxxx>
>>
>> This patch is an experiment to port the wincred helper
>> to the generic implementation. As of know, it is
>> completely untested.
>>
>> In addition to porting the helper to the generic API,
>> this patch clears up all passwords from memory, which
>> reduces the total amount to saved lines.
>>
>> Signed-off-by: Philipp A. Hartmann <pah@xxxxx>
>> ---
>>
>> The porting was fairly easy, but due to the lack of a testing
>> platform, it is completely untested.
>>
>> Erik: Can you try to have look?
>
> Sorry for the late reply, I'm currently in bed with pneumonia.
>
> But I gave it a quick go, but as-is it's a NACK; a wall of warnings and errors.
>
> But with this patch on top, it seems to at least compile OK:
>
> ---8<---
> diff --git a/contrib/credential/helper/credential_helper.h
> b/contrib/credential/helper/credential_helper.h
> index 7e73fc6..13b611e 100644
> --- a/contrib/credential/helper/credential_helper.h
> +++ b/contrib/credential/helper/credential_helper.h
> @@ -125,6 +125,7 @@ static inline char *xstrdup(const char *str)
> return ret;
> }
>
> +#ifndef NO_STRNDUP
> static inline char *xstrndup(const char *str, size_t len)
> {
> char *ret = strndup(str,len);
> @@ -133,5 +134,6 @@ static inline char *xstrndup(const char *str, size_t len)
>
> return ret;
> }
> +#endif
>
> #endif /* CREDENTIAL_HELPER_H_INCLUDED_ */
> diff --git a/contrib/credential/wincred/Makefile
> b/contrib/credential/wincred/Makefile
> index ee7a8ef..3900322 100644
> --- a/contrib/credential/wincred/Makefile
> +++ b/contrib/credential/wincred/Makefile
> @@ -1,9 +1,10 @@
> MAIN:=git-credential-wincred
> -all:: $(MAIN)
> +all:: $(MAIN).exe
>
> CC = gcc
> RM = rm -f
> CFLAGS = -O2 -Wall
> +CPPFLAGS = -DNO_STRNDUP
>
> -include ../../../config.mak.autogen
> -include ../../../config.mak
> diff --git a/contrib/credential/wincred/git-credential-wincred.c
> b/contrib/credential/wincred/git-credential-wincred.c
> index 721e59f..e26ba47 100644
> --- a/contrib/credential/wincred/git-credential-wincred.c
> +++ b/contrib/credential/wincred/git-credential-wincred.c
> @@ -6,6 +6,7 @@
> #include <stdio.h>
> #include <io.h>
> #include <fcntl.h>
> +#include <credential_helper.h>
>
> /* MinGW doesn't have wincred.h, so we need to define stuff */
>
> @@ -124,9 +125,8 @@ static int prepare_credential(struct credential *c)
> wusername = utf8_to_utf16_dup(c->username);
> if (c->password)
> wpassword = utf8_to_utf16_dup(c->password);
> - if (c->port) {
> - snprintf(port_buf,"%hd",c->port);
> - }
> + if (c->port)
> + snprintf(port_buf, sizeof(port_buf), "%hd", c->port);
> return EXIT_SUCCESS;
> }
>
> @@ -170,7 +170,7 @@ static int get_credential(struct credential *c)
>
> /* search for the first credential that matches username */
> for (i = 0; i < num_creds; ++i)
> - if (match_cred(creds[i])) {
> + if (match_cred(creds[i], c)) {
> cred = creds[i];
> break;
> }
> ---8<---
>
> However, I haven't been able to successfully run the tests on the
> result. When I did, I got this error:
>
> ---8<---
> rm: cannot remove `t/trash directory.t0303-credential-external/stderr': Permissi
> on denied
> rm: cannot remove `t/trash directory.t0303-credential-external/stdout': Permissi
> on denied
> rm: cannot remove directory `t/trash directory.t0303-credential-external': Direc
> tory not empty
> ---8<---
>
> And I'm not currently feeling up to debugging stuck processes or whatever it is.
OK, that was due to a stuck process, and after killing it I got to
test properly. However, three tests fail now:
$ (cd t && GIT_TEST_CREDENTIAL_HELPER=wincred ./t0303-credential-external.sh)
ok 1 - helper (wincred) has no existing data
ok 2 - helper (wincred) stores password
not ok - 3 helper (wincred) can retrieve password
#
# check fill $HELPER <<-\EOF
# protocol=https
# host=example.com
# --
# protocol=https
# host=example.com
# username=store-user
# password=store-pass
# --
# EOF
#
ok 4 - helper (wincred) requires matching protocol
ok 5 - helper (wincred) requires matching host
ok 6 - helper (wincred) requires matching username
ok 7 - helper (wincred) requires matching path
ok 8 - helper (wincred) can forget host
not ok - 9 helper (wincred) can store multiple users
#
# check approve $HELPER <<-\EOF &&
# protocol=https
# host=example.com
# username=user1
# password=pass1
# EOF
# check approve $HELPER <<-\EOF &&
# protocol=https
# host=example.com
# username=user2
# password=pass2
# EOF
# check fill $HELPER <<-\EOF &&
# protocol=https
# host=example.com
# username=user1
# --
# protocol=https
# host=example.com
# username=user1
# password=pass1
# EOF
# check fill $HELPER <<-\EOF
# protocol=https
# host=example.com
# username=user2
# --
# protocol=https
# host=example.com
# username=user2
# password=pass2
# EOF
#
ok 10 - helper (wincred) can forget user
not ok - 11 helper (wincred) remembers other user
#
# check fill $HELPER <<-\EOF
# protocol=https
# host=example.com
# username=user2
# --
# protocol=https
# host=example.com
# username=user2
# password=pass2
# EOF
#
# skipping timeout tests (GIT_TEST_CREDENTIAL_HELPER_TIMEOUT not set)
# failed 3 among 11 test(s)
1..11
So, something else is up as well.
--
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