Re: [PATCH 3/3] mingw: use domain information for default email
Hi Eric,
On Mon, 15 Oct 2018, Eric Sunshine wrote:
> On Mon, Oct 15, 2018 at 5:47 AM Johannes Schindelin via GitGitGadget
> wrote:
> > When a user is registered in a Windows domain, it is really easy to
> > obtain the email address. So let's do that.
> > [...]
> > Signed-off-by: Johannes Schindelin
> > ---
> > diff --git a/compat/mingw.c b/compat/mingw.c
> > @@ -1826,6 +1826,11 @@ static char *get_extended_user_info(enum
> > EXTENDED_NAME_FORMAT type)
> > +char *mingw_query_user_email(void)
> > +{
> > + return get_extended_user_info(NameUserPrincipal);
> > +}
> > diff --git a/ident.c b/ident.c
> > @@ -168,6 +168,9 @@ const char *ident_default_email(void)
> > + } else if ((email = query_user_email()) && email[0]) {
> > + strbuf_addstr(_default_email, email);
> > + free((char *)email);
>
> If query_user_email(), which calls get_extended_user_info(), returns
> NULL, then we do nothing (and don't worry about freeing the result).
> However, if query_user_email() returns a zero-length allocated string,
> then this conditional will leak that string (due to the email[0]
> check). From patch 2/3, we see in get_extended_user_info():
>
> +static char *get_extended_user_info(enum EXTENDED_NAME_FORMAT type)
> +{
> + if (GetUserNameExW(type, wbuffer, )) {
> + char *converted = xmalloc((len *= 3));
> + if (xwcstoutf(converted, wbuffer, len) >= 0)
> + return converted;
>
> that it may possibly return a zero-length string (due to the ">=0").
> Do we care about this potential leak (or is GetUserNameExW()
> guaranteed never to return such a string)?
Quite honestly, I think that this is so rare an instance (if it *can*
happen at all) that I simply don't care ;-)
Ciao,
Dscho
Re: [PATCH 3/3] mingw: use domain information for default email
"Johannes Schindelin via GitGitGadget"
writes:
> +char *mingw_query_user_email(void)
> +{
> + return get_extended_user_info(NameUserPrincipal);
> +}
> +
> ...
>
> +#ifndef query_user_email
> +#define query_user_email() NULL
> +#endif
The three patches look sensible to me; will queue.
You may already have audited our use of "struct passwd",
"getpwnam()" and "getpwuid()"--I haven't. I think we use these only
to learn user's email (to be used as the default ident) and home
directory (to expand "git config --type=path"). If that is really
the case, it may be a worthwhile clean-up to introduce our own API
that offers these two exact functions, have the per-platform
implementation of the API in compat/, and get rid of "struct passwd"
and calls to getpw*() functions out of the core Git proper, to wean
ourselves away from depending on POSIX too much.
But of course that is a separate topic.
Re: [PATCH 3/3] mingw: use domain information for default email
On Mon, Oct 15, 2018 at 5:47 AM Johannes Schindelin via GitGitGadget
wrote:
> When a user is registered in a Windows domain, it is really easy to
> obtain the email address. So let's do that.
> [...]
> Signed-off-by: Johannes Schindelin
> ---
> diff --git a/compat/mingw.c b/compat/mingw.c
> @@ -1826,6 +1826,11 @@ static char *get_extended_user_info(enum
> EXTENDED_NAME_FORMAT type)
> +char *mingw_query_user_email(void)
> +{
> + return get_extended_user_info(NameUserPrincipal);
> +}
> diff --git a/ident.c b/ident.c
> @@ -168,6 +168,9 @@ const char *ident_default_email(void)
> + } else if ((email = query_user_email()) && email[0]) {
> + strbuf_addstr(_default_email, email);
> + free((char *)email);
If query_user_email(), which calls get_extended_user_info(), returns
NULL, then we do nothing (and don't worry about freeing the result).
However, if query_user_email() returns a zero-length allocated string,
then this conditional will leak that string (due to the email[0]
check). From patch 2/3, we see in get_extended_user_info():
+static char *get_extended_user_info(enum EXTENDED_NAME_FORMAT type)
+{
+ if (GetUserNameExW(type, wbuffer, )) {
+ char *converted = xmalloc((len *= 3));
+ if (xwcstoutf(converted, wbuffer, len) >= 0)
+ return converted;
that it may possibly return a zero-length string (due to the ">=0").
Do we care about this potential leak (or is GetUserNameExW()
guaranteed never to return such a string)?
[PATCH 3/3] mingw: use domain information for default email
From: Johannes Schindelin
When a user is registered in a Windows domain, it is really easy to
obtain the email address. So let's do that.
Suggested by Lutz Roeder.
Signed-off-by: Johannes Schindelin
---
compat/mingw.c| 5 +
compat/mingw.h| 2 ++
git-compat-util.h | 4
ident.c | 3 +++
4 files changed, 14 insertions(+)
diff --git a/compat/mingw.c b/compat/mingw.c
index 623ff5daf5..44264fe3fd 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1826,6 +1826,11 @@ static char *get_extended_user_info(enum
EXTENDED_NAME_FORMAT type)
return NULL;
}
+char *mingw_query_user_email(void)
+{
+ return get_extended_user_info(NameUserPrincipal);
+}
+
struct passwd *getpwuid(int uid)
{
static unsigned initialized;
diff --git a/compat/mingw.h b/compat/mingw.h
index 571019d0bd..f31dcff2be 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -424,6 +424,8 @@ static inline void convert_slashes(char *path)
int mingw_offset_1st_component(const char *path);
#define offset_1st_component mingw_offset_1st_component
#define PATH_SEP ';'
+extern char *mingw_query_user_email(void);
+#define query_user_email mingw_query_user_email
#if !defined(__MINGW64_VERSION_MAJOR) && (!defined(_MSC_VER) || _MSC_VER <
1800)
#define PRIuMAX "I64u"
#define PRId64 "I64d"
diff --git a/git-compat-util.h b/git-compat-util.h
index 5f2e90932f..71779cb0ae 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -382,6 +382,10 @@ static inline char *git_find_last_dir_sep(const char *path)
#define find_last_dir_sep git_find_last_dir_sep
#endif
+#ifndef query_user_email
+#define query_user_email() NULL
+#endif
+
#if defined(__HP_cc) && (__HP_cc >= 61000)
#define NORETURN __attribute__((noreturn))
#define NORETURN_PTR
diff --git a/ident.c b/ident.c
index 327abe557f..33bcf40644 100644
--- a/ident.c
+++ b/ident.c
@@ -168,6 +168,9 @@ const char *ident_default_email(void)
strbuf_addstr(_default_email, email);
committer_ident_explicitly_given |= IDENT_MAIL_GIVEN;
author_ident_explicitly_given |= IDENT_MAIL_GIVEN;
+ } else if ((email = query_user_email()) && email[0]) {
+ strbuf_addstr(_default_email, email);
+ free((char *)email);
} else
copy_email(xgetpwuid_self(_email_is_bogus),
_default_email, _email_is_bogus);
--
gitgitgadget
