From 27fd33b1d33516763e8b01dbc10c24ee116be26c Mon Sep 17 00:00:00 2001 From: wildart Date: Wed, 11 May 2016 11:27:11 -0400 Subject: [PATCH] fixed errors, typos & deprecated calls added comments and documentation --- base/libgit2/callbacks.jl | 14 ++++++++------ base/libgit2/types.jl | 22 ++++++++++++++-------- base/libgit2/utils.jl | 3 ++- test/libgit2.jl | 28 ++++++++++++++-------------- 4 files changed, 38 insertions(+), 29 deletions(-) diff --git a/base/libgit2/callbacks.jl b/base/libgit2/callbacks.jl index 82e843bb9361b..1f4b6b76ff5ba 100644 --- a/base/libgit2/callbacks.jl +++ b/base/libgit2/callbacks.jl @@ -35,6 +35,8 @@ In addition, if payload has field `used` it can be set to `true` to indicate tha Order of credential checks (if supported): - ssh key pair (ssh-agent if specified in `payload`) - plain text + +**Note**: Due to the specifics of `libgit2` authentication procedure, when authentication is failed, this functions is called again without any indication whether authentication was successful or not. In order not to stuck in infinite loop by repeatedly using same faulty credentials, use a call counting strategy to counteract this behaviour. """ function credentials_callback(cred::Ptr{Ptr{Void}}, url_ptr::Cstring, username_ptr::Cstring, @@ -62,17 +64,17 @@ function credentials_callback(cred::Ptr{Ptr{Void}}, url_ptr::Cstring, if isset(allowed_types, Cuint(Consts.CREDTYPE_SSH_KEY)) credid = "ssh://$host" # first try ssh-agent if credentials support its usage - if (isdefined(creds, :use_ssh_agent) ? getfield(creds, :use_ssh_agent) : 0xFF) == 0x00 + if isdefined(creds, :use_ssh_agent) && getfield(creds, :use_ssh_agent) == 0x00 err = ccall((:git_cred_ssh_key_from_agent, :libgit2), Cint, (Ptr{Ptr{Void}}, Cstring), cred, username_ptr) - setfield!(creds, :use_ssh_agent, 0x01) # used ssh-agent ones + setfield!(creds, :use_ssh_agent, 0x01) # used ssh-agent only one time err == 0 && return Cint(0) end errcls, errmsg = Error.last_error() if errcls != Error.None # Check if we used ssh-agent - if (isdefined(creds, :use_ssh_agent) ? getfield(creds, :use_ssh_agent) : 0xFF) == 0x01 + if isdefined(creds, :use_ssh_agent) && getfield(creds, :use_ssh_agent) == 0x01 println("ERROR: $errmsg ssh-agent") setfield!(creds, :use_ssh_agent, 0x02) # reported ssh-agent error else @@ -83,7 +85,7 @@ function credentials_callback(cred::Ptr{Ptr{Void}}, url_ptr::Cstring, # if username is not provided, then prompt for it username = if username_ptr == Cstring(C_NULL) uname = creds[:user, credid] # check if credentials were already used - uname !== nothing && !isusedcreds ? uname : prompt("Username for `$schema$host'") + uname !== nothing && !isusedcreds ? uname : prompt("Username for '$schema$host'") else bytestring(username_ptr) end @@ -98,7 +100,7 @@ function credentials_callback(cred::Ptr{Ptr{Void}}, url_ptr::Cstring, keydef # use cached value else keydef = homedir()*"/.ssh/id_rsa.pub" - prompt("Public key location for `$schema$username@$host'", default=keydef) + prompt("Public key location for '$schema$username@$host'", default=keydef) end end creds[:pubkey, credid] = publickey # save credentials @@ -111,7 +113,7 @@ function credentials_callback(cred::Ptr{Ptr{Void}}, url_ptr::Cstring, keydef # use cached value else keydef = homedir()*"/.ssh/id_rsa" - prompt("Private key location for `$schema$username@$host'", default=keydef) + prompt("Private key location for '$schema$username@$host'", default=keydef) end end creds[:prvkey, credid] = privatekey # save credentials diff --git a/base/libgit2/types.jl b/base/libgit2/types.jl index e0d7d273b0f1b..085280d4afa06 100644 --- a/base/libgit2/types.jl +++ b/base/libgit2/types.jl @@ -58,7 +58,7 @@ abstract AbstractCredentials <: AbstractPayload "Returns a credentials parameter" function Base.getindex(p::AbstractCredentials, keys...) for k in keys - ks = symbol(k) + ks = Symbol(k) isdefined(p, ks) && return getfield(p, ks) end return nothing @@ -66,14 +66,14 @@ end "Sets credentials with `key` parameter with value" function Base.setindex!(p::AbstractCredentials, val, keys...) for k in keys - ks = symbol(k) + ks = Symbol(k) isdefined(p, ks) && setfield!(p, ks, val) end return nothing end -"Check if credentials were used" +"Checks if credentials were used" isused(p::AbstractCredentials) = true -"Reset credentials for another usage" +"Resets credentials for another usage" reset!(p::AbstractCredentials, cnt::Int=3) = nothing immutable CheckoutOptions @@ -638,15 +638,17 @@ type EmptyCredentials <: AbstractCredentials end type UserPasswordCredentials <: AbstractCredentials user::AbstractString pass::AbstractString - count::Int - use_ssh_agent::UInt8 + count::Int # authentication failure protection count + use_ssh_agent::UInt8 # used for ssh-agent authentication UserPasswordCredentials(u::AbstractString,p::AbstractString) = new(u,p,3,zero(UInt8)) end +"Checks if credentials were used or failed authentication, see `LibGit2.credentials_callback`" function isused(p::UserPasswordCredentials) p.count <= 0 && return true p.count -= 1 return false end +"Resets authentication failure protection count" reset!(p::UserPasswordCredentials, cnt::Int=3) = (p.count = cnt) "Field map for cached credentials type" @@ -655,10 +657,11 @@ const CachedCredentialsFieldMap = Dict(:user=>1, :pass=>2, :pubkey=>3, :prvkey=> "Credentials that support caching" type CachedCredentials <: AbstractCredentials cred::Dict{AbstractString,Vector{AbstractString}} - count::Int - use_ssh_agent::UInt8 + count::Int # authentication failure protection count + use_ssh_agent::UInt8 # used for ssh-agent authentication CachedCredentials() = new(Dict{AbstractString,Vector{AbstractString}}(),3,zero(UInt8)) end +"Returns specific credential parameter value: first index is a credential parameter name, second index is a host name (with schema)" function Base.getindex(p::CachedCredentials, keys...) length(keys) != 2 && return nothing key, host = keys @@ -669,6 +672,7 @@ function Base.getindex(p::CachedCredentials, keys...) end return nothing end +"Sets specific credential parameter value: first index is a credential parameter name, second index is a host name (with schema)" function Base.setindex!(p::CachedCredentials, val, keys...) length(keys) != 2 && return nothing key, host = keys @@ -676,9 +680,11 @@ function Base.setindex!(p::CachedCredentials, val, keys...) haskey(CachedCredentialsFieldMap, key) && return setindex!(p.cred[host], val, CachedCredentialsFieldMap[key]) return nothing end +"Checks if credentials were used or failed authentication, see `LibGit2.credentials_callback`" function isused(p::CachedCredentials) p.count <= 0 && return true p.count -= 1 return false end +"Resets authentication failure protection count" reset!(p::CachedCredentials, cnt::Int=3) = (p.count = cnt) diff --git a/base/libgit2/utils.jl b/base/libgit2/utils.jl index 1f907e7e2bba1..8a0b61a9ac537 100644 --- a/base/libgit2/utils.jl +++ b/base/libgit2/utils.jl @@ -42,7 +42,8 @@ toggle(val::Integer, flag::Integer) = (val |= flag) return "" end -@unix_only getpass(prompt::AbstractString) = bytestring(pointer_to_string(ccall(:getpass, Cstring, (Cstring,), prompt), true)) +@unix_only getpass(prompt::AbstractString) = + bytestring(pointer_to_string(ccall(:getpass, Cstring, (Cstring,), prompt), true)) function prompt(msg::AbstractString; default::AbstractString="", password::Bool=false) msg = !isempty(default) ? msg*" [$default]:" : msg*":" diff --git a/test/libgit2.jl b/test/libgit2.jl index 490d6448608d7..14826fb3de79d 100644 --- a/test/libgit2.jl +++ b/test/libgit2.jl @@ -438,8 +438,8 @@ mktempdir() do dir #@testset "Credentials" begin creds = LibGit2.EmptyCredentials() - @test isused(creds) - @test reset!(creds) === nothing + @test LibGit2.isused(creds) + @test LibGit2.reset!(creds) === nothing @test creds[:user] === nothing @test creds[:pass] === nothing @test creds[:pubkey, "localhost"] === nothing @@ -447,12 +447,12 @@ mktempdir() do dir creds_user = "USER" creds_pass = "PASS" creds = LibGit2.UserPasswordCredentials(creds_user, creds_pass) - @test !isused(creds) - @test !isused(creds) - @test !isused(creds) - @test isused(creds) - @test reset!(creds) == 3 - @test !isused(creds) + @test !LibGit2.isused(creds) + @test !LibGit2.isused(creds) + @test !LibGit2.isused(creds) + @test LibGit2.isused(creds) + @test LibGit2.reset!(creds) == 3 + @test !LibGit2.isused(creds) @test creds.count == 2 @test creds[:user] == creds_user @test creds[:pass] == creds_pass @@ -461,12 +461,12 @@ mktempdir() do dir @test creds[:pubkey, "localhost"] === nothing creds = LibGit2.CachedCredentials() - @test !isused(creds) - @test !isused(creds) - @test !isused(creds) - @test isused(creds) - @test reset!(creds) == 3 - @test !isused(creds) + @test !LibGit2.isused(creds) + @test !LibGit2.isused(creds) + @test !LibGit2.isused(creds) + @test LibGit2.isused(creds) + @test LibGit2.reset!(creds) == 3 + @test !LibGit2.isused(creds) @test creds.count == 2 @test creds[:user, "localhost"] === nothing @test creds[:pass, "localhost"] === nothing