diff --git a/docs/src/changelog.md b/docs/src/changelog.md index 5314351..d84ac49 100644 --- a/docs/src/changelog.md +++ b/docs/src/changelog.md @@ -15,10 +15,18 @@ Changelog](https://keepachangelog.com). construction ([#21]). - Our [`Base.run()`](@ref) methods now accept plain `String`s as well as `Cmd`s ([#24]). +- Implemented convenience [`Base.read(::String, ::SftpSession)`](@ref) methods + that will take a `String` filename without having to open the file explicitly + ([#25]). +- Added support for specifying whether a [`Session`](@ref) should use the users + SSH config with the `process_config` option ([#25]). ### Fixed - Improved handling of possible errors in [`Base.readdir()`](@ref) ([#20]). +- Fixed exception handling for [`Base.run()`](@ref), now it throws a + [`SshProcessFailedException`](@ref) or [`LibSSHException`](@ref) on command + failure instead of a plain `TaskFailedException` ([#25]). ## [v0.6.0] - 2024-10-11 diff --git a/docs/src/sessions_and_channels.md b/docs/src/sessions_and_channels.md index 34c27dc..da7cd3e 100644 --- a/docs/src/sessions_and_channels.md +++ b/docs/src/sessions_and_channels.md @@ -111,11 +111,11 @@ SshProcessFailedException SshProcess Base.wait(::SshProcess) Base.success(::SshProcess) -Base.run(::Cmd, ::Session) -Base.read(::Cmd, ::Session) -Base.read(::Cmd, ::Session, ::Type{String}) -Base.readchomp(::Cmd, ::Session) -Base.success(::Cmd, ::Session) +Base.run(::Union{Cmd, String}, ::Session) +Base.read(::Union{Cmd, String}, ::Session) +Base.read(::Union{Cmd, String}, ::Session, ::Type{String}) +Base.readchomp(::Union{Cmd, String}, ::Session) +Base.success(::Union{Cmd, String}, ::Session) ``` #### Direct port forwarding diff --git a/docs/src/sftp.md b/docs/src/sftp.md index a8a1997..ec1a702 100644 --- a/docs/src/sftp.md +++ b/docs/src/sftp.md @@ -59,7 +59,9 @@ Base.open(::String, ::SftpSession) Base.open(::Function, ::String, ::SftpSession) Base.close(::SftpFile) Base.read(::SftpFile) +Base.read(::String, ::SftpSession) Base.read(::SftpFile, ::Type{String}) +Base.read(::String, ::SftpSession, ::Type{String}) Base.read!(::SftpFile, ::Vector{UInt8}) Base.write(::SftpFile, ::DenseVector) Base.write(::SftpFile, ::AbstractString) diff --git a/src/channel.jl b/src/channel.jl index 57be5c1..1ca40a8 100644 --- a/src/channel.jl +++ b/src/channel.jl @@ -493,8 +493,18 @@ $(TYPEDSIGNATURES) function Base.wait(process::SshProcess) try wait(process._task) - catch ex - if process.cmd isa Cmd && !process.cmd.ignorestatus + catch task_ex + ex = process._task.exception + + # The idea is that SshProcessFailedException's and LibSSHException's are + # somewhat expected so we always unwrap them from the + # TaskFailedException before throwing, which is a slightly nicer API to + # work with. + if ex isa SshProcessFailedException || ex isa LibSSHException + if !(process.cmd isa Cmd && process.cmd.ignorestatus) + throw(process._task.exception) + end + else rethrow() end end @@ -591,6 +601,8 @@ An easy way of getting around these restrictions is to pass the command as a # Throws - [`SshProcessFailedException`](@ref): if the command fails and `ignorestatus()` wasn't used. +- [`LibSSHException`](@ref): if running the command fails for some other + reason. # Arguments - `cmd`: The command to run. This will be converted to a string for running @@ -652,10 +664,10 @@ function Base.run(cmd::Union{Cmd, String}, session::Session; set_channel_callbacks(process._sshchan, callbacks) process._task = Threads.@spawn _exec_command(process) - errormonitor(process._task) + if wait # Note the use of Base.wait() to avoid aliasing with the `wait` argument - Base.wait(process._task) + Base.wait(process) if print_out print(String(copy(process.out))) diff --git a/src/session.jl b/src/session.jl index 23c1bcc..6d6efef 100644 --- a/src/session.jl +++ b/src/session.jl @@ -26,6 +26,7 @@ mutable struct Session ssh_dir::Union{String, Nothing} known_hosts::Union{String, Nothing} gssapi_server_identity::Union{String, Nothing} + process_config::Bool _lock::ReentrantLock _auth_methods::Union{Vector{AuthMethod}, Nothing} @@ -68,7 +69,7 @@ mutable struct Session lib.ssh_set_blocking(ptr, 0) session = new(ptr, own, [], nothing, - -1, nothing, nothing, nothing, + -1, nothing, nothing, nothing, true, ReentrantLock(), nothing, AuthMethod[], true, Threads.Event(true), CloseableCondition(), false) @@ -137,6 +138,13 @@ $(TYPEDSIGNATURES) Constructor for creating a client session. Use this if you want to connect to a server. +!!! warning + By default libssh will try to follow the settings in any found SSH config + files. If a proxyjump is configured for `host` libssh will try to set up the + proxy itself, which usually does not play well with Julia's event loop. In + such situations you will probably want to pass `process_config=false` and + set up the proxyjump explicitly using a [`Forwarder`](@ref). + # Throws - [`LibSSHException`](@ref): if a session couldn't be created, or there was an error initializing the `user` property. @@ -152,6 +160,7 @@ server. - `log_verbosity=nothing`: Set the log verbosity for the session. - `auto_connect=true`: Whether to automatically call [`connect()`](@ref). +- `process_config=true`: Whether to process any found SSH config files. # Examples @@ -163,7 +172,8 @@ julia> session = ssh.Session(ip"12.34.56.78", 2222) """ function Session(host::Union{AbstractString, Sockets.IPAddr}, port=22; socket::Union{Sockets.TCPSocket, RawFD, Nothing}=nothing, - user=nothing, log_verbosity=nothing, auto_connect=true) + user=nothing, log_verbosity=nothing, auto_connect=true, + process_config=true) session_ptr = lib.ssh_new() if session_ptr == C_NULL throw(LibSSHException("Could not initialize Session for host $(host)")) @@ -192,6 +202,8 @@ function Session(host::Union{AbstractString, Sockets.IPAddr}, port=22; session.user = user end + session.process_config = process_config + if auto_connect connect(session) end @@ -295,10 +307,11 @@ const SESSION_PROPERTY_OPTIONS = Dict(:host => (SSH_OPTIONS_HOST, Cstring), :ssh_dir => (SSH_OPTIONS_SSH_DIR, Cstring), :known_hosts => (SSH_OPTIONS_KNOWNHOSTS, Cstring), :gssapi_server_identity => (SSH_OPTIONS_GSSAPI_SERVER_IDENTITY, Cstring), - :log_verbosity => (SSH_OPTIONS_LOG_VERBOSITY, Cuint)) + :log_verbosity => (SSH_OPTIONS_LOG_VERBOSITY, Cuint), + :process_config => (SSH_OPTIONS_PROCESS_CONFIG, Bool)) # These properties cannot be retrieved from the libssh API (i.e. with # ssh_options_get()), so we store them in the Session object instead. -const SAVED_PROPERTIES = (:log_verbosity, :gssapi_server_identity, :ssh_dir, :known_hosts) +const SAVED_PROPERTIES = (:log_verbosity, :gssapi_server_identity, :ssh_dir, :known_hosts, :process_config) function Base.propertynames(::Session, private::Bool=false) fields = fieldnames(Session) diff --git a/src/sftp.jl b/src/sftp.jl index 0c1f830..f735502 100644 --- a/src/sftp.jl +++ b/src/sftp.jl @@ -142,8 +142,8 @@ mutable struct SftpSession - [`LibSSHException`](@ref): If creating the SFTP session fails. """ function SftpSession(session::Session) - if !isopen(session) - throw(ArgumentError("Session is closed, cannot create an SftpSession with it")) + if !isconnected(session) + throw(ArgumentError("Session is disconnected, cannot create an SftpSession with it")) end ptr = @lockandblock session lib.sftp_new(session.ptr) @@ -907,6 +907,9 @@ function Base.read(file::SftpFile, nb::Integer=typemax(Int)) return out end +"$(TYPEDSIGNATURES)" +Base.read(filename::AbstractString, sftp::SftpSession) = open(read, filename, sftp) + """ $(TYPEDSIGNATURES) @@ -973,6 +976,10 @@ Read the whole file as a `String`. """ Base.read(file::SftpFile, ::Type{String}) = String(read(file)) +"$(TYPEDSIGNATURES)" +Base.read(filename::AbstractString, sftp::SftpSession, ::Type{String}) = String(read(filename, sftp)) + + """ $(TYPEDSIGNATURES) diff --git a/test/LibSSHTests.jl b/test/LibSSHTests.jl index f7898d7..fa6f795 100644 --- a/test/LibSSHTests.jl +++ b/test/LibSSHTests.jl @@ -355,6 +355,8 @@ end session.gssapi_server_identity = "foo.com" @test session.gssapi_server_identity == "foo.com" @test session.fd == RawFD(-1) + session.process_config = false + @test !session.process_config # Test setting an initial user ssh.Session("localhost"; user="foo", auto_connect=false) do session2 @@ -562,16 +564,8 @@ end cmd = setenv(`echo \$foo`, "foo" => "bar") @test readchomp(cmd, session) == "bar" - # Test command failure. We redirect error output to devnull to hide - # the errors displayed by the errormonitor() task. - try - redirect_stderr(devnull) do - run(`foo`, session) - end - catch ex - @test ex isa TaskFailedException - @test current_exceptions(ex.task)[1][1] isa ssh.SshProcessFailedException - end + # Test command failure + @test_throws ssh.SshProcessFailedException run(`foo`, session) # Test passing a String instead of a Cmd mktempdir() do tmpdir @@ -773,6 +767,10 @@ end # Shouldn't be able to read from a closed file close(file) @test_throws ArgumentError read(file) + + # Test reading by passing just a filename + write(path, msg) + @test read(path, sftp, String) == msg end end end