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.

Sign up for GitHub

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

Add preflight checks. #16668

Merged
merged 19 commits into from
Dec 14, 2024
Merged

Add preflight checks. #16668

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
6952e1e
Include file reference in failure.
dkocher Dec 12, 2024
012daf9
Fix #16667.
dkocher Dec 12, 2024
77a9cee
Only fail preflight check for copy operation.
dkocher Dec 13, 2024
f334fab
Add test.
dkocher Dec 13, 2024
e049392
Ensure target is not deleted on case change rename only.
dkocher Dec 13, 2024
c80e5d5
Ensure case-insensitive lookup.
dkocher Dec 13, 2024
63c8d4d
Redundant normalization.
dkocher Dec 13, 2024
effd5ce
Ensure case-insensitive lookup.
dkocher Dec 13, 2024
0c90dac
Ensure target is not deleted on case change rename only.
dkocher Dec 13, 2024
cdb9159
Add test.
dkocher Dec 13, 2024
3914e6f
Remove redundant overrides.
dkocher Dec 13, 2024
5fd2458
Deny override of itself.
dkocher Dec 13, 2024
5ece443
Validate case-sensitive by default.
dkocher Dec 13, 2024
a7b6517
Ensure target is not deleted on case change rename only.
dkocher Dec 13, 2024
0186092
Make aware of case sensitivity.
dkocher Dec 13, 2024
a2834e8
Assume case-insensitive filesystem as `IOCase` makes wrong assumptions.
dkocher Dec 13, 2024
bbaff14
Fix target argument for validation.
dkocher Dec 13, 2024
5fe23d7
Fix tests.
dkocher Dec 13, 2024
2618d46
Add missing annotation.
ylangisc Dec 13, 2024
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
3 changes: 3 additions & 0 deletions box/src/main/java/ch/cyberduck/core/box/BoxCopyFeature.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@
import java.util.Collections;
import java.util.EnumSet;

import static ch.cyberduck.core.features.Copy.validate;

public class BoxCopyFeature implements Copy {
private static final Logger log = LogManager.getLogger(BoxCopyFeature.class);

Expand Down Expand Up @@ -89,5 +91,6 @@ public void preflight(final Path source, final Path target) throws BackgroundExc
if(!BoxTouchFeature.validate(target.getName())) {
throw new InvalidFilenameException(MessageFormat.format(LocaleFactory.localizedString("Cannot create {0}", "Error"), target.getName()));
}
validate(session.getCaseSensitivity(), source, target);
}
}
4 changes: 2 additions & 2 deletions box/src/main/java/ch/cyberduck/core/box/BoxFileidProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@
*/

import ch.cyberduck.core.CachingFileIdProvider;
import ch.cyberduck.core.CaseInsensitivePathPredicate;
import ch.cyberduck.core.DisabledListProgressListener;
import ch.cyberduck.core.Path;
import ch.cyberduck.core.SimplePathPredicate;
import ch.cyberduck.core.exception.BackgroundException;
import ch.cyberduck.core.exception.NotfoundException;
import ch.cyberduck.core.features.FileIdProvider;
Expand Down Expand Up @@ -53,7 +53,7 @@ public String getFileId(final Path file) throws BackgroundException {
return ROOT;
}
final Path f = new BoxListService(session, this).list(file.getParent(),
new DisabledListProgressListener()).find(new SimplePathPredicate(file));
new DisabledListProgressListener()).find(new CaseInsensitivePathPredicate(file));
if(null == f) {
throw new NotfoundException(file.getAbsolute());
}
Expand Down
8 changes: 5 additions & 3 deletions box/src/main/java/ch/cyberduck/core/box/BoxMoveFeature.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,10 @@ public BoxMoveFeature(final BoxSession session, final BoxFileidProvider fileid)
public Path move(final Path file, final Path renamed, final TransferStatus status, final Delete.Callback delete, final ConnectionCallback callback) throws BackgroundException {
try {
if(status.isExists()) {
log.warn("Delete file {} to be replaced with {}", renamed, file);
new BoxDeleteFeature(session, fileid).delete(Collections.singletonList(renamed), callback, delete);
if(!fileid.getFileId(file).equals(fileid.getFileId(renamed))) {
log.warn("Delete file {} to be replaced with {}", renamed, file);
new BoxDeleteFeature(session, fileid).delete(Collections.singletonList(renamed), callback, delete);
}
}
final String id = fileid.getFileId(file);
if(file.isDirectory()) {
Expand Down Expand Up @@ -93,7 +95,7 @@ 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()));
throw new InvalidFilenameException(MessageFormat.format(LocaleFactory.localizedString("Cannot create {0}", "Error"), target.getName())).withFile(source);
}
}
}
36 changes: 35 additions & 1 deletion box/src/test/java/ch/cyberduck/core/box/BoxFileidProviderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,24 @@
* GNU General Public License for more details.
*/

import ch.cyberduck.core.AlphanumericRandomStringService;
import ch.cyberduck.core.DisabledLoginCallback;
import ch.cyberduck.core.Path;
import ch.cyberduck.core.PathAttributes;
import ch.cyberduck.core.exception.NotfoundException;
import ch.cyberduck.core.features.Delete;
import ch.cyberduck.core.shared.DefaultHomeFinderService;
import ch.cyberduck.core.transfer.TransferStatus;
import ch.cyberduck.test.IntegrationTest;

import org.apache.commons.lang3.StringUtils;
import org.junit.Test;
import org.junit.experimental.categories.Category;

import java.util.Collections;
import java.util.EnumSet;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.*;

@Category(IntegrationTest.class)
public class BoxFileidProviderTest extends AbstractBoxTest {
Expand All @@ -33,4 +42,29 @@ public void getFileIdRoot() throws Exception {
assertEquals(BoxFileidProvider.ROOT, new BoxFileidProvider(session).getFileId(
new Path("/", EnumSet.of(Path.Type.directory))));
}

@Test
public void getFileIdFile() throws Exception {
final BoxFileidProvider nodeid = new BoxFileidProvider(session);
final Path home = new DefaultHomeFinderService(session).find();
final String name = String.format("%s%s", new AlphanumericRandomStringService().random(), new AlphanumericRandomStringService().random());
final Path file = new BoxTouchFeature(session, nodeid).touch(new Path(home, name, EnumSet.of(Path.Type.file)), new TransferStatus());
nodeid.clear();
final String nodeId = nodeid.getFileId(new Path(home, name, EnumSet.of(Path.Type.file)));
assertNotNull(nodeId);
nodeid.clear();
assertEquals(nodeId, nodeid.getFileId(new Path(home.withAttributes(PathAttributes.EMPTY), name, EnumSet.of(Path.Type.file))));
nodeid.clear();
assertEquals(nodeId, nodeid.getFileId(new Path(home, StringUtils.upperCase(name), EnumSet.of(Path.Type.file))));
nodeid.clear();
assertEquals(nodeId, nodeid.getFileId(new Path(home, StringUtils.lowerCase(name), EnumSet.of(Path.Type.file))));
try {
assertNull(nodeid.getFileId(new Path(home, new AlphanumericRandomStringService().random(), EnumSet.of(Path.Type.file))));
fail();
}
catch(NotfoundException e) {
// Expected
}
new BoxDeleteFeature(session, nodeid).delete(Collections.singletonList(file), new DisabledLoginCallback(), new Delete.DisabledCallback());
}
}
11 changes: 11 additions & 0 deletions box/src/test/java/ch/cyberduck/core/box/BoxMoveFeatureTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import ch.cyberduck.core.transfer.TransferStatus;
import ch.cyberduck.test.IntegrationTest;

import org.apache.commons.lang3.StringUtils;
import org.junit.Test;
import org.junit.experimental.categories.Category;

Expand Down Expand Up @@ -90,4 +91,14 @@ public void testMoveNotFound() throws Exception {
final Path test = new Path(new DefaultHomeFinderService(session).find(), new AlphanumericRandomStringService().random(), EnumSet.of(Path.Type.file));
new BoxMoveFeature(session, fileid).move(test, new Path(new DefaultHomeFinderService(session).find(), new AlphanumericRandomStringService().random(), EnumSet.of(Path.Type.file)), new TransferStatus(), new Delete.DisabledCallback(), new DisabledConnectionCallback());
}

@Test
public void testRenameCaseOnly() throws Exception {
final BoxFileidProvider fileid = new BoxFileidProvider(session);
final String name = new AlphanumericRandomStringService().random();
final Path file = new BoxTouchFeature(session, fileid).touch(new Path(new DefaultHomeFinderService(session).find(), StringUtils.capitalize(name), EnumSet.of(Path.Type.file)), new TransferStatus());
final Path rename = new Path(new DefaultHomeFinderService(session).find(), StringUtils.lowerCase(name), EnumSet.of(Path.Type.file));
new BoxMoveFeature(session, fileid).move(file, rename, new TransferStatus().exists(true), new Delete.DisabledCallback(), new DisabledConnectionCallback());
new BoxDeleteFeature(session, fileid).delete(Collections.singletonList(rename), new DisabledLoginCallback(), new Delete.DisabledCallback());
}
}
8 changes: 8 additions & 0 deletions brick/src/main/java/ch/cyberduck/core/brick/BrickCopyFeature.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@
import java.util.Collections;
import java.util.EnumSet;

import static ch.cyberduck.core.features.Copy.validate;

public class BrickCopyFeature extends BrickFileMigrationFeature implements Copy {
private static final Logger log = LogManager.getLogger(BrickCopyFeature.class);

Expand Down Expand Up @@ -69,4 +71,10 @@ public Path copy(final Path file, final Path target, final TransferStatus status
public EnumSet<Flags> features(final Path source, final Path target) {
return EnumSet.of(Flags.recursive);
}

@Override
public void preflight(final Path source, final Path target) throws BackgroundException {
Copy.super.preflight(source, target);
validate(session.getCaseSensitivity(), source, target);
}
}
13 changes: 6 additions & 7 deletions core/src/main/java/ch/cyberduck/core/CaseInsensitivePathPredicate.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,17 @@
* GNU General Public License for more details.
*/

import ch.cyberduck.core.unicode.NFCNormalizer;
import ch.cyberduck.core.unicode.UnicodeNormalizer;

import org.apache.commons.lang3.StringUtils;

public final class CaseInsensitivePathPredicate extends SimplePathPredicate {

private static final UnicodeNormalizer normalizer = new NFCNormalizer();
public class CaseInsensitivePathPredicate extends SimplePathPredicate {

public CaseInsensitivePathPredicate(final Path file) {
super(file.isSymbolicLink() ? Path.Type.symboliclink : file.isFile() ? Path.Type.file : Path.Type.directory,
StringUtils.lowerCase(normalizer.normalize(file.getAbsolute()).toString()));
StringUtils.lowerCase(file.getAbsolute()));
}

public CaseInsensitivePathPredicate(final Path.Type type, final String path) {
super(type, StringUtils.lowerCase(path));
}

@Override
Expand Down
27 changes: 27 additions & 0 deletions core/src/main/java/ch/cyberduck/core/features/Copy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,13 @@
* GNU General Public License for more details.
*/

import ch.cyberduck.core.CaseInsensitivePathPredicate;
import ch.cyberduck.core.ConnectionCallback;
import ch.cyberduck.core.LocaleFactory;
import ch.cyberduck.core.Path;
import ch.cyberduck.core.Protocol;
import ch.cyberduck.core.Session;
import ch.cyberduck.core.SimplePathPredicate;
import ch.cyberduck.core.exception.AccessDeniedException;
import ch.cyberduck.core.exception.BackgroundException;
import ch.cyberduck.core.exception.InvalidFilenameException;
Expand Down Expand Up @@ -86,8 +89,32 @@ default void preflight(final Path source, final Path target) throws BackgroundEx
throw new UnsupportedException(MessageFormat.format(LocaleFactory.localizedString("Cannot copy {0}", "Error"),
source.getName())).withFile(source);
}
validate(Protocol.Case.sensitive, source, target);
}

/**
* Fail when source and target only differ with change in case of filename and protocol is case-insensitive
*
* @throws UnsupportedException Copying file to same location is not supported
*/
static void validate(final Protocol.Case sensitivity, final Path source, final Path target) throws BackgroundException {
switch(sensitivity) {
case insensitive:
if(new CaseInsensitivePathPredicate(source).test(target)) {
throw new UnsupportedException(MessageFormat.format(LocaleFactory.localizedString("Cannot copy {0}", "Error"),
source.getName())).withFile(source);
}
break;
case sensitive:
if(new SimplePathPredicate(source).test(target)) {
throw new AccessDeniedException(MessageFormat.format(LocaleFactory.localizedString("Cannot copy {0}", "Error"),
source.getName())).withFile(source);
}
break;
}
}


/**
* @return Supported features
*/
Expand Down
13 changes: 9 additions & 4 deletions core/src/main/java/ch/cyberduck/core/features/Move.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import ch.cyberduck.core.LocaleFactory;
import ch.cyberduck.core.Path;
import ch.cyberduck.core.Session;
import ch.cyberduck.core.SimplePathPredicate;
import ch.cyberduck.core.exception.AccessDeniedException;
import ch.cyberduck.core.exception.BackgroundException;
import ch.cyberduck.core.exception.InvalidFilenameException;
Expand Down Expand Up @@ -85,15 +86,19 @@ default void preflight(final Path source, final Path target) throws BackgroundEx
throw new AccessDeniedException(MessageFormat.format(LocaleFactory.localizedString("Cannot rename {0}", "Error"),
source.getName())).withFile(source);
}
// Deny move to self
if(new SimplePathPredicate(source).test(target)) {
throw new AccessDeniedException(MessageFormat.format(LocaleFactory.localizedString("Cannot rename {0}", "Error"),
source.getName())).withFile(source);
}
}


/**
* @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
2 changes: 1 addition & 1 deletion cryptomator/src/main/java/ch/cyberduck/core/cryptomator/features/CryptoMoveV6Feature.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ 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()));
throw new InvalidFilenameException(MessageFormat.format(LocaleFactory.localizedString("Cannot create {0}", "Error"), target.getName())).withFile(source);
}
proxy.preflight(source, target);
}
Expand Down
2 changes: 1 addition & 1 deletion cryptomator/src/main/java/ch/cyberduck/core/cryptomator/features/CryptoMoveV7Feature.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ 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()));
throw new InvalidFilenameException(MessageFormat.format(LocaleFactory.localizedString("Cannot create {0}", "Error"), target.getName())).withFile(source);
}
proxy.preflight(source, target);
}
Expand Down
2 changes: 1 addition & 1 deletion ctera/src/main/java/ch/cyberduck/core/ctera/CteraMoveFeature.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public CteraMoveFeature(final CteraSession session) {
@Override
public void preflight(final Path source, final Path target) throws BackgroundException {
if(!CteraTouchFeature.validate(target.getName())) {
throw new InvalidFilenameException(MessageFormat.format(LocaleFactory.localizedString("Cannot rename {0}", "Error"), source.getName())).withFile(source);
throw new InvalidFilenameException(MessageFormat.format(LocaleFactory.localizedString("Cannot rename {0}", "Error"), target.getName())).withFile(source);
}
assumeRole(source, DELETEPERMISSION);
// defaults to Acl.EMPTY (disabling role checking) if target does not exist
Expand Down
2 changes: 1 addition & 1 deletion deepbox/src/main/java/ch/cyberduck/core/deepbox/DeepboxMoveFeature.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ public void preflight(final Path source, final Path target) throws BackgroundExc
throw new AccessDeniedException(MessageFormat.format(LocaleFactory.localizedString("Cannot rename {0}", "Error"), source.getName())).withFile(source);
}
if(target.isRoot() || new DeepboxPathContainerService(session, fileid).isContainer(target) || new DeepboxPathContainerService(session, fileid).isInTrash(target)) {
throw new AccessDeniedException(MessageFormat.format(LocaleFactory.localizedString("Cannot create {0}", "Error"), target.getName()));
throw new AccessDeniedException(MessageFormat.format(LocaleFactory.localizedString("Cannot create {0}", "Error"), target.getName())).withFile(source);
}
final Acl acl = source.attributes().getAcl();
if(Acl.EMPTY == acl) {
Expand Down
4 changes: 3 additions & 1 deletion deepbox/src/test/java/ch/cyberduck/core/deepbox/DeepboxFindFeatureTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import ch.cyberduck.core.transfer.TransferStatus;
import ch.cyberduck.test.IntegrationTest;

import org.apache.commons.lang3.StringUtils;
import org.junit.Test;
import org.junit.experimental.categories.Category;

Expand Down Expand Up @@ -66,10 +67,11 @@ public void testFindDirectory() throws Exception {
@Test
public void testFindFile() throws Exception {
final Path box = new Path("/ORG 4 - DeepBox Desktop App/ORG 4 - DeepBox Desktop App/ORG3:Box1/Documents", EnumSet.of(Path.Type.directory, Path.Type.volume));
final Path file = new Path(box, new AlphanumericRandomStringService().random(), EnumSet.of(Path.Type.file));
final Path file = new Path(box, StringUtils.lowerCase(new AlphanumericRandomStringService().random()), EnumSet.of(Path.Type.file));
final DeepboxIdProvider nodeid = new DeepboxIdProvider(session);
new DeepboxTouchFeature(session, nodeid).touch(file, new TransferStatus());
assertTrue(new DeepboxFindFeature(session, nodeid).find(file));
assertFalse(new DeepboxFindFeature(session, nodeid).find(new Path(box, StringUtils.upperCase(file.getName()), EnumSet.of(Path.Type.file))));
assertFalse(new DeepboxFindFeature(session, nodeid).find(new Path(file.getAbsolute(), EnumSet.of(Path.Type.directory))));
new DeepboxDeleteFeature(session, nodeid).delete(Collections.singletonList(file), new DisabledLoginCallback(), new Delete.DisabledCallback());
}
Expand Down
3 changes: 3 additions & 0 deletions dracoon/src/main/java/ch/cyberduck/core/sds/SDSCopyFeature.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@
import java.util.EnumSet;
import java.util.Objects;

import static ch.cyberduck.core.features.Copy.validate;

public class SDSCopyFeature implements Copy {
private static final Logger log = LogManager.getLogger(SDSCopyFeature.class);

Expand Down Expand Up @@ -101,5 +103,6 @@ public void preflight(final Path source, final Path target) throws BackgroundExc
log.warn("Deny copy of {} to {}", source, target);
throw new UnsupportedException(MessageFormat.format(LocaleFactory.localizedString("Cannot copy {0}", "Error"), source.getName())).withFile(source);
}
validate(session.getCaseSensitivity(), source, target);
}
}
2 changes: 1 addition & 1 deletion dracoon/src/main/java/ch/cyberduck/core/sds/SDSMoveFeature.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ public void preflight(final Path source, final Path target) throws BackgroundExc
}
if(!SDSTouchFeature.validate(target.getName())) {
log.warn("Validation failed for target name {}", target);
throw new InvalidFilenameException(MessageFormat.format(LocaleFactory.localizedString("Cannot rename {0}", "Error"), source.getName())).withFile(target);
throw new InvalidFilenameException(MessageFormat.format(LocaleFactory.localizedString("Cannot rename {0}", "Error"), target.getName())).withFile(source);
}
final SDSPermissionsFeature acl = new SDSPermissionsFeature(session, nodeid);
if(!new SimplePathPredicate(source.getParent()).test(target.getParent())) {
Expand Down
3 changes: 3 additions & 0 deletions dropbox/src/main/java/ch/cyberduck/core/dropbox/DropboxCopyFeature.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@
import com.dropbox.core.v2.files.DbxUserFilesRequests;
import com.dropbox.core.v2.files.RelocationResult;

import static ch.cyberduck.core.features.Copy.validate;

public class DropboxCopyFeature implements Copy {
private static final Logger log = LogManager.getLogger(DropboxCopyFeature.class);

Expand Down Expand Up @@ -75,5 +77,6 @@ public void preflight(final Path source, final Path target) throws BackgroundExc
if(!DropboxTouchFeature.validate(target.getName())) {
throw new InvalidFilenameException(MessageFormat.format(LocaleFactory.localizedString("Cannot create {0}", "Error"), target.getName()));
}
validate(session.getCaseSensitivity(), source, target);
}
}
Loading
Loading