From a61a88f36b00414114a94c447b97740171ca13c2 Mon Sep 17 00:00:00 2001 From: liumangmang Date: Wed, 4 Feb 2026 15:03:37 +0800 Subject: [PATCH] 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. --- .../sshmanager/controller/SftpController.java | 190 +++++++++++++----- 1 file changed, 143 insertions(+), 47 deletions(-) diff --git a/backend/src/main/java/com/sshmanager/controller/SftpController.java b/backend/src/main/java/com/sshmanager/controller/SftpController.java index f97f230..fae8e7d 100644 --- a/backend/src/main/java/com/sshmanager/controller/SftpController.java +++ b/backend/src/main/java/com/sshmanager/controller/SftpController.java @@ -20,6 +20,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; +import java.util.function.Supplier; import java.util.stream.Collectors; @RestController @@ -33,6 +34,11 @@ public class SftpController { private final SftpService sftpService; private final Map 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 sessionLocks = new ConcurrentHashMap<>(); public SftpController(ConnectionService connectionService, UserRepository userRepository, @@ -51,6 +57,13 @@ public class SftpController { return userId + ":" + connectionId; } + private T withSessionLock(String key, Supplier action) { + Object lock = sessionLocks.computeIfAbsent(key, k -> new Object()); + synchronized (lock) { + return action.get(); + } + } + private SftpService.SftpSession getOrCreateSession(Long connectionId, Long userId) throws Exception { String key = sessionKey(userId, connectionId); SftpService.SftpSession session = sessions.get(key); @@ -72,12 +85,24 @@ public class SftpController { Authentication authentication) { try { Long userId = getCurrentUserId(authentication); - SftpService.SftpSession session = getOrCreateSession(connectionId, userId); - List files = sftpService.listFiles(session, path); - List dtos = files.stream() - .map(f -> new SftpFileInfo(f.name, f.directory, f.size, f.mtime)) - .collect(Collectors.toList()); - return ResponseEntity.ok(dtos); + String key = sessionKey(userId, connectionId); + return withSessionLock(key, () -> { + try { + SftpService.SftpSession session = getOrCreateSession(connectionId, userId); + List files = sftpService.listFiles(session, path); + List dtos = files.stream() + .map(f -> new SftpFileInfo(f.name, f.directory, f.size, f.mtime)) + .collect(Collectors.toList()); + 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) { String errorMsg = toSftpErrorMessage(e, path, "list"); 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()) { return e.getMessage(); } - Throwable cause = e.getCause(); - if (cause instanceof SftpException) { - return SftpService.formatSftpExceptionMessage((SftpException) cause, path, operation); - } - if (e instanceof SftpException) { - return SftpService.formatSftpExceptionMessage((SftpException) e, path, operation); + // Unwrap nested RuntimeExceptions to find the underlying SftpException (if any). + Throwable cur = e; + for (int i = 0; i < 10 && cur != null; i++) { + if (cur instanceof SftpException) { + return SftpService.formatSftpExceptionMessage((SftpException) cur, path, operation); + } + if (cur.getMessage() != null && !cur.getMessage().trim().isEmpty()) { + return cur.getMessage(); + } + cur = cur.getCause(); } return operation + " failed"; } @@ -107,11 +136,22 @@ public class SftpController { Authentication authentication) { try { Long userId = getCurrentUserId(authentication); - SftpService.SftpSession session = getOrCreateSession(connectionId, userId); - String pwd = sftpService.pwd(session); - Map result = new HashMap<>(); - result.put("path", pwd); - return ResponseEntity.ok(result); + String key = sessionKey(userId, connectionId); + return withSessionLock(key, () -> { + try { + SftpService.SftpSession session = getOrCreateSession(connectionId, userId); + String pwd = sftpService.pwd(session); + Map result = new HashMap<>(); + result.put("path", pwd); + 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) { log.warn("SFTP pwd failed: connectionId={}", connectionId, e); Map err = new HashMap<>(); @@ -127,13 +167,24 @@ public class SftpController { Authentication authentication) { try { Long userId = getCurrentUserId(authentication); - SftpService.SftpSession session = getOrCreateSession(connectionId, userId); - byte[] data = sftpService.download(session, path); - String filename = path.contains("/") ? path.substring(path.lastIndexOf('/') + 1) : path; - return ResponseEntity.ok() - .header(HttpHeaders.CONTENT_DISPOSITION, "attachment; filename=\"" + filename + "\"") - .contentType(MediaType.APPLICATION_OCTET_STREAM) - .body(data); + String key = sessionKey(userId, connectionId); + return withSessionLock(key, () -> { + try { + SftpService.SftpSession session = getOrCreateSession(connectionId, userId); + byte[] data = sftpService.download(session, path); + String filename = path.contains("/") ? path.substring(path.lastIndexOf('/') + 1) : path; + return ResponseEntity.ok() + .header(HttpHeaders.CONTENT_DISPOSITION, "attachment; filename=\"" + filename + "\"") + .contentType(MediaType.APPLICATION_OCTET_STREAM) + .body(data); + } catch (Exception e) { + SftpService.SftpSession existing = sessions.remove(key); + if (existing != null) { + existing.disconnect(); + } + throw new RuntimeException(e); + } + }); } catch (Exception e) { return ResponseEntity.status(500).build(); } @@ -147,14 +198,25 @@ public class SftpController { Authentication authentication) { try { Long userId = getCurrentUserId(authentication); - SftpService.SftpSession session = getOrCreateSession(connectionId, userId); - String remotePath = (path == null || path.isEmpty() || path.equals("/")) - ? "/" + file.getOriginalFilename() - : (path.endsWith("/") ? path + file.getOriginalFilename() : path + "/" + file.getOriginalFilename()); - sftpService.upload(session, remotePath, file.getBytes()); - Map result = new HashMap<>(); - result.put("message", "Uploaded"); - return ResponseEntity.ok(result); + String key = sessionKey(userId, connectionId); + return withSessionLock(key, () -> { + try { + SftpService.SftpSession session = getOrCreateSession(connectionId, userId); + String remotePath = (path == null || path.isEmpty() || path.equals("/")) + ? "/" + file.getOriginalFilename() + : (path.endsWith("/") ? path + file.getOriginalFilename() : path + "/" + file.getOriginalFilename()); + sftpService.upload(session, remotePath, file.getBytes()); + Map result = new HashMap<>(); + result.put("message", "Uploaded"); + 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) { Map error = new HashMap<>(); error.put("error", e.getMessage()); @@ -170,11 +232,22 @@ public class SftpController { Authentication authentication) { try { Long userId = getCurrentUserId(authentication); - SftpService.SftpSession session = getOrCreateSession(connectionId, userId); - sftpService.delete(session, path, directory); - Map result = new HashMap<>(); - result.put("message", "Deleted"); - return ResponseEntity.ok(result); + String key = sessionKey(userId, connectionId); + return withSessionLock(key, () -> { + try { + SftpService.SftpSession session = getOrCreateSession(connectionId, userId); + sftpService.delete(session, path, directory); + Map result = new HashMap<>(); + result.put("message", "Deleted"); + 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) { Map error = new HashMap<>(); error.put("error", e.getMessage()); @@ -189,11 +262,22 @@ public class SftpController { Authentication authentication) { try { Long userId = getCurrentUserId(authentication); - SftpService.SftpSession session = getOrCreateSession(connectionId, userId); - sftpService.mkdir(session, path); - Map result = new HashMap<>(); - result.put("message", "Created"); - return ResponseEntity.ok(result); + String key = sessionKey(userId, connectionId); + return withSessionLock(key, () -> { + try { + SftpService.SftpSession session = getOrCreateSession(connectionId, userId); + sftpService.mkdir(session, path); + Map result = new HashMap<>(); + result.put("message", "Created"); + 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) { Map error = new HashMap<>(); error.put("error", e.getMessage()); @@ -209,11 +293,22 @@ public class SftpController { Authentication authentication) { try { Long userId = getCurrentUserId(authentication); - SftpService.SftpSession session = getOrCreateSession(connectionId, userId); - sftpService.rename(session, oldPath, newPath); - Map result = new HashMap<>(); - result.put("message", "Renamed"); - return ResponseEntity.ok(result); + String key = sessionKey(userId, connectionId); + return withSessionLock(key, () -> { + try { + SftpService.SftpSession session = getOrCreateSession(connectionId, userId); + sftpService.rename(session, oldPath, newPath); + Map result = new HashMap<>(); + result.put("message", "Renamed"); + 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) { Map error = new HashMap<>(); error.put("error", e.getMessage()); @@ -267,6 +362,7 @@ public class SftpController { if (session != null) { session.disconnect(); } + sessionLocks.remove(key); Map result = new HashMap<>(); result.put("message", "Disconnected"); return ResponseEntity.ok(result);