I have a one-line fix for this issue, which I've tested by hand:
[[[
Index: subversion/libsvn_subr/gpg_agent.c
===================================================================
--- subversion/libsvn_subr/gpg_agent.c (revision 1688253)
+++ subversion/libsvn_subr/gpg_agent.c (working copy)
@@ -232,6 +232,7 @@ find_running_gpg_agent(int *new_sd, apr_
if (!homedir)
return SVN_NO_ERROR;
+ homedir = svn_dirent_canonicalize(homedir, pool);
socket_name = svn_dirent_join_many(pool, homedir, ".gnupg",
"S.gpg-agent", SVN_VA_NULL);
}
]]]
I don't have an automated regression test. Should I go ahead and commit the
fix anyway, and propose for backport to 1.8 and 1.9?
(The bug was introduced in 1.8.11.)
This bug arose because svn_user_get_homedir() returns a non-canonical path
if the environment variable 'HOME' has a non-canonical value. Our APIs
should return canonical paths. A better fix would be to do so, perhaps like
this:
[[[
Index: subversion/include/svn_user.h
===================================================================
--- subversion/include/svn_user.h (revision 1688253)
+++ subversion/include/svn_user.h (working copy)
@@ -45,6 +45,8 @@
* any necessary allocation, returning NULL on error.
*
* @since New in 1.4.
+ * @since 1.10 canonicalizes the result; before that it could return a
+ * non-canonical path.
*/
const char *
svn_user_get_homedir(apr_pool_t *pool);
Index: subversion/libsvn_subr/user.c
===================================================================
--- subversion/libsvn_subr/user.c (revision 1688253)
+++ subversion/libsvn_subr/user.c (working copy)
@@ -30,2 +30,3 @@
#include "svn_utf.h"
+#include "svn_dirent_uri.h"
@@ -71,4 +71,4 @@ svn_user_get_name(apr_pool_t *pool)
-const char *
-svn_user_get_homedir(apr_pool_t *pool)
+static const char *
+user_get_homedir(apr_pool_t *pool)
{
@@ -86,1 +87,12 @@ svn_user_get_homedir(apr_pool_t *pool)
}
+
+const char *
+svn_user_get_homedir(apr_pool_t *pool)
+{
+ const char *homedir = user_get_homedir(pool);
+
+ if (homedir)
+ return svn_dirent_canonicalize(homedir, pool);
+
+ return NULL;
+}
Index: subversion/libsvn_subr/config_file.c
===================================================================
--- subversion/libsvn_subr/config_file.c (revision 1688253)
+++ subversion/libsvn_subr/config_file.c (working copy)
@@ -1407,7 +1407,7 @@ svn_config_get_user_config_path(const ch
if (! homedir)
return SVN_NO_ERROR;
*path = svn_dirent_join_many(pool,
- svn_dirent_canonicalize(homedir, pool),
+ homedir,
SVN_CONFIG__USR_DIRECTORY, fname,
SVN_VA_NULL);
}
#endif /* WIN32 */
]]]
Alternatively, perhaps, bump the API to svn_user_get_homedir2() and leave
the non-canonical version as it is; but I think that's a worse idea.
What does anybody say?
- Julian
On 17 July 2015 at 18:27, <julianfoad_at_tigris.org> wrote:
> http://subversion.tigris.org/issues/show_bug.cgi?id=4584
> Issue #|4584
> Summary|Non-canonical $HOME crashes GPG-agent support code
>
> The 'svn' client can suffer an assertion failure during authentication to
> a server:
>
> svn: subversion/libsvn_subr/dirent_uri.c:1050: svn_dirent_join_many:
> Assertion
> `svn_dirent_is_canonical(base, pool)' failed.
>
> if (at least) the following conditions are true:
>
> * The value of $HOME [1] is non-canonical according to
> svn_dirent_is_canonical()
> but resolves to a valid home directory according to typical
> interpretations.
> For example, '/home/myname/' or '//home/myname/'.
>
> * GPG-agent support is enabled in the 'password-stores' config option.
>
> * The environment variable $GPG_AGENT_INFO is not set.
>
> * Some conditions related to the sequence in which the Subversion client
> gets
> authentication credentials. For example, it can be triggered after a
> correct
> password is entered at a prompt, or if the correct username and
> password are
> given on the command line and the authentication username is not equal
> to
> the OS username.
>
> The problem was seen and reported by a WANdisco customer running Subversion
> 1.8.11 and observing that this was a regression from 1.8.8. In this
> instance,
> the value of $HOME had a trailing slash.
>
> I found that the problem was introduced in Subversion 1.8.11, in the
> function
> find_running_gpg_agent() which calls:
>
> homedir = svn_user_get_homedir(pool);
> ... socket_name = svn_dirent_join_many(pool, homedir, ...);
>
> The only previous caller of svn_user_get_homedir() canonicalizes the result
> before passing it to svn_dirent_join_many(); this new code does not.
>
> [1] If $HOME is not set, then Subversion would instead use the home
> directory
> path reported by the operating system via APR. I have not confirmed
> whether that
> could be non-canonical.
>
Received on 2015-07-20 14:46:06 CEST