You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mina.apache.org by lg...@apache.org on 2018/04/23 07:55:46 UTC
[1/3] mina-sshd git commit: [SSHD-814] Added open failure callback in
SftpEventListener
Repository: mina-sshd
Updated Branches:
refs/heads/master 6cd387daa -> 1bbe4154d
[SSHD-814] Added open failure callback in SftpEventListener
Project: http://git-wip-us.apache.org/repos/asf/mina-sshd/repo
Commit: http://git-wip-us.apache.org/repos/asf/mina-sshd/commit/1bbe4154
Tree: http://git-wip-us.apache.org/repos/asf/mina-sshd/tree/1bbe4154
Diff: http://git-wip-us.apache.org/repos/asf/mina-sshd/diff/1bbe4154
Branch: refs/heads/master
Commit: 1bbe4154d51ecae589535389a107a599163d8417
Parents: 08a774e
Author: Goldstein Lyor <ly...@c-b4.com>
Authored: Mon Apr 23 10:20:51 2018 +0300
Committer: Goldstein Lyor <ly...@c-b4.com>
Committed: Mon Apr 23 10:55:36 2018 +0300
----------------------------------------------------------------------
.../sftp/AbstractSftpSubsystemHelper.java | 12 +++++
.../subsystem/sftp/SftpEventListener.java | 16 +++++++
.../server/subsystem/sftp/SftpSubsystem.java | 49 +++++++++++++-------
3 files changed, 61 insertions(+), 16 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/1bbe4154/sshd-sftp/src/main/java/org/apache/sshd/server/subsystem/sftp/AbstractSftpSubsystemHelper.java
----------------------------------------------------------------------
diff --git a/sshd-sftp/src/main/java/org/apache/sshd/server/subsystem/sftp/AbstractSftpSubsystemHelper.java b/sshd-sftp/src/main/java/org/apache/sshd/server/subsystem/sftp/AbstractSftpSubsystemHelper.java
index bdcb0c5..cc6621e 100644
--- a/sshd-sftp/src/main/java/org/apache/sshd/server/subsystem/sftp/AbstractSftpSubsystemHelper.java
+++ b/sshd-sftp/src/main/java/org/apache/sshd/server/subsystem/sftp/AbstractSftpSubsystemHelper.java
@@ -506,6 +506,18 @@ public abstract class AbstractSftpSubsystemHelper
*/
protected abstract String doOpen(int id, String path, int pflags, int access, Map<String, Object> attrs) throws IOException;
+ protected <E extends IOException> E signalOpenFailure(int id, String pathValue, Path path, boolean isDir, E thrown) throws IOException {
+ SftpEventListener listener = getSftpEventListenerProxy();
+ ServerSession session = getServerSession();
+ if (log.isDebugEnabled()) {
+ log.debug("signalOpenFailure(id={})[{}] signal {} for {}: {}",
+ id, pathValue, thrown.getClass().getSimpleName(), path, thrown.getMessage());
+ }
+
+ listener.openFailed(session, pathValue, path, isDir, thrown);
+ return thrown;
+ }
+
protected void doClose(Buffer buffer, int id) throws IOException {
String handle = buffer.getString();
try {
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/1bbe4154/sshd-sftp/src/main/java/org/apache/sshd/server/subsystem/sftp/SftpEventListener.java
----------------------------------------------------------------------
diff --git a/sshd-sftp/src/main/java/org/apache/sshd/server/subsystem/sftp/SftpEventListener.java b/sshd-sftp/src/main/java/org/apache/sshd/server/subsystem/sftp/SftpEventListener.java
index c518af3..f72a66d 100644
--- a/sshd-sftp/src/main/java/org/apache/sshd/server/subsystem/sftp/SftpEventListener.java
+++ b/sshd-sftp/src/main/java/org/apache/sshd/server/subsystem/sftp/SftpEventListener.java
@@ -82,6 +82,22 @@ public interface SftpEventListener extends SshdEventListener {
}
/**
+ * Specified file / directory could not be opened - <B>Note:</B> this call may occur
+ * without {@link #opening(ServerSession, String, Handle)} ever having been called
+ *
+ * @param session The {@link ServerSession} through which the request was handled
+ * @param remotePath The path that was specified in the command
+ * @param localPath The matching resolved local path
+ * @param isDirectory Whether this was a folder or a file
+ * @param thrown Non-{@code null} reason for the failure
+ * @throws IOException If failed to handle the call
+ */
+ default void openFailed(ServerSession session, String remotePath, Path localPath, boolean isDirectory, Throwable thrown)
+ throws IOException {
+ // ignored
+ }
+
+ /**
* Result of reading entries from a directory - <B>Note:</B> it may be a
* <U>partial</U> result if the directory contains more entries than can
* be accommodated in the response
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/1bbe4154/sshd-sftp/src/main/java/org/apache/sshd/server/subsystem/sftp/SftpSubsystem.java
----------------------------------------------------------------------
diff --git a/sshd-sftp/src/main/java/org/apache/sshd/server/subsystem/sftp/SftpSubsystem.java b/sshd-sftp/src/main/java/org/apache/sshd/server/subsystem/sftp/SftpSubsystem.java
index bd1f251..9989b7e 100644
--- a/sshd-sftp/src/main/java/org/apache/sshd/server/subsystem/sftp/SftpSubsystem.java
+++ b/sshd-sftp/src/main/java/org/apache/sshd/server/subsystem/sftp/SftpSubsystem.java
@@ -22,6 +22,7 @@ import java.io.EOFException;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
+import java.io.StreamCorruptedException;
import java.net.UnknownServiceException;
import java.nio.file.AccessDeniedException;
import java.nio.file.FileSystem;
@@ -58,6 +59,7 @@ import org.apache.sshd.common.io.IoOutputStream;
import org.apache.sshd.common.random.Random;
import org.apache.sshd.common.session.Session;
import org.apache.sshd.common.subsystem.sftp.SftpConstants;
+import org.apache.sshd.common.subsystem.sftp.SftpException;
import org.apache.sshd.common.subsystem.sftp.SftpHelper;
import org.apache.sshd.common.util.GenericUtils;
import org.apache.sshd.common.util.ValidateUtils;
@@ -740,22 +742,30 @@ public class SftpSubsystem
protected String doOpenDir(int id, String path, Path p, LinkOption... options) throws IOException {
Boolean status = IoUtils.checkFileExists(p, options);
if (status == null) {
- throw new AccessDeniedException(p.toString(), p.toString(), "Cannot determine open-dir existence");
+ throw signalOpenFailure(id, path, p, true,
+ new AccessDeniedException(p.toString(), p.toString(), "Cannot determine open-dir existence"));
}
if (!status) {
- throw new NoSuchFileException(path, path, "Referenced target directory N/A");
+ throw signalOpenFailure(id, path, p, true,
+ new NoSuchFileException(path, path, "Referenced target directory N/A"));
} else if (!Files.isDirectory(p, options)) {
- throw new NotDirectoryException(path);
+ throw signalOpenFailure(id, path, p, true, new NotDirectoryException(path));
} else if (!Files.isReadable(p)) {
- throw new AccessDeniedException(p.toString(), p.toString(), "Not readable");
+ throw signalOpenFailure(id, path, p, true,
+ new AccessDeniedException(p.toString(), p.toString(), "Not readable"));
} else {
String handle;
- synchronized (handles) {
- handle = generateFileHandle(p);
- DirectoryHandle dirHandle = new DirectoryHandle(this, p, handle);
- handles.put(handle, dirHandle);
+ try {
+ synchronized (handles) {
+ handle = generateFileHandle(p);
+ DirectoryHandle dirHandle = new DirectoryHandle(this, p, handle);
+ handles.put(handle, dirHandle);
+ }
+ } catch (IOException e) {
+ throw signalOpenFailure(id, path, p, true, e);
}
+
return handle;
}
}
@@ -862,18 +872,25 @@ public class SftpSubsystem
log.debug("doOpen({})[id={}] SSH_FXP_OPEN (path={}, access=0x{}, pflags=0x{}, attrs={})",
session, id, path, Integer.toHexString(access), Integer.toHexString(pflags), attrs);
}
+
+ Path file = resolveFile(path);
int curHandleCount = handles.size();
int maxHandleCount = session.getIntProperty(MAX_OPEN_HANDLES_PER_SESSION, DEFAULT_MAX_OPEN_HANDLES);
if (curHandleCount > maxHandleCount) {
- throw new IllegalStateException("Too many open handles: current=" + curHandleCount + ", max.=" + maxHandleCount);
+ throw signalOpenFailure(id, path, file, false,
+ new SftpException(SftpConstants.SSH_FX_NO_SPACE_ON_FILESYSTEM,
+ "Too many open handles: current=" + curHandleCount + ", max.=" + maxHandleCount));
}
- Path file = resolveFile(path);
String handle;
- synchronized (handles) {
- handle = generateFileHandle(file);
- FileHandle fileHandle = new FileHandle(this, file, handle, pflags, access, attrs);
- handles.put(handle, fileHandle);
+ try {
+ synchronized (handles) {
+ handle = generateFileHandle(file);
+ FileHandle fileHandle = new FileHandle(this, file, handle, pflags, access, attrs);
+ handles.put(handle, fileHandle);
+ }
+ } catch (IOException e) {
+ throw signalOpenFailure(id, path, file, false, e);
}
return handle;
@@ -881,7 +898,7 @@ public class SftpSubsystem
// we stringify our handles and treat them as such on decoding as well as it is easier to use as a map key
// NOTE: assume handles map is locked
- protected String generateFileHandle(Path file) {
+ protected String generateFileHandle(Path file) throws IOException {
// use several rounds in case the file handle size is relatively small so we might get conflicts
ServerSession session = getServerSession();
boolean traceEnabled = log.isTraceEnabled();
@@ -903,7 +920,7 @@ public class SftpSubsystem
return handle;
}
- throw new IllegalStateException("Failed to generate a unique file handle for " + file);
+ throw new StreamCorruptedException("Failed to generate a unique file handle for " + file);
}
@Override
[3/3] mina-sshd git commit: [SSHD-812] Made SftpSubsystem more
thread-safe
Posted by lg...@apache.org.
[SSHD-812] Made SftpSubsystem more thread-safe
Project: http://git-wip-us.apache.org/repos/asf/mina-sshd/repo
Commit: http://git-wip-us.apache.org/repos/asf/mina-sshd/commit/092a8bba
Tree: http://git-wip-us.apache.org/repos/asf/mina-sshd/tree/092a8bba
Diff: http://git-wip-us.apache.org/repos/asf/mina-sshd/diff/092a8bba
Branch: refs/heads/master
Commit: 092a8bba2a01d3593bedcfcb9a2fd85425e4fee3
Parents: 6cd387d
Author: Goldstein Lyor <ly...@c-b4.com>
Authored: Mon Apr 23 07:59:46 2018 +0300
Committer: Goldstein Lyor <ly...@c-b4.com>
Committed: Mon Apr 23 10:55:36 2018 +0300
----------------------------------------------------------------------
.../server/subsystem/sftp/SftpSubsystem.java | 45 ++++++++++++--------
1 file changed, 28 insertions(+), 17 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/092a8bba/sshd-sftp/src/main/java/org/apache/sshd/server/subsystem/sftp/SftpSubsystem.java
----------------------------------------------------------------------
diff --git a/sshd-sftp/src/main/java/org/apache/sshd/server/subsystem/sftp/SftpSubsystem.java b/sshd-sftp/src/main/java/org/apache/sshd/server/subsystem/sftp/SftpSubsystem.java
index d4cc411..3cc5265 100644
--- a/sshd-sftp/src/main/java/org/apache/sshd/server/subsystem/sftp/SftpSubsystem.java
+++ b/sshd-sftp/src/main/java/org/apache/sshd/server/subsystem/sftp/SftpSubsystem.java
@@ -34,16 +34,17 @@ import java.nio.file.NotDirectoryException;
import java.nio.file.Path;
import java.util.Collection;
import java.util.Comparator;
-import java.util.HashMap;
import java.util.Iterator;
import java.util.Map;
import java.util.Objects;
import java.util.TreeMap;
import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Future;
import java.util.concurrent.LinkedBlockingQueue;
import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicLong;
import org.apache.sshd.common.Factory;
import org.apache.sshd.common.FactoryManager;
@@ -125,6 +126,13 @@ public class SftpSubsystem
protected static final Buffer CLOSE = new ByteArrayBuffer(null, 0, 0);
+ protected final AtomicBoolean closed = new AtomicBoolean(false);
+ protected final AtomicLong requestsCount = new AtomicLong(0L);
+ protected final Map<String, byte[]> extensions = new TreeMap<>(Comparator.naturalOrder());
+ protected final Map<String, Handle> handles = new ConcurrentHashMap<>();
+ protected final Buffer buffer = new ByteArrayBuffer(1024);
+ protected final BlockingQueue<Buffer> requests = new LinkedBlockingQueue<>();
+
protected ExitCallback callback;
protected IoOutputStream out;
protected IoOutputStream err;
@@ -136,16 +144,10 @@ public class SftpSubsystem
protected byte[] workBuf = new byte[Math.max(DEFAULT_FILE_HANDLE_SIZE, Integer.BYTES)];
protected FileSystem fileSystem = FileSystems.getDefault();
protected Path defaultDir = fileSystem.getPath(System.getProperty("user.dir"));
- protected long requestsCount;
protected int version;
- protected final Map<String, byte[]> extensions = new TreeMap<>(Comparator.naturalOrder());
- protected final Map<String, Handle> handles = new HashMap<>();
- protected final Buffer buffer = new ByteArrayBuffer(1024);
- protected final BlockingQueue<Buffer> requests = new LinkedBlockingQueue<>();
protected ServerSession serverSession;
protected ChannelSession channelSession;
- protected final AtomicBoolean closed = new AtomicBoolean(false);
protected ExecutorService executorService;
protected boolean shutdownOnExit;
@@ -346,7 +348,7 @@ public class SftpSubsystem
protected void doProcess(Buffer buffer, int length, int type, int id) throws IOException {
super.doProcess(buffer, length, type, id);
if (type != SftpConstants.SSH_FXP_INIT) {
- requestsCount++;
+ requestsCount.incrementAndGet();
}
}
@@ -518,10 +520,10 @@ public class SftpSubsystem
* server; if it is not, the server MUST fail the request and close the
* channel.
*/
- if (requestsCount > 0L) {
+ if (requestsCount.get() > 0L) {
sendStatus(prepareReply(buffer), id,
- SftpConstants.SSH_FX_FAILURE,
- "Version selection not the 1st request for proposal = " + proposed);
+ SftpConstants.SSH_FX_FAILURE,
+ "Version selection not the 1st request for proposal = " + proposed);
session.close(true);
return;
}
@@ -748,9 +750,12 @@ public class SftpSubsystem
} else if (!Files.isReadable(p)) {
throw new AccessDeniedException(p.toString(), p.toString(), "Not readable");
} else {
- String handle = generateFileHandle(p);
- DirectoryHandle dirHandle = new DirectoryHandle(this, p, handle);
- handles.put(handle, dirHandle);
+ String handle;
+ synchronized (handles) {
+ handle = generateFileHandle(p);
+ DirectoryHandle dirHandle = new DirectoryHandle(this, p, handle);
+ handles.put(handle, dirHandle);
+ }
return handle;
}
}
@@ -864,13 +869,18 @@ public class SftpSubsystem
}
Path file = resolveFile(path);
- String handle = generateFileHandle(file);
- FileHandle fileHandle = new FileHandle(this, file, handle, pflags, access, attrs);
- handles.put(handle, fileHandle);
+ String handle;
+ synchronized (handles) {
+ handle = generateFileHandle(file);
+ FileHandle fileHandle = new FileHandle(this, file, handle, pflags, access, attrs);
+ handles.put(handle, fileHandle);
+ }
+
return handle;
}
// we stringify our handles and treat them as such on decoding as well as it is easier to use as a map key
+ // NOTE: assume handles map is locked
protected String generateFileHandle(Path file) {
// use several rounds in case the file handle size is relatively small so we might get conflicts
ServerSession session = getServerSession();
@@ -878,6 +888,7 @@ public class SftpSubsystem
for (int index = 0; index < maxFileHandleRounds; index++) {
randomizer.fill(workBuf, 0, fileHandleSize);
String handle = BufferUtils.toHex(workBuf, 0, fileHandleSize, BufferUtils.EMPTY_HEX_SEPARATOR);
+
if (handles.containsKey(handle)) {
if (traceEnabled) {
log.trace("generateFileHandle({})[{}] handle={} in use at round {}",
[2/3] mina-sshd git commit: [SSHD-814] Added signalling of failing to
remove SFTP local file/folder due to preconditions having failed
Posted by lg...@apache.org.
[SSHD-814] Added signalling of failing to remove SFTP local file/folder due to preconditions having failed
Project: http://git-wip-us.apache.org/repos/asf/mina-sshd/repo
Commit: http://git-wip-us.apache.org/repos/asf/mina-sshd/commit/08a774e0
Tree: http://git-wip-us.apache.org/repos/asf/mina-sshd/tree/08a774e0
Diff: http://git-wip-us.apache.org/repos/asf/mina-sshd/diff/08a774e0
Branch: refs/heads/master
Commit: 08a774e0a42c3784f1000725f6df009f42d57c65
Parents: 092a8bb
Author: Goldstein Lyor <ly...@c-b4.com>
Authored: Mon Apr 23 08:09:42 2018 +0300
Committer: Goldstein Lyor <ly...@c-b4.com>
Committed: Mon Apr 23 10:55:36 2018 +0300
----------------------------------------------------------------------
.../sftp/AbstractSftpSubsystemHelper.java | 26 +++++++++++++++-----
.../server/subsystem/sftp/SftpSubsystem.java | 4 +--
2 files changed, 22 insertions(+), 8 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/08a774e0/sshd-sftp/src/main/java/org/apache/sshd/server/subsystem/sftp/AbstractSftpSubsystemHelper.java
----------------------------------------------------------------------
diff --git a/sshd-sftp/src/main/java/org/apache/sshd/server/subsystem/sftp/AbstractSftpSubsystemHelper.java b/sshd-sftp/src/main/java/org/apache/sshd/server/subsystem/sftp/AbstractSftpSubsystemHelper.java
index b67163c..bdcb0c5 100644
--- a/sshd-sftp/src/main/java/org/apache/sshd/server/subsystem/sftp/AbstractSftpSubsystemHelper.java
+++ b/sshd-sftp/src/main/java/org/apache/sshd/server/subsystem/sftp/AbstractSftpSubsystemHelper.java
@@ -1451,7 +1451,7 @@ public abstract class AbstractSftpSubsystemHelper
if (Files.isDirectory(p, options)) {
doRemove(id, p);
} else {
- throw new NotDirectoryException(p.toString());
+ throw signalRemovalPreConditionFailure(id, path, p, new NotDirectoryException(p.toString()));
}
}
@@ -1546,17 +1546,31 @@ public abstract class AbstractSftpSubsystemHelper
Boolean status = IoUtils.checkFileExists(p, options);
if (status == null) {
- throw new AccessDeniedException(p.toString(), p.toString(), "Cannot determine existence of remove candidate");
- }
- if (!status) {
- throw new NoSuchFileException(p.toString(), p.toString(), "Removal candidate not found");
+ throw signalRemovalPreConditionFailure(id, path, p,
+ new AccessDeniedException(p.toString(), p.toString(), "Cannot determine existence of remove candidate"));
+ } else if (!status) {
+ throw signalRemovalPreConditionFailure(id, path, p,
+ new NoSuchFileException(p.toString(), p.toString(), "Removal candidate not found"));
} else if (Files.isDirectory(p, options)) {
- throw new SftpException(SftpConstants.SSH_FX_FILE_IS_A_DIRECTORY, p.toString() + " is a folder");
+ throw signalRemovalPreConditionFailure(id, path, p,
+ new SftpException(SftpConstants.SSH_FX_FILE_IS_A_DIRECTORY, p.toString() + " is a folder"));
} else {
doRemove(id, p);
}
}
+ protected <E extends IOException> E signalRemovalPreConditionFailure(int id, String pathValue, Path path, E thrown) throws IOException {
+ SftpEventListener listener = getSftpEventListenerProxy();
+ ServerSession session = getServerSession();
+ if (log.isDebugEnabled()) {
+ log.debug("signalRemovalPreConditionFailure(id={})[{}] signal {} for {}: {}",
+ id, pathValue, thrown.getClass().getSimpleName(), path, thrown.getMessage());
+ }
+ listener.removing(session, path);
+ listener.removed(session, path, thrown);
+ return thrown;
+ }
+
protected void doExtended(Buffer buffer, int id) throws IOException {
executeExtendedCommand(buffer, id, buffer.getString());
}
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/08a774e0/sshd-sftp/src/main/java/org/apache/sshd/server/subsystem/sftp/SftpSubsystem.java
----------------------------------------------------------------------
diff --git a/sshd-sftp/src/main/java/org/apache/sshd/server/subsystem/sftp/SftpSubsystem.java b/sshd-sftp/src/main/java/org/apache/sshd/server/subsystem/sftp/SftpSubsystem.java
index 3cc5265..bd1f251 100644
--- a/sshd-sftp/src/main/java/org/apache/sshd/server/subsystem/sftp/SftpSubsystem.java
+++ b/sshd-sftp/src/main/java/org/apache/sshd/server/subsystem/sftp/SftpSubsystem.java
@@ -522,8 +522,8 @@ public class SftpSubsystem
*/
if (requestsCount.get() > 0L) {
sendStatus(prepareReply(buffer), id,
- SftpConstants.SSH_FX_FAILURE,
- "Version selection not the 1st request for proposal = " + proposed);
+ SftpConstants.SSH_FX_FAILURE,
+ "Version selection not the 1st request for proposal = " + proposed);
session.close(true);
return;
}