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

Adjust preflight checks only requiring parent directory and filename. #16078

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -70,11 +70,11 @@ public Path copy(final Path source, final Path copy, final TransferStatus status
}

@Override
public void preflight(final Path source, final Path target) throws BackgroundException {
public void preflight(final Path source, final Path directory, final String filename) throws BackgroundException {
if(containerService.isContainer(source)) {
throw new UnsupportedException(MessageFormat.format(LocaleFactory.localizedString("Cannot copy {0}", "Error"), source.getName())).withFile(source);
}
if(containerService.isContainer(target)) {
if(containerService.isContainer(directory)) {
throw new UnsupportedException(MessageFormat.format(LocaleFactory.localizedString("Cannot copy {0}", "Error"), source.getName())).withFile(source);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ public AzureMoveFeature(final AzureSession session) {
}

@Override
public void preflight(final Path source, final Path target) throws BackgroundException {
proxy.preflight(source, target);
public void preflight(final Path source, final Path directory, final String filename) throws BackgroundException {
proxy.preflight(source, directory, filename);
delete.preflight(source);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,9 @@ public void testCopy() throws Exception {
final Path test = new AzureTouchFeature(session).touch(
new Path(container, new AlphanumericRandomStringService().random(), EnumSet.of(Path.Type.file)), new TransferStatus());
final AzureCopyFeature feature = new AzureCopyFeature(session);
assertThrows(UnsupportedException.class, () -> feature.preflight(container, test));
assertThrows(UnsupportedException.class, () -> feature.preflight(container, container, test.getName()));
try {
feature.preflight(container, test);
feature.preflight(container, container, test.getName());
}
catch(UnsupportedException e) {
assertEquals("Unsupported", e.getMessage());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ public void testMove() throws Exception {
@Test
public void testSupport() {
final Path c = new Path("/c", EnumSet.of(Path.Type.directory));
assertFalse(new AzureMoveFeature(session).isSupported(c, c));
assertFalse(new AzureMoveFeature(session).isSupported(c, c.getParent(), c.getName()));
final Path cf = new Path("/c/f", EnumSet.of(Path.Type.directory));
assertTrue(new AzureMoveFeature(session).isSupported(cf, cf));
assertTrue(new AzureMoveFeature(session).isSupported(cf, cf.getParent(), cf.getName()));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -65,17 +65,17 @@ public Path copy(final Path source, final Path target, final TransferStatus stat
}

@Override
public void preflight(final Path source, final Path target) throws BackgroundException {
public void preflight(final Path source, final Path directory, final String filename) throws BackgroundException {
if(source.getType().contains(Path.Type.upload)) {
throw new UnsupportedException(MessageFormat.format(LocaleFactory.localizedString("Cannot copy {0}", "Error"), source.getName())).withFile(source);
}
if(containerService.isContainer(source)) {
throw new UnsupportedException(MessageFormat.format(LocaleFactory.localizedString("Cannot copy {0}", "Error"), source.getName())).withFile(source);
}
if(containerService.isContainer(target)) {
if(directory.isRoot()) {
throw new UnsupportedException(MessageFormat.format(LocaleFactory.localizedString("Cannot copy {0}", "Error"), source.getName())).withFile(source);
}
if(!new SimplePathPredicate(containerService.getContainer(source)).test(containerService.getContainer(target))) {
if(!new SimplePathPredicate(containerService.getContainer(source)).test(containerService.getContainer(directory))) {
throw new UnsupportedException(MessageFormat.format(LocaleFactory.localizedString("Cannot copy {0}", "Error"), source.getName())).withFile(source);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
public class B2MoveFeature implements Move {

private final PathContainerService containerService
= new B2PathContainerService();
= new B2PathContainerService();

private final B2Session session;
private final B2VersionIdProvider fileid;
Expand All @@ -54,11 +54,11 @@ public Path move(final Path source, final Path target, final TransferStatus stat
}

@Override
public void preflight(final Path source, final Path target) throws BackgroundException {
public void preflight(final Path source, final Path directory, final String filename) throws BackgroundException {
if(containerService.isContainer(source)) {
throw new UnsupportedException(MessageFormat.format(LocaleFactory.localizedString("Cannot rename {0}", "Error"), source.getName())).withFile(source);
}
proxy.preflight(source, target);
proxy.preflight(source, directory, filename);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ public Path copy(final Path source, final Path target, final TransferStatus stat
}

@Override
public void preflight(final Path source, final Path target) throws BackgroundException {
new B2CopyFeature(session, fileid).preflight(source, target);
public void preflight(final Path source, final Path directory, final String filename) throws BackgroundException {
new B2CopyFeature(session, fileid).preflight(source, directory, filename);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,9 @@ public void testCopy() throws Exception {
final Path test = new B2TouchFeature(session, fileid).touch(new Path(container, name, EnumSet.of(Path.Type.file)), new TransferStatus());
assertTrue(new B2FindFeature(session, fileid).find(test));
final B2CopyFeature feature = new B2CopyFeature(session, fileid);
assertThrows(UnsupportedException.class, () -> feature.preflight(container, test));
assertThrows(UnsupportedException.class, () -> feature.preflight(container, test.getParent(), test.getName()));
try {
feature.preflight(container, test);
feature.preflight(container, test.getParent(), test.getName());
}
catch(UnsupportedException e) {
assertEquals("Unsupported", e.getMessage());
Expand Down
6 changes: 3 additions & 3 deletions box/src/main/java/ch/cyberduck/core/box/BoxCopyFeature.java
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,9 @@ public EnumSet<Flags> features(final Path source, final Path target) {
}

@Override
public void preflight(final Path source, final Path target) throws BackgroundException {
if(!BoxTouchFeature.validate(target.getName())) {
throw new InvalidFilenameException(MessageFormat.format(LocaleFactory.localizedString("Cannot create {0}", "Error"), target.getName()));
public void preflight(final Path source, final Path directory, final String filename) throws BackgroundException {
if(!BoxTouchFeature.validate(filename)) {
throw new InvalidFilenameException(MessageFormat.format(LocaleFactory.localizedString("Cannot create {0}", "Error"), filename));
}
}
}
6 changes: 3 additions & 3 deletions box/src/main/java/ch/cyberduck/core/box/BoxMoveFeature.java
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,9 @@ public EnumSet<Flags> features(final Path source, final Path target) {
}

@Override
public void preflight(final Path source, final Path target) throws BackgroundException {
if(!BoxTouchFeature.validate(target.getName())) {
throw new InvalidFilenameException(MessageFormat.format(LocaleFactory.localizedString("Cannot create {0}", "Error"), target.getName()));
public void preflight(final Path source, final Path directory, final String filename) throws BackgroundException {
if(!BoxTouchFeature.validate(filename)) {
throw new InvalidFilenameException(MessageFormat.format(LocaleFactory.localizedString("Cannot create {0}", "Error"), filename));
}
}
}
14 changes: 9 additions & 5 deletions core/src/main/java/ch/cyberduck/core/features/Copy.java
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,13 @@ default boolean isRecursive(final Path source, final Path target) {

/**
* @param source Source file or folder
* @param target Target file or folder
* @param folder Target folder
* @param filename Target filename
* @return False if not supported for given files
*/
default boolean isSupported(final Path source, final Path target) {
default boolean isSupported(final Path source, final Path folder, final String filename) {
try {
this.preflight(source, target);
this.preflight(source, folder, filename);
return true;
}
catch(BackgroundException e) {
Expand All @@ -77,12 +78,15 @@ default Copy withTarget(final Session<?> session) {
}

/**
* @param source Existing file or folder
* @param directory Target directory
* @param filename Target filename
* @throws AccessDeniedException Permission failure to create target directory
* @throws UnsupportedException Copy operation not supported for source
* @throws InvalidFilenameException Target filename not supported
*/
default void preflight(final Path source, final Path target) throws BackgroundException {
if(!target.getParent().attributes().getPermission().isWritable()) {
default void preflight(final Path source, final Path directory, final String filename) throws BackgroundException {
if(!directory.getParent().attributes().getPermission().isWritable()) {
throw new UnsupportedException(MessageFormat.format(LocaleFactory.localizedString("Cannot copy {0}", "Error"),
source.getName())).withFile(source);
}
Expand Down
22 changes: 13 additions & 9 deletions core/src/main/java/ch/cyberduck/core/features/Move.java
Original file line number Diff line number Diff line change
Expand Up @@ -53,13 +53,14 @@ default boolean isRecursive(final Path source, final Path target) {
}

/**
* @param source Source file or folder
* @param target Target file or folder
* @param source Source file or folder
* @param directory Target directory
* @param filename Target filename
* @return False if not supported for given files
*/
default boolean isSupported(final Path source, final Path target) {
default boolean isSupported(final Path source, final Path directory, final String filename) {
try {
this.preflight(source, target);
this.preflight(source, directory, filename);
return true;
}
catch(BackgroundException e) {
Expand All @@ -76,12 +77,15 @@ default Move withTarget(Session<?> session) {
}

/**
* @param source Existing file or folder
* @param directory Target directory
* @param filename Target filename
* @throws AccessDeniedException Permission failure for target parent directory
* @throws UnsupportedException Move operation not supported for source
* @throws InvalidFilenameException Target filename not supported
*/
default void preflight(final Path source, final Path target) throws BackgroundException {
if(!target.getParent().attributes().getPermission().isWritable()) {
default void preflight(final Path source, final Path directory, final String filename) throws BackgroundException {
if(!directory.getParent().attributes().getPermission().isWritable()) {
throw new AccessDeniedException(MessageFormat.format(LocaleFactory.localizedString("Cannot rename {0}", "Error"),
source.getName())).withFile(source);
}
Expand All @@ -91,9 +95,9 @@ default void preflight(final Path source, final Path target) throws BackgroundEx
/**
* @return Supported features
*/
default EnumSet<Flags> features(Path source, Path target) {
return EnumSet.noneOf(Flags.class);
}
default EnumSet<Flags> features(Path source, Path target) {
return EnumSet.noneOf(Flags.class);
}

/**
* Feature flags
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public Path copy(final Path source, final Path target, final TransferStatus stat
}

@Override
public void preflight(final Path source, final Path target) throws BackgroundException {
public void preflight(final Path source, final Path directory, final String filename) throws BackgroundException {
switch(from.getHost().getProtocol().getType()) {
case ftp:
case irods:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ public void setConfiguration(final Path container, final PasswordCallback prompt
public boolean save(final Path file) throws BackgroundException {
final Path version = new Path(provider.provide(file), formatter.toVersion(file.getName()), file.getType());
final Move feature = session.getFeature(Move.class);
if(!feature.isSupported(file, version)) {
if(!feature.isSupported(file, version.getParent(), version.getName())) {
log.warn(String.format("Skip saving version for %s", file));
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public Path move(final Path file, final Path renamed, final TransferStatus statu
}

@Override
public void preflight(final Path source, final Path target) throws BackgroundException {
public void preflight(final Path source, final Path directory, final String filename) throws BackgroundException {
throw new AccessDeniedException(LocaleFactory.localizedString("Unsupported", "Error"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -157,15 +157,14 @@ public TransferStatus prepare(final Path file, final Local local, final Transfer
}
if(options.temporary) {
final Move feature = session.getFeature(Move.class);
final Path renamed = new Path(file.getParent(),
MessageFormat.format(preferences.getProperty("queue.upload.file.temporary.format"),
file.getName(), new AlphanumericRandomStringService().random()), file.getType());
if(feature.isSupported(file, renamed)) {
final String renamed = MessageFormat.format(preferences.getProperty("queue.upload.file.temporary.format"),
file.getName(), new AlphanumericRandomStringService().random());
if(feature.isSupported(file, file.getParent(), renamed)) {
if(log.isDebugEnabled()) {
log.debug(String.format("Set temporary filename %s", renamed));
}
// Set target name after transfer
status.withRename(renamed).withDisplayname(file);
status.withRename(new Path(file.getParent(), renamed, file.getType())).withDisplayname(file);
// Remember status of target file for later rename
status.getDisplayname().exists(status.isExists());
// Keep exist flag for subclasses to determine additional rename strategy
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,20 +74,20 @@ else if(registry.find(session, copy, false).equals(Vault.DISABLED)) {
}

@Override
public void preflight(final Path source, final Path copy) throws BackgroundException {
public void preflight(final Path source, final Path copy, final String filename) throws BackgroundException {
try {
if(registry.find(session, source, false).equals(Vault.DISABLED)) {
registry.find(session, copy, false).getFeature(session, Copy.class, proxy).withTarget(destination).preflight(source, copy);
registry.find(session, copy, false).getFeature(session, Copy.class, proxy).withTarget(destination).preflight(source, copy, filename);
}
else if(registry.find(session, copy, false).equals(Vault.DISABLED)) {
registry.find(session, source, false).getFeature(session, Copy.class, proxy).withTarget(destination).preflight(source, copy);
registry.find(session, source, false).getFeature(session, Copy.class, proxy).withTarget(destination).preflight(source, copy, filename);
}
else {
registry.find(session, copy, false).getFeature(session, Copy.class, proxy).withTarget(destination).preflight(source, copy);
registry.find(session, copy, false).getFeature(session, Copy.class, proxy).withTarget(destination).preflight(source, copy, filename);
}
}
catch(VaultUnlockCancelException e) {
proxy.preflight(source, copy);
proxy.preflight(source, copy, filename);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,17 +85,17 @@ public EnumSet<Flags> features(final Path source, final Path target) {
}

@Override
public void preflight(final Path source, final Path target) throws BackgroundException {
public void preflight(final Path source, final Path directory, final String filename) throws BackgroundException {
try {
if(registry.find(session, source, false).equals(registry.find(session, target, false))) {
registry.find(session, source, false).getFeature(session, Move.class, proxy).preflight(source, target);
if(registry.find(session, source, false).equals(registry.find(session, directory, false))) {
registry.find(session, source, false).getFeature(session, Move.class, proxy).preflight(source, directory, filename);
}
else {
session.getFeature(Copy.class).preflight(source, target);
session.getFeature(Copy.class).preflight(source, directory, filename);
}
}
catch(VaultUnlockCancelException e) {
proxy.preflight(source, target);
proxy.preflight(source, directory, filename);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
public class NullMoveFeature extends DisabledMoveFeature {

@Override
public void preflight(final Path source, final Path target) throws BackgroundException {
public void preflight(final Path source, final Path directory, final String filename) throws BackgroundException {
//
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ public Path move(final Path file, final Path renamed, final TransferStatus statu
}

@Override
public void preflight(final Path source, final Path target) throws BackgroundException {
public void preflight(final Path source, final Path directory, final String filename) throws BackgroundException {
//
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,12 +90,12 @@ public EnumSet<Flags> features(final Path source, final Path target) {
}

@Override
public void preflight(final Path source, final Path copy) throws BackgroundException {
public void preflight(final Path source, final Path copy, final String filename) throws BackgroundException {
if(vault.contains(source) && vault.contains(copy)) {
proxy.withTarget(target).preflight(source, copy);
proxy.withTarget(target).preflight(source, copy, filename);
}
else {
new DefaultCopyFeature(session).withTarget(target).preflight(source, copy);
new DefaultCopyFeature(session).withTarget(target).preflight(source, copy, filename);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,11 @@ public EnumSet<Flags> features(final Path source, final Path target) {
}

@Override
public void preflight(final Path source, final Path target) throws BackgroundException {
if(!vault.getFilenameProvider().isValid(target.getName())) {
throw new InvalidFilenameException(MessageFormat.format(LocaleFactory.localizedString("Cannot create {0}", "Error"), target.getName()));
public void preflight(final Path source, final Path directory, final String filename) throws BackgroundException {
if(!vault.getFilenameProvider().isValid(filename)) {
throw new InvalidFilenameException(MessageFormat.format(LocaleFactory.localizedString("Cannot create {0}", "Error"), filename));
}
proxy.preflight(source, target);
proxy.preflight(source, directory, filename);
}

@Override
Expand Down
Loading
Loading