From 987edb92b255d8928848e76f9f217ddcf2ec1b7e Mon Sep 17 00:00:00 2001 From: LantaoJin Date: Fri, 25 Nov 2016 16:33:44 +0800 Subject: [PATCH] HDFS-11111. Delete items in .Trash using rm should be forbidden without safety option --- .../hadoop/fs/PathIsNotInTrashException.java | 26 ++ .../fs/TrashOptionNotExistsException.java | 26 ++ .../org/apache/hadoop/fs/shell/Delete.java | 20 +- .../apache/hadoop/fs/shell/TestDelete.java | 224 ++++++++++++++++++ 4 files changed, 294 insertions(+), 2 deletions(-) create mode 100644 hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/PathIsNotInTrashException.java create mode 100644 hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/TrashOptionNotExistsException.java create mode 100644 hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/TestDelete.java diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/PathIsNotInTrashException.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/PathIsNotInTrashException.java new file mode 100644 index 00000000000..e35771e2cee --- /dev/null +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/PathIsNotInTrashException.java @@ -0,0 +1,26 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.fs; + +public class PathIsNotInTrashException extends PathExistsException { + static final long serialVersionUID = 0L; + /** @param path for the exception */ + public PathIsNotInTrashException(String path) { + super(path, "Is not in .Trash. A non-trash item can't be deleted using -T option."); + } +} \ No newline at end of file diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/TrashOptionNotExistsException.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/TrashOptionNotExistsException.java new file mode 100644 index 00000000000..fce91bfa2d7 --- /dev/null +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/TrashOptionNotExistsException.java @@ -0,0 +1,26 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.fs; + +public class TrashOptionNotExistsException extends PathExistsException { + static final long serialVersionUID = 0L; + /** @param path for the exception */ + public TrashOptionNotExistsException(String path) { + super(path, "Is in .Trash. Delete it need to add -T option"); + } +} \ No newline at end of file diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Delete.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Delete.java index a0663950159..278aace9f0b 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Delete.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Delete.java @@ -29,8 +29,10 @@ import org.apache.hadoop.fs.ContentSummary; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.PathIOException; import org.apache.hadoop.fs.PathIsDirectoryException; +import org.apache.hadoop.fs.TrashOptionNotExistsException; import org.apache.hadoop.fs.PathIsNotDirectoryException; import org.apache.hadoop.fs.PathIsNotEmptyDirectoryException; +import org.apache.hadoop.fs.PathIsNotInTrashException; import org.apache.hadoop.fs.PathNotFoundException; import org.apache.hadoop.fs.Trash; import org.apache.hadoop.util.ToolRunner; @@ -55,7 +57,7 @@ class Delete { /** remove non-directory paths */ public static class Rm extends FsCommand { public static final String NAME = "rm"; - public static final String USAGE = "[-f] [-r|-R] [-skipTrash] " + + public static final String USAGE = "[-f] [-r|-R] [-skipTrash] [-T]" + "[-safely] ..."; public static final String DESCRIPTION = "Delete all files that match the specified file pattern. " + @@ -65,6 +67,7 @@ class Delete { "-[rR]: Recursively deletes directories.\n" + "-skipTrash: option bypasses trash, if enabled, and immediately " + "deletes .\n" + + "-T: Delete items in .Trash must add this option.\n" + "-safely: option requires safety confirmation, if enabled, " + "requires confirmation before deleting large directory with more " + "than files. Delay is " + @@ -75,15 +78,17 @@ class Delete { private boolean deleteDirs = false; private boolean ignoreFNF = false; private boolean safeDelete = false; + private boolean deleteTrash = false; @Override protected void processOptions(LinkedList args) throws IOException { CommandFormat cf = new CommandFormat( - 1, Integer.MAX_VALUE, "f", "r", "R", "skipTrash", "safely"); + 1, Integer.MAX_VALUE, "f", "r", "R", "skipTrash", "T", "safely"); cf.parse(args); ignoreFNF = cf.getOpt("f"); deleteDirs = cf.getOpt("r") || cf.getOpt("R"); skipTrash = cf.getOpt("skipTrash"); + deleteTrash = cf.getOpt("T"); safeDelete = cf.getOpt("safely"); } @@ -111,6 +116,13 @@ class Delete { throw new PathIsDirectoryException(item.toString()); } + if (deleteTrash && !inTrash(item)) { + throw new PathIsNotInTrashException(item.toString()); + } + if (!deleteTrash && inTrash(item)) { + throw new TrashOptionNotExistsException(item.toString()); + } + // TODO: if the user wants the trash to be used but there is any // problem (ie. creating the trash dir, moving the item to be deleted, // etc), then the path will just be deleted because moveToTrash returns @@ -124,6 +136,10 @@ class Delete { out.println("Deleted " + item); } + private boolean inTrash(PathData item) { + return item.uri.getPath().contains(".Trash"); + } + private boolean canBeSafelyDeleted(PathData item) throws IOException { boolean shouldDelete = true; diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/TestDelete.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/TestDelete.java new file mode 100644 index 00000000000..57e9131161c --- /dev/null +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/TestDelete.java @@ -0,0 +1,224 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hadoop.fs.shell; + +import static org.junit.Assert.*; +import static org.mockito.Matchers.anyBoolean; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.*; + +import java.io.IOException; +import java.net.URI; +import java.net.URISyntaxException; + +import org.apache.hadoop.fs.Path; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.fs.FileStatus; +import org.apache.hadoop.fs.FileSystem; +import org.apache.hadoop.fs.FilterFileSystem; +import org.apache.hadoop.fs.FsServerDefaults; +import org.apache.hadoop.fs.PathIsNotInTrashException; +import org.apache.hadoop.fs.TrashOptionNotExistsException; +import org.apache.hadoop.fs.Options.Rename; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Test; + +public class TestDelete { + static Configuration conf; + static FileSystem mockFs; + + @BeforeClass + public static void setup() throws IOException, URISyntaxException { + mockFs = mock(FileSystem.class); + conf = new Configuration(); + conf.setClass("fs.mockfs.impl", MockFileSystem.class, FileSystem.class); + } + + @Before + public void resetMock() throws IOException { + reset(mockFs); + } + + @Test + public void testDeleteFileInTrashWithoutTrashOption() throws Exception { + Path fileInTrash = new Path("mockfs://user/someone/.Trash/Current/someone/file"); + InstrumentedRM cmd; + String[] cmdargs = new String[]{"mockfs://user/someone/.Trash/Current/someone/file"}; + FileStatus fileInTrash_Stat = mock(FileStatus.class); + + when(fileInTrash_Stat.isDirectory()).thenReturn(false); + when(fileInTrash_Stat.getPath()).thenReturn(fileInTrash); + when(mockFs.getFileStatus(eq(fileInTrash))).thenReturn(fileInTrash_Stat); + + cmd = new InstrumentedRM(); + cmd.setConf(conf); + cmd.run(cmdargs); + + // make sure command failed with the proper exception + assertTrue("Rename should have failed with trash option not exists exception", + cmd.error instanceof TrashOptionNotExistsException); + } + + @Test + public void testDeleteDirectoryInTrashWithoutTrashOption() throws Exception { + Path directoryInTrash = new Path("mockfs://user/someone/.Trash/Current/someone/fold1"); + InstrumentedRM cmd; + String[] cmdargs = new String[]{"-r", "mockfs://user/someone/.Trash/Current/someone/fold1"}; + FileStatus directoryInTrash_Stat = mock(FileStatus.class); + + when(directoryInTrash_Stat.isDirectory()).thenReturn(true); + when(directoryInTrash_Stat.getPath()).thenReturn(directoryInTrash); + when(mockFs.getFileStatus(eq(directoryInTrash))).thenReturn(directoryInTrash_Stat); + + cmd = new InstrumentedRM(); + cmd.setConf(conf); + cmd.run(cmdargs); + + // make sure command failed with the proper exception + assertTrue("RM should have failed with trash option not exists exception", + cmd.error instanceof TrashOptionNotExistsException); + } + + @Test + public void testDeleteFileNotInTrashWithTrashOption() throws Exception { + Path fileNotInTrash = new Path("mockfs://user/someone/fold0/file0"); + InstrumentedRM cmd; + String[] cmdargs = new String[]{"-T", "mockfs://user/someone/fold0/file0"}; + FileStatus fileNotInTrash_Stat = mock(FileStatus.class); + + when(fileNotInTrash_Stat.isDirectory()).thenReturn(false); + when(fileNotInTrash_Stat.getPath()).thenReturn(fileNotInTrash); + when(mockFs.getFileStatus(eq(fileNotInTrash))).thenReturn(fileNotInTrash_Stat); + + cmd = new InstrumentedRM(); + cmd.setConf(conf); + cmd.run(cmdargs); + + // make sure command failed with the proper exception + assertTrue("Rename should have failed with pathIsNotInTrash exception", + cmd.error instanceof PathIsNotInTrashException); + } + + @Test + public void testDeleteDirectoryNotInTrashWithTrashOption() throws Exception { + Path directoryNotInTrash = new Path("mockfs://user/someone/fold0/fold1"); + InstrumentedRM cmd; + String[] cmdargs = new String[]{"-r", "-T", "mockfs://user/someone/fold0/fold1"}; + FileStatus directoryNotInTrash_Stat = mock(FileStatus.class); + + when(directoryNotInTrash_Stat.isDirectory()).thenReturn(true); + when(directoryNotInTrash_Stat.getPath()).thenReturn(directoryNotInTrash); + when(mockFs.getFileStatus(eq(directoryNotInTrash))).thenReturn(directoryNotInTrash_Stat); + + cmd = new InstrumentedRM(); + cmd.setConf(conf); + cmd.run(cmdargs); + + // make sure command failed with the proper exception + assertTrue("Rename should have failed with pathIsNotInTrash exception", + cmd.error instanceof PathIsNotInTrashException); + } + + /* + * hadoop -fs -rm -r /user/someone/ .Trash + * The purpose is to clean trash for saving space. + * But a blank space added before dot by mistake. + * That will delete all data under /user/someone permanently. + * Below test shows that HDFS-11111 can help to avoid this mistake. + */ + @Test + public void testMixedUseCase() throws Exception { + Path trash = new Path("mockfs://user/someone/.Trash"); + Path fileNotInTrash = new Path("mockfs://user/someone/fold0/file0"); + Path directoryInTrash = new Path("mockfs://user/someone/.Trash/Current/someone/fold1"); + Path fileAfterDelete = new Path("mockfs://user/someone/.Trash/Current/someone/fold0/file0"); + + InstrumentedRM cmd; + String[] cmdargs = new String[]{"-r", "mockfs://user/someone/fold0/file0", + "mockfs://user/someone/.Trash/Current/someone/fold1"}; + FileStatus fileNotInTrash_Stat = mock(FileStatus.class); + FileStatus directoryInTrash_Stat = mock(FileStatus.class); + + when(fileNotInTrash_Stat.isDirectory()).thenReturn(false); + when(fileNotInTrash_Stat.getPath()).thenReturn(fileNotInTrash); + when(mockFs.getFileStatus(eq(fileNotInTrash))).thenReturn(fileNotInTrash_Stat); + when(directoryInTrash_Stat.isDirectory()).thenReturn(true); + when(directoryInTrash_Stat.getPath()).thenReturn(directoryInTrash); + when(mockFs.getFileStatus(eq(directoryInTrash))).thenReturn(directoryInTrash_Stat); + + when(mockFs.getTrashRoot(any())).thenReturn(trash); + when(mockFs.mkdirs(eq(new Path("mockfs://user/someone/.Trash/Current/someone/fold0")), any())).thenReturn(true); + when(mockFs.rename(eq(fileNotInTrash), eq(fileAfterDelete))).thenReturn(true); + + cmd = new InstrumentedRM(); + cmd.setConf(conf); + cmd.run(cmdargs); + + verify(mockFs).mkdirs(eq(new Path("mockfs://user/someone/.Trash/Current/someone/fold0")), any()); + verify(mockFs).rename(eq(fileNotInTrash), eq(fileAfterDelete)); + verify(mockFs, never()).delete(eq(directoryInTrash), anyBoolean()); + // make sure command failed with the proper exception + assertTrue("Rename should have failed with trash option not exists exception", + cmd.error instanceof TrashOptionNotExistsException); + } + + static class MockFileSystem extends FilterFileSystem { + Configuration conf; + MockFileSystem() { + super(mockFs); + } + @Override + public void initialize(URI uri, Configuration conf) { + this.conf = conf; + } + @Override + public Path makeQualified(Path path) { + return path; + } + @Override + public Configuration getConf() { + return conf; + } + @Override + public Path resolvePath(Path p) { + return p; + } + @Override + public FsServerDefaults getServerDefaults(Path f) throws IOException { + FsServerDefaults mockFsServerDefaults = mock(FsServerDefaults.class); + when(mockFsServerDefaults.getTrashInterval()).thenReturn(1000L); + return mockFsServerDefaults; + } + @Override + protected void rename(final Path src, final Path dst, + final Rename... options) throws IOException { + mockFs.rename(src, dst); + } + } + + private static class InstrumentedRM extends Delete.Rm { + public static String NAME = "InstrumentedRM"; + private Exception error = null; + @Override + public void displayError(Exception e) { + error = e; + } + } +}