[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] Fix issue #3239

From: Karl Fogel <kfogel_at_red-bean.com>
Date: Mon, 21 Jul 2008 14:56:28 -0400

Senthil Kumaran S <senthil_at_collab.net> writes:
> Hi,
>
> I am attaching a patch along with this email which fixes issue #3239 -
> client cert passphrase cache keyed by relative path, not absolute
> path. This includes a change to the public API
> "svn_cmdline_auth_ssl_client_cert_prompt" though it does not affect
> the existing functionality and the function prototype.
>
> I am not sure whether this is the right way to put such trivial fix
> into the public API, so I am expecting comments whether my change is
> right. I enquired about this in IRC to Arfrever, who asked me to send
> the patch to dev list.

Actually, I think this brings the code into compliance with this API:

   typedef struct svn_auth_cred_ssl_client_cert_t
   {
     /** Full paths to the certificate file */
     const char *cert_file;
     /** Indicates if the credentials may be saved (to disk). For example, a
      * GUI prompt implementation with a remember certificate checkbox shall
      * set @a may_save to TRUE if the checkbox is checked.
      */
     svn_boolean_t may_save;
   } svn_auth_cred_ssl_client_cert_t;

Since svn_cmdline_auth_ssl_client_cert_prompt() just sets the
'cert_file' field in *cred, and that is documented to be the "full
paths", there is no incompatibility here. This can be considered a
bugfix, really.

However, it might be good to change "Full paths" above to "Absolute
path", to be completely clear (and to fix the grammar -- I don't know
why it was plural before). And in the documentation for
svn_cmdline_auth_ssl_client_cert_prompt(), it would be good to state
that an absolute path will be recorded too, even though the receiving
data structure will also say that.

-Karl

> [[[
> Fix issue #3239 - client cert passphrase cache keyed by relative path,
> not absolute path
>
> * subversion/libsvn_subr/prompt.c
> (svn_cmdline_auth_ssl_client_cert_prompt): Get the absolute path of cert
> file and set it in the credentials.
>
> Patch by: stylesen
> ]]]
>
> Thank You.
> --
> Senthil Kumaran S
> http://www.stylesen.org/
>
>
> Index: subversion/libsvn_subr/prompt.c
> ===================================================================
> --- subversion/libsvn_subr/prompt.c (revision 32158)
> +++ subversion/libsvn_subr/prompt.c (working copy)
> @@ -29,6 +29,7 @@
> #include "svn_string.h"
> #include "svn_auth.h"
> #include "svn_error.h"
> +#include "svn_path.h"
>
> #include "svn_private_config.h"
>
> @@ -340,14 +341,16 @@
> {
> svn_auth_cred_ssl_client_cert_t *cred = NULL;
> const char *cert_file = NULL;
> + const char *abs_cert_file = NULL;
> svn_cmdline_prompt_baton2_t *pb = baton;
>
> SVN_ERR(maybe_print_realm(realm, pool));
> SVN_ERR(prompt(&cert_file, _("Client certificate filename: "),
> FALSE, pb, pool));
> + SVN_ERR(svn_path_get_absolute(&abs_cert_file, cert_file, pool));
>
> cred = apr_palloc(pool, sizeof(*cred));
> - cred->cert_file = cert_file;
> + cred->cert_file = abs_cert_file;
> cred->may_save = may_save;
> *cred_p = cred;
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
> For additional commands, e-mail: dev-help_at_subversion.tigris.org

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-07-21 20:56:46 CEST

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.