Implement session locking in SftpController to ensure thread safety during concurrent SFTP operations. Introduce a method to handle session locks and improve error handling by forcing reconnections on exceptions. This change addresses potential issues with shared ChannelSftp instances in concurrent requests.
This commit is contained in:
@@ -20,6 +20,7 @@ import java.util.HashMap;
|
|||||||
import java.util.List;
|
import java.util.List;
|
||||||
import java.util.Map;
|
import java.util.Map;
|
||||||
import java.util.concurrent.ConcurrentHashMap;
|
import java.util.concurrent.ConcurrentHashMap;
|
||||||
|
import java.util.function.Supplier;
|
||||||
import java.util.stream.Collectors;
|
import java.util.stream.Collectors;
|
||||||
|
|
||||||
@RestController
|
@RestController
|
||||||
@@ -33,6 +34,11 @@ public class SftpController {
|
|||||||
private final SftpService sftpService;
|
private final SftpService sftpService;
|
||||||
|
|
||||||
private final Map<String, SftpService.SftpSession> sessions = new ConcurrentHashMap<>();
|
private final Map<String, SftpService.SftpSession> sessions = new ConcurrentHashMap<>();
|
||||||
|
/**
|
||||||
|
* JSch ChannelSftp is not thread-safe. If the frontend triggers concurrent requests (e.g. rapid ".." navigation),
|
||||||
|
* sharing one ChannelSftp can crash with internal stream exceptions. We serialize all SFTP ops per (user, connection).
|
||||||
|
*/
|
||||||
|
private final Map<String, Object> sessionLocks = new ConcurrentHashMap<>();
|
||||||
|
|
||||||
public SftpController(ConnectionService connectionService,
|
public SftpController(ConnectionService connectionService,
|
||||||
UserRepository userRepository,
|
UserRepository userRepository,
|
||||||
@@ -51,6 +57,13 @@ public class SftpController {
|
|||||||
return userId + ":" + connectionId;
|
return userId + ":" + connectionId;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private <T> T withSessionLock(String key, Supplier<T> action) {
|
||||||
|
Object lock = sessionLocks.computeIfAbsent(key, k -> new Object());
|
||||||
|
synchronized (lock) {
|
||||||
|
return action.get();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
private SftpService.SftpSession getOrCreateSession(Long connectionId, Long userId) throws Exception {
|
private SftpService.SftpSession getOrCreateSession(Long connectionId, Long userId) throws Exception {
|
||||||
String key = sessionKey(userId, connectionId);
|
String key = sessionKey(userId, connectionId);
|
||||||
SftpService.SftpSession session = sessions.get(key);
|
SftpService.SftpSession session = sessions.get(key);
|
||||||
@@ -72,12 +85,24 @@ public class SftpController {
|
|||||||
Authentication authentication) {
|
Authentication authentication) {
|
||||||
try {
|
try {
|
||||||
Long userId = getCurrentUserId(authentication);
|
Long userId = getCurrentUserId(authentication);
|
||||||
|
String key = sessionKey(userId, connectionId);
|
||||||
|
return withSessionLock(key, () -> {
|
||||||
|
try {
|
||||||
SftpService.SftpSession session = getOrCreateSession(connectionId, userId);
|
SftpService.SftpSession session = getOrCreateSession(connectionId, userId);
|
||||||
List<SftpService.FileInfo> files = sftpService.listFiles(session, path);
|
List<SftpService.FileInfo> files = sftpService.listFiles(session, path);
|
||||||
List<SftpFileInfo> dtos = files.stream()
|
List<SftpFileInfo> dtos = files.stream()
|
||||||
.map(f -> new SftpFileInfo(f.name, f.directory, f.size, f.mtime))
|
.map(f -> new SftpFileInfo(f.name, f.directory, f.size, f.mtime))
|
||||||
.collect(Collectors.toList());
|
.collect(Collectors.toList());
|
||||||
return ResponseEntity.ok(dtos);
|
return ResponseEntity.ok(dtos);
|
||||||
|
} catch (Exception e) {
|
||||||
|
// If the underlying SFTP channel got into a bad state, force reconnect on next request.
|
||||||
|
SftpService.SftpSession existing = sessions.remove(key);
|
||||||
|
if (existing != null) {
|
||||||
|
existing.disconnect();
|
||||||
|
}
|
||||||
|
throw new RuntimeException(e);
|
||||||
|
}
|
||||||
|
});
|
||||||
} catch (Exception e) {
|
} catch (Exception e) {
|
||||||
String errorMsg = toSftpErrorMessage(e, path, "list");
|
String errorMsg = toSftpErrorMessage(e, path, "list");
|
||||||
log.warn("SFTP list failed: connectionId={}, path={}, error={}", connectionId, path, errorMsg, e);
|
log.warn("SFTP list failed: connectionId={}, path={}, error={}", connectionId, path, errorMsg, e);
|
||||||
@@ -91,12 +116,16 @@ public class SftpController {
|
|||||||
if (e.getMessage() != null && !e.getMessage().trim().isEmpty()) {
|
if (e.getMessage() != null && !e.getMessage().trim().isEmpty()) {
|
||||||
return e.getMessage();
|
return e.getMessage();
|
||||||
}
|
}
|
||||||
Throwable cause = e.getCause();
|
// Unwrap nested RuntimeExceptions to find the underlying SftpException (if any).
|
||||||
if (cause instanceof SftpException) {
|
Throwable cur = e;
|
||||||
return SftpService.formatSftpExceptionMessage((SftpException) cause, path, operation);
|
for (int i = 0; i < 10 && cur != null; i++) {
|
||||||
|
if (cur instanceof SftpException) {
|
||||||
|
return SftpService.formatSftpExceptionMessage((SftpException) cur, path, operation);
|
||||||
}
|
}
|
||||||
if (e instanceof SftpException) {
|
if (cur.getMessage() != null && !cur.getMessage().trim().isEmpty()) {
|
||||||
return SftpService.formatSftpExceptionMessage((SftpException) e, path, operation);
|
return cur.getMessage();
|
||||||
|
}
|
||||||
|
cur = cur.getCause();
|
||||||
}
|
}
|
||||||
return operation + " failed";
|
return operation + " failed";
|
||||||
}
|
}
|
||||||
@@ -107,11 +136,22 @@ public class SftpController {
|
|||||||
Authentication authentication) {
|
Authentication authentication) {
|
||||||
try {
|
try {
|
||||||
Long userId = getCurrentUserId(authentication);
|
Long userId = getCurrentUserId(authentication);
|
||||||
|
String key = sessionKey(userId, connectionId);
|
||||||
|
return withSessionLock(key, () -> {
|
||||||
|
try {
|
||||||
SftpService.SftpSession session = getOrCreateSession(connectionId, userId);
|
SftpService.SftpSession session = getOrCreateSession(connectionId, userId);
|
||||||
String pwd = sftpService.pwd(session);
|
String pwd = sftpService.pwd(session);
|
||||||
Map<String, String> result = new HashMap<>();
|
Map<String, String> result = new HashMap<>();
|
||||||
result.put("path", pwd);
|
result.put("path", pwd);
|
||||||
return ResponseEntity.ok(result);
|
return ResponseEntity.ok(result);
|
||||||
|
} catch (Exception e) {
|
||||||
|
SftpService.SftpSession existing = sessions.remove(key);
|
||||||
|
if (existing != null) {
|
||||||
|
existing.disconnect();
|
||||||
|
}
|
||||||
|
throw new RuntimeException(e);
|
||||||
|
}
|
||||||
|
});
|
||||||
} catch (Exception e) {
|
} catch (Exception e) {
|
||||||
log.warn("SFTP pwd failed: connectionId={}", connectionId, e);
|
log.warn("SFTP pwd failed: connectionId={}", connectionId, e);
|
||||||
Map<String, String> err = new HashMap<>();
|
Map<String, String> err = new HashMap<>();
|
||||||
@@ -127,6 +167,9 @@ public class SftpController {
|
|||||||
Authentication authentication) {
|
Authentication authentication) {
|
||||||
try {
|
try {
|
||||||
Long userId = getCurrentUserId(authentication);
|
Long userId = getCurrentUserId(authentication);
|
||||||
|
String key = sessionKey(userId, connectionId);
|
||||||
|
return withSessionLock(key, () -> {
|
||||||
|
try {
|
||||||
SftpService.SftpSession session = getOrCreateSession(connectionId, userId);
|
SftpService.SftpSession session = getOrCreateSession(connectionId, userId);
|
||||||
byte[] data = sftpService.download(session, path);
|
byte[] data = sftpService.download(session, path);
|
||||||
String filename = path.contains("/") ? path.substring(path.lastIndexOf('/') + 1) : path;
|
String filename = path.contains("/") ? path.substring(path.lastIndexOf('/') + 1) : path;
|
||||||
@@ -134,6 +177,14 @@ public class SftpController {
|
|||||||
.header(HttpHeaders.CONTENT_DISPOSITION, "attachment; filename=\"" + filename + "\"")
|
.header(HttpHeaders.CONTENT_DISPOSITION, "attachment; filename=\"" + filename + "\"")
|
||||||
.contentType(MediaType.APPLICATION_OCTET_STREAM)
|
.contentType(MediaType.APPLICATION_OCTET_STREAM)
|
||||||
.body(data);
|
.body(data);
|
||||||
|
} catch (Exception e) {
|
||||||
|
SftpService.SftpSession existing = sessions.remove(key);
|
||||||
|
if (existing != null) {
|
||||||
|
existing.disconnect();
|
||||||
|
}
|
||||||
|
throw new RuntimeException(e);
|
||||||
|
}
|
||||||
|
});
|
||||||
} catch (Exception e) {
|
} catch (Exception e) {
|
||||||
return ResponseEntity.status(500).build();
|
return ResponseEntity.status(500).build();
|
||||||
}
|
}
|
||||||
@@ -147,6 +198,9 @@ public class SftpController {
|
|||||||
Authentication authentication) {
|
Authentication authentication) {
|
||||||
try {
|
try {
|
||||||
Long userId = getCurrentUserId(authentication);
|
Long userId = getCurrentUserId(authentication);
|
||||||
|
String key = sessionKey(userId, connectionId);
|
||||||
|
return withSessionLock(key, () -> {
|
||||||
|
try {
|
||||||
SftpService.SftpSession session = getOrCreateSession(connectionId, userId);
|
SftpService.SftpSession session = getOrCreateSession(connectionId, userId);
|
||||||
String remotePath = (path == null || path.isEmpty() || path.equals("/"))
|
String remotePath = (path == null || path.isEmpty() || path.equals("/"))
|
||||||
? "/" + file.getOriginalFilename()
|
? "/" + file.getOriginalFilename()
|
||||||
@@ -155,6 +209,14 @@ public class SftpController {
|
|||||||
Map<String, String> result = new HashMap<>();
|
Map<String, String> result = new HashMap<>();
|
||||||
result.put("message", "Uploaded");
|
result.put("message", "Uploaded");
|
||||||
return ResponseEntity.ok(result);
|
return ResponseEntity.ok(result);
|
||||||
|
} catch (Exception e) {
|
||||||
|
SftpService.SftpSession existing = sessions.remove(key);
|
||||||
|
if (existing != null) {
|
||||||
|
existing.disconnect();
|
||||||
|
}
|
||||||
|
throw new RuntimeException(e);
|
||||||
|
}
|
||||||
|
});
|
||||||
} catch (Exception e) {
|
} catch (Exception e) {
|
||||||
Map<String, String> error = new HashMap<>();
|
Map<String, String> error = new HashMap<>();
|
||||||
error.put("error", e.getMessage());
|
error.put("error", e.getMessage());
|
||||||
@@ -170,11 +232,22 @@ public class SftpController {
|
|||||||
Authentication authentication) {
|
Authentication authentication) {
|
||||||
try {
|
try {
|
||||||
Long userId = getCurrentUserId(authentication);
|
Long userId = getCurrentUserId(authentication);
|
||||||
|
String key = sessionKey(userId, connectionId);
|
||||||
|
return withSessionLock(key, () -> {
|
||||||
|
try {
|
||||||
SftpService.SftpSession session = getOrCreateSession(connectionId, userId);
|
SftpService.SftpSession session = getOrCreateSession(connectionId, userId);
|
||||||
sftpService.delete(session, path, directory);
|
sftpService.delete(session, path, directory);
|
||||||
Map<String, String> result = new HashMap<>();
|
Map<String, String> result = new HashMap<>();
|
||||||
result.put("message", "Deleted");
|
result.put("message", "Deleted");
|
||||||
return ResponseEntity.ok(result);
|
return ResponseEntity.ok(result);
|
||||||
|
} catch (Exception e) {
|
||||||
|
SftpService.SftpSession existing = sessions.remove(key);
|
||||||
|
if (existing != null) {
|
||||||
|
existing.disconnect();
|
||||||
|
}
|
||||||
|
throw new RuntimeException(e);
|
||||||
|
}
|
||||||
|
});
|
||||||
} catch (Exception e) {
|
} catch (Exception e) {
|
||||||
Map<String, String> error = new HashMap<>();
|
Map<String, String> error = new HashMap<>();
|
||||||
error.put("error", e.getMessage());
|
error.put("error", e.getMessage());
|
||||||
@@ -189,11 +262,22 @@ public class SftpController {
|
|||||||
Authentication authentication) {
|
Authentication authentication) {
|
||||||
try {
|
try {
|
||||||
Long userId = getCurrentUserId(authentication);
|
Long userId = getCurrentUserId(authentication);
|
||||||
|
String key = sessionKey(userId, connectionId);
|
||||||
|
return withSessionLock(key, () -> {
|
||||||
|
try {
|
||||||
SftpService.SftpSession session = getOrCreateSession(connectionId, userId);
|
SftpService.SftpSession session = getOrCreateSession(connectionId, userId);
|
||||||
sftpService.mkdir(session, path);
|
sftpService.mkdir(session, path);
|
||||||
Map<String, String> result = new HashMap<>();
|
Map<String, String> result = new HashMap<>();
|
||||||
result.put("message", "Created");
|
result.put("message", "Created");
|
||||||
return ResponseEntity.ok(result);
|
return ResponseEntity.ok(result);
|
||||||
|
} catch (Exception e) {
|
||||||
|
SftpService.SftpSession existing = sessions.remove(key);
|
||||||
|
if (existing != null) {
|
||||||
|
existing.disconnect();
|
||||||
|
}
|
||||||
|
throw new RuntimeException(e);
|
||||||
|
}
|
||||||
|
});
|
||||||
} catch (Exception e) {
|
} catch (Exception e) {
|
||||||
Map<String, String> error = new HashMap<>();
|
Map<String, String> error = new HashMap<>();
|
||||||
error.put("error", e.getMessage());
|
error.put("error", e.getMessage());
|
||||||
@@ -209,11 +293,22 @@ public class SftpController {
|
|||||||
Authentication authentication) {
|
Authentication authentication) {
|
||||||
try {
|
try {
|
||||||
Long userId = getCurrentUserId(authentication);
|
Long userId = getCurrentUserId(authentication);
|
||||||
|
String key = sessionKey(userId, connectionId);
|
||||||
|
return withSessionLock(key, () -> {
|
||||||
|
try {
|
||||||
SftpService.SftpSession session = getOrCreateSession(connectionId, userId);
|
SftpService.SftpSession session = getOrCreateSession(connectionId, userId);
|
||||||
sftpService.rename(session, oldPath, newPath);
|
sftpService.rename(session, oldPath, newPath);
|
||||||
Map<String, String> result = new HashMap<>();
|
Map<String, String> result = new HashMap<>();
|
||||||
result.put("message", "Renamed");
|
result.put("message", "Renamed");
|
||||||
return ResponseEntity.ok(result);
|
return ResponseEntity.ok(result);
|
||||||
|
} catch (Exception e) {
|
||||||
|
SftpService.SftpSession existing = sessions.remove(key);
|
||||||
|
if (existing != null) {
|
||||||
|
existing.disconnect();
|
||||||
|
}
|
||||||
|
throw new RuntimeException(e);
|
||||||
|
}
|
||||||
|
});
|
||||||
} catch (Exception e) {
|
} catch (Exception e) {
|
||||||
Map<String, String> error = new HashMap<>();
|
Map<String, String> error = new HashMap<>();
|
||||||
error.put("error", e.getMessage());
|
error.put("error", e.getMessage());
|
||||||
@@ -267,6 +362,7 @@ public class SftpController {
|
|||||||
if (session != null) {
|
if (session != null) {
|
||||||
session.disconnect();
|
session.disconnect();
|
||||||
}
|
}
|
||||||
|
sessionLocks.remove(key);
|
||||||
Map<String, String> result = new HashMap<>();
|
Map<String, String> result = new HashMap<>();
|
||||||
result.put("message", "Disconnected");
|
result.put("message", "Disconnected");
|
||||||
return ResponseEntity.ok(result);
|
return ResponseEntity.ok(result);
|
||||||
|
|||||||
Reference in New Issue
Block a user