Re: [PATCH] git-svn: enable platform-specific authentication

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

 



Hey folks,

I sent the below patch a few months ago, and not having it applied in
git-svn bit me again just now. Did any of you get a chance to have a
look at it?

I'm still not 100% sure if this patch is correct for all the corner
cases, but it works like a charm in the regular case.

Perhaps it should just be included as is?

Gr.

Matthijs

On Tue, Aug 09, 2011 at 11:06:38PM +0200, Matthijs Kooijman wrote:
> Hey folks,
> 
> > > Use the platform-specific authentication providers that are
> > > exposed to subversion bindings starting with subversion 1.6.
> > 
> > This came up several months ago, I understand there were some issues
> > with the SVN Perl bindings.  Cc-ing interested parties.
> I missed the CC, sorry for that.
> 
> > >  sub _auth_providers () {
> > >  	[
> > > +	  $SVN::Core::VERSION lt '1.6' ? () :
> > > +	    @{SVN::Core::auth_get_platform_specific_client_providers(
> > > +	      undef,undef)},
> > 
> > I think it needs to take into account the config from
> > SVN::Core::config_get_config, otherwise people with non-standard SVN
> > configurations could get locked out.  I seem to recall this was the
> > broken part in the SVN Perl bindings, but one of the Cc-ed parties would
> > know for sure.
> 
> Indeed, but a proposed patch by Eric for this did not work. I solved the
> problem quite some time ago, but apparently I never sent out the
> solution (I think I got distracted by trying to get a passphrase prompt
> to unlock locked keychains). I couldn't find my fixes anymore either,
> but I think I've managed to reproduce them just now.
> 
> Some basic testing shows below patch works, but I think it might need
> some more testing and work. At least the below patch allows for example
> to disable the gnome-keyring provider from a different svn config
> directory by passing --config-dir /some/path to git-svn (which is not
> possible using above patch passing undef, which will only read from
> ~/.subversion).
> 
> Using strace, I did notice that git-svn still reads stuff
> from ~/.subversion/auth/svn.ssl.server/ and
> .subversion/auth/svn.simple/, but I couldn't exactly find why this is
> right away. In any case, it also happens without this patch applied, so
> I guess it's a completely separate issue.
> 
> As for the actual patch, notice that config_get_config returns a hash
> that consists again of a "config" and "servers" patch. Previous attempts
> at this patch passed the entire hash to
> auth_get_platform_specific_client_providers, but it only wants the
> "client" part. It's a bit confusing until you realize that the
> config_get_config return value represents your ~/.subversion directory,
> which again contains a "config" and "servers" file.
> 
> I'm not 100% sure this patch is correct as it is now. I hope to get
> another look at my "automatically unlock keychain" work tomorrow,
> in case there are some hints about flaws in this patch there. In the
> meanwhile, feedback on this patch is welcome.
> 
> Gr.
> 
> Matthijs
> 
> diff --git a/git-svn.perl b/git-svn.perl
> index da3fea8..6dc5196 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -4916,7 +4916,7 @@ BEGIN {
>  }
>  
>  sub _auth_providers () {
> -       [
> +       my @rv = (
>           SVN::Client::get_simple_provider(),
>           SVN::Client::get_ssl_server_trust_file_provider(),
>           SVN::Client::get_simple_prompt_provider(
> @@ -4932,7 +4932,23 @@ sub _auth_providers () {
>             \&Git::SVN::Prompt::ssl_server_trust),
>           SVN::Client::get_username_prompt_provider(
>             \&Git::SVN::Prompt::username, 2)
> -       ]
> +       );
> +
> +       # earlier 1.6.x versions would segfault, and <= 1.5.x didn't have
> +       # this function
> +       if ($SVN::Core::VERSION gt '1.6.12') {
> +               my $config = SVN::Core::config_get_config($config_dir);
> +               my ($p, @a);
> +              # config_get_config returns all config files from
> +              # ~/.subversion, auth_get_platform_specific_client_providers
> +              # just wants the config "file".
> +               @a = ($config->{'config'}, undef);
> +               $p = SVN::Core::auth_get_platform_specific_client_providers(@a);
> +              # Insert the return value from
> +              # auth_get_platform_specific_providers
> +               unshift @rv, @$p;
> +       }
> +       \@rv;
>  }
>  
>  sub escape_uri_only {
> 


Attachment: signature.asc
Description: Digital signature


[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]