Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

adds fallback to posix-rename@openssh.com extension if possible and c… #827

Merged
merged 15 commits into from
Oct 23, 2023
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 62 additions & 7 deletions src/main/java/net/schmizz/sshj/sftp/SFTPEngine.java
Original file line number Diff line number Diff line change
Expand Up @@ -237,16 +237,71 @@ public void rename(String oldPath, String newPath, Set<RenameFlags> flags)
if (operativeVersion < 1)
throw new SFTPException("RENAME is not supported in SFTPv" + operativeVersion);

final Request request = newRequest(PacketType.RENAME).putString(oldPath, sub.getRemoteCharset()).putString(newPath, sub.getRemoteCharset());
// SFTP Version 5 introduced rename flags according to Section 6.5 of the specification
if (operativeVersion >= 5) {
long renameFlagMask = 0L;
for (RenameFlags flag : flags) {
renameFlagMask = renameFlagMask | flag.longValue();
// request variables to be determined
PacketType type = PacketType.RENAME; // Default
long renameFlagMask = 0L;
String serverExtension = null;

if (!flags.isEmpty()) {
// SFTP Version 5 introduced rename flags according to Section 6.5 of the specification
if (operativeVersion >= 5) {
for (RenameFlags flag : flags) {
renameFlagMask = renameFlagMask | flag.longValue();
}
}
request.putUInt32(renameFlagMask);
// Try to find a fallback solution if flags are not supported by the server.

// "posix-rename@openssh.com" provides ATOMIC and OVERWRITE behaviour.
// From the SFTP-spec, Section 6.5:
// "If SSH_FXP_RENAME_OVERWRITE is specified, the server MAY perform an atomic rename even if it is
// not requested."
// So, if overwrite is allowed we can always use the posix-rename as a fallback.
else if (flags.contains(RenameFlags.OVERWRITE) &&
supportsServerExtension("posix-rename","openssh.com")) {

type = PacketType.EXTENDED;
serverExtension = "posix-rename@openssh.com";
}

// Because the OVERWRITE flag changes the behaviour in a possibly unintended way, it has to be
// explicitly requested for the above fallback to be applicable.
// Tell this to the developer if ATOMIC is requested without OVERWRITE.
else if (flags.contains(RenameFlags.ATOMIC) &&
!flags.contains(RenameFlags.OVERWRITE) &&
!flags.contains(RenameFlags.NATIVE) && // see next case below
supportsServerExtension("posix-rename","openssh.com")) {

throw new SFTPException("RENAME-FLAGS are not supported in SFTPv" + operativeVersion + " but " +
"the \"posix-rename@openssh.com\" extension could be used as fallback if OVERWRITE " +
"behaviour is acceptable (needs to be activated via RenameFlags.OVERWRITE).");
}

// From the SFTP-spec, Section 6.5:
// "If flags includes SSH_FXP_RENAME_NATIVE, the server is free to do the rename operation in whatever
// fashion it deems appropriate. Other flag values are considered hints as to desired behavior, but not
// requirements."
else if (flags.contains(RenameFlags.NATIVE)) {
log.debug("Flags are not supported but NATIVE-flag allows to ignore other requested flags: " +
flags.toString());
}

// finally: let the user know that the server does not support what was asked
else throw new SFTPException("RENAME-FLAGS are not supported in SFTPv" + operativeVersion + " and no " +
"supported server extension could be found to achieve a similar result.");
}

// build and send request
final Request request = newRequest(type);

if (serverExtension != null)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add { around the if-statement... Everywhere where that's not done is a leftover ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, i just pushed a corrected version.
Thanks for your quick reply.

request.putString(serverExtension);

request.putString(oldPath, sub.getRemoteCharset())
.putString(newPath, sub.getRemoteCharset());

if (renameFlagMask != 0L)
request.putUInt32(renameFlagMask);

doRequest(request).ensureStatusPacketIsOK();
}

Expand Down