Skip to content

Commit 987edb9

Browse files
LantaoJinLantaoJin
committed
HDFS-11111. Delete items in .Trash using rm should be forbidden without safety option
1 parent 57187fd commit 987edb9

File tree

4 files changed

+294
-2
lines changed

4 files changed

+294
-2
lines changed

‎hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/PathIsNotInTrashException.java

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
/**
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
package org.apache.hadoop.fs;
19+
20+
public class PathIsNotInTrashException extends PathExistsException {
21+
static final long serialVersionUID = 0L;
22+
/** @param path for the exception */
23+
public PathIsNotInTrashException(String path) {
24+
super(path, "Is not in .Trash. A non-trash item can't be deleted using -T option.");
25+
}
26+
}

‎hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/TrashOptionNotExistsException.java

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
/**
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
package org.apache.hadoop.fs;
19+
20+
public class TrashOptionNotExistsException extends PathExistsException {
21+
static final long serialVersionUID = 0L;
22+
/** @param path for the exception */
23+
public TrashOptionNotExistsException(String path) {
24+
super(path, "Is in .Trash. Delete it need to add -T option");
25+
}
26+
}

‎hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Delete.java

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,10 @@
2929
import org.apache.hadoop.fs.FileSystem;
3030
import org.apache.hadoop.fs.PathIOException;
3131
import org.apache.hadoop.fs.PathIsDirectoryException;
32+
import org.apache.hadoop.fs.TrashOptionNotExistsException;
3233
import org.apache.hadoop.fs.PathIsNotDirectoryException;
3334
import org.apache.hadoop.fs.PathIsNotEmptyDirectoryException;
35+
import org.apache.hadoop.fs.PathIsNotInTrashException;
3436
import org.apache.hadoop.fs.PathNotFoundException;
3537
import org.apache.hadoop.fs.Trash;
3638
import org.apache.hadoop.util.ToolRunner;
@@ -55,7 +57,7 @@ public static void registerCommands(CommandFactory factory) {
5557
/** remove non-directory paths */
5658
public static class Rm extends FsCommand {
5759
public static final String NAME = "rm";
58-
public static final String USAGE = "[-f] [-r|-R] [-skipTrash] " +
60+
public static final String USAGE = "[-f] [-r|-R] [-skipTrash] [-T]" +
5961
"[-safely] <src> ...";
6062
public static final String DESCRIPTION =
6163
"Delete all files that match the specified file pattern. " +
@@ -65,6 +67,7 @@ public static class Rm extends FsCommand {
6567
"-[rR]: Recursively deletes directories.\n" +
6668
"-skipTrash: option bypasses trash, if enabled, and immediately " +
6769
"deletes <src>.\n" +
70+
"-T: Delete items in .Trash must add this option.\n" +
6871
"-safely: option requires safety confirmation, if enabled, " +
6972
"requires confirmation before deleting large directory with more " +
7073
"than <hadoop.shell.delete.limit.num.files> files. Delay is " +
@@ -75,15 +78,17 @@ public static class Rm extends FsCommand {
7578
private boolean deleteDirs = false;
7679
private boolean ignoreFNF = false;
7780
private boolean safeDelete = false;
81+
private boolean deleteTrash = false;
7882

7983
@Override
8084
protected void processOptions(LinkedList<String> args) throws IOException {
8185
CommandFormat cf = new CommandFormat(
82-
1, Integer.MAX_VALUE, "f", "r", "R", "skipTrash", "safely");
86+
1, Integer.MAX_VALUE, "f", "r", "R", "skipTrash", "T", "safely");
8387
cf.parse(args);
8488
ignoreFNF = cf.getOpt("f");
8589
deleteDirs = cf.getOpt("r") || cf.getOpt("R");
8690
skipTrash = cf.getOpt("skipTrash");
91+
deleteTrash = cf.getOpt("T");
8792
safeDelete = cf.getOpt("safely");
8893
}
8994

@@ -111,6 +116,13 @@ protected void processPath(PathData item) throws IOException {
111116
throw new PathIsDirectoryException(item.toString());
112117
}
113118

119+
if (deleteTrash && !inTrash(item)) {
120+
throw new PathIsNotInTrashException(item.toString());
121+
}
122+
if (!deleteTrash && inTrash(item)) {
123+
throw new TrashOptionNotExistsException(item.toString());
124+
}
125+
114126
// TODO: if the user wants the trash to be used but there is any
115127
// problem (ie. creating the trash dir, moving the item to be deleted,
116128
// etc), then the path will just be deleted because moveToTrash returns
@@ -124,6 +136,10 @@ protected void processPath(PathData item) throws IOException {
124136
out.println("Deleted " + item);
125137
}
126138

139+
private boolean inTrash(PathData item) {
140+
return item.uri.getPath().contains(".Trash");
141+
}
142+
127143
private boolean canBeSafelyDeleted(PathData item)
128144
throws IOException {
129145
boolean shouldDelete = true;

‎hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/TestDelete.java

Lines changed: 224 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,224 @@
1+
/**
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
19+
package org.apache.hadoop.fs.shell;
20+
21+
import static org.junit.Assert.*;
22+
import static org.mockito.Matchers.anyBoolean;
23+
import static org.mockito.Matchers.eq;
24+
import static org.mockito.Mockito.*;
25+
26+
import java.io.IOException;
27+
import java.net.URI;
28+
import java.net.URISyntaxException;
29+
30+
import org.apache.hadoop.fs.Path;
31+
import org.apache.hadoop.conf.Configuration;
32+
import org.apache.hadoop.fs.FileStatus;
33+
import org.apache.hadoop.fs.FileSystem;
34+
import org.apache.hadoop.fs.FilterFileSystem;
35+
import org.apache.hadoop.fs.FsServerDefaults;
36+
import org.apache.hadoop.fs.PathIsNotInTrashException;
37+
import org.apache.hadoop.fs.TrashOptionNotExistsException;
38+
import org.apache.hadoop.fs.Options.Rename;
39+
import org.junit.Before;
40+
import org.junit.BeforeClass;
41+
import org.junit.Test;
42+
43+
public class TestDelete {
44+
static Configuration conf;
45+
static FileSystem mockFs;
46+
47+
@BeforeClass
48+
public static void setup() throws IOException, URISyntaxException {
49+
mockFs = mock(FileSystem.class);
50+
conf = new Configuration();
51+
conf.setClass("fs.mockfs.impl", MockFileSystem.class, FileSystem.class);
52+
}
53+
54+
@Before
55+
public void resetMock() throws IOException {
56+
reset(mockFs);
57+
}
58+
59+
@Test
60+
public void testDeleteFileInTrashWithoutTrashOption() throws Exception {
61+
Path fileInTrash = new Path("mockfs://user/someone/.Trash/Current/someone/file");
62+
InstrumentedRM cmd;
63+
String[] cmdargs = new String[]{"mockfs://user/someone/.Trash/Current/someone/file"};
64+
FileStatus fileInTrash_Stat = mock(FileStatus.class);
65+
66+
when(fileInTrash_Stat.isDirectory()).thenReturn(false);
67+
when(fileInTrash_Stat.getPath()).thenReturn(fileInTrash);
68+
when(mockFs.getFileStatus(eq(fileInTrash))).thenReturn(fileInTrash_Stat);
69+
70+
cmd = new InstrumentedRM();
71+
cmd.setConf(conf);
72+
cmd.run(cmdargs);
73+
74+
// make sure command failed with the proper exception
75+
assertTrue("Rename should have failed with trash option not exists exception",
76+
cmd.error instanceof TrashOptionNotExistsException);
77+
}
78+
79+
@Test
80+
public void testDeleteDirectoryInTrashWithoutTrashOption() throws Exception {
81+
Path directoryInTrash = new Path("mockfs://user/someone/.Trash/Current/someone/fold1");
82+
InstrumentedRM cmd;
83+
String[] cmdargs = new String[]{"-r", "mockfs://user/someone/.Trash/Current/someone/fold1"};
84+
FileStatus directoryInTrash_Stat = mock(FileStatus.class);
85+
86+
when(directoryInTrash_Stat.isDirectory()).thenReturn(true);
87+
when(directoryInTrash_Stat.getPath()).thenReturn(directoryInTrash);
88+
when(mockFs.getFileStatus(eq(directoryInTrash))).thenReturn(directoryInTrash_Stat);
89+
90+
cmd = new InstrumentedRM();
91+
cmd.setConf(conf);
92+
cmd.run(cmdargs);
93+
94+
// make sure command failed with the proper exception
95+
assertTrue("RM should have failed with trash option not exists exception",
96+
cmd.error instanceof TrashOptionNotExistsException);
97+
}
98+
99+
@Test
100+
public void testDeleteFileNotInTrashWithTrashOption() throws Exception {
101+
Path fileNotInTrash = new Path("mockfs://user/someone/fold0/file0");
102+
InstrumentedRM cmd;
103+
String[] cmdargs = new String[]{"-T", "mockfs://user/someone/fold0/file0"};
104+
FileStatus fileNotInTrash_Stat = mock(FileStatus.class);
105+
106+
when(fileNotInTrash_Stat.isDirectory()).thenReturn(false);
107+
when(fileNotInTrash_Stat.getPath()).thenReturn(fileNotInTrash);
108+
when(mockFs.getFileStatus(eq(fileNotInTrash))).thenReturn(fileNotInTrash_Stat);
109+
110+
cmd = new InstrumentedRM();
111+
cmd.setConf(conf);
112+
cmd.run(cmdargs);
113+
114+
// make sure command failed with the proper exception
115+
assertTrue("Rename should have failed with pathIsNotInTrash exception",
116+
cmd.error instanceof PathIsNotInTrashException);
117+
}
118+
119+
@Test
120+
public void testDeleteDirectoryNotInTrashWithTrashOption() throws Exception {
121+
Path directoryNotInTrash = new Path("mockfs://user/someone/fold0/fold1");
122+
InstrumentedRM cmd;
123+
String[] cmdargs = new String[]{"-r", "-T", "mockfs://user/someone/fold0/fold1"};
124+
FileStatus directoryNotInTrash_Stat = mock(FileStatus.class);
125+
126+
when(directoryNotInTrash_Stat.isDirectory()).thenReturn(true);
127+
when(directoryNotInTrash_Stat.getPath()).thenReturn(directoryNotInTrash);
128+
when(mockFs.getFileStatus(eq(directoryNotInTrash))).thenReturn(directoryNotInTrash_Stat);
129+
130+
cmd = new InstrumentedRM();
131+
cmd.setConf(conf);
132+
cmd.run(cmdargs);
133+
134+
// make sure command failed with the proper exception
135+
assertTrue("Rename should have failed with pathIsNotInTrash exception",
136+
cmd.error instanceof PathIsNotInTrashException);
137+
}
138+
139+
/*
140+
* hadoop -fs -rm -r /user/someone/ .Trash
141+
* The purpose is to clean trash for saving space.
142+
* But a blank space added before dot by mistake.
143+
* That will delete all data under /user/someone permanently.
144+
* Below test shows that HDFS-11111 can help to avoid this mistake.
145+
*/
146+
@Test
147+
public void testMixedUseCase() throws Exception {
148+
Path trash = new Path("mockfs://user/someone/.Trash");
149+
Path fileNotInTrash = new Path("mockfs://user/someone/fold0/file0");
150+
Path directoryInTrash = new Path("mockfs://user/someone/.Trash/Current/someone/fold1");
151+
Path fileAfterDelete = new Path("mockfs://user/someone/.Trash/Current/someone/fold0/file0");
152+
153+
InstrumentedRM cmd;
154+
String[] cmdargs = new String[]{"-r", "mockfs://user/someone/fold0/file0",
155+
"mockfs://user/someone/.Trash/Current/someone/fold1"};
156+
FileStatus fileNotInTrash_Stat = mock(FileStatus.class);
157+
FileStatus directoryInTrash_Stat = mock(FileStatus.class);
158+
159+
when(fileNotInTrash_Stat.isDirectory()).thenReturn(false);
160+
when(fileNotInTrash_Stat.getPath()).thenReturn(fileNotInTrash);
161+
when(mockFs.getFileStatus(eq(fileNotInTrash))).thenReturn(fileNotInTrash_Stat);
162+
when(directoryInTrash_Stat.isDirectory()).thenReturn(true);
163+
when(directoryInTrash_Stat.getPath()).thenReturn(directoryInTrash);
164+
when(mockFs.getFileStatus(eq(directoryInTrash))).thenReturn(directoryInTrash_Stat);
165+
166+
when(mockFs.getTrashRoot(any())).thenReturn(trash);
167+
when(mockFs.mkdirs(eq(new Path("mockfs://user/someone/.Trash/Current/someone/fold0")), any())).thenReturn(true);
168+
when(mockFs.rename(eq(fileNotInTrash), eq(fileAfterDelete))).thenReturn(true);
169+
170+
cmd = new InstrumentedRM();
171+
cmd.setConf(conf);
172+
cmd.run(cmdargs);
173+
174+
verify(mockFs).mkdirs(eq(new Path("mockfs://user/someone/.Trash/Current/someone/fold0")), any());
175+
verify(mockFs).rename(eq(fileNotInTrash), eq(fileAfterDelete));
176+
verify(mockFs, never()).delete(eq(directoryInTrash), anyBoolean());
177+
// make sure command failed with the proper exception
178+
assertTrue("Rename should have failed with trash option not exists exception",
179+
cmd.error instanceof TrashOptionNotExistsException);
180+
}
181+
182+
static class MockFileSystem extends FilterFileSystem {
183+
Configuration conf;
184+
MockFileSystem() {
185+
super(mockFs);
186+
}
187+
@Override
188+
public void initialize(URI uri, Configuration conf) {
189+
this.conf = conf;
190+
}
191+
@Override
192+
public Path makeQualified(Path path) {
193+
return path;
194+
}
195+
@Override
196+
public Configuration getConf() {
197+
return conf;
198+
}
199+
@Override
200+
public Path resolvePath(Path p) {
201+
return p;
202+
}
203+
@Override
204+
public FsServerDefaults getServerDefaults(Path f) throws IOException {
205+
FsServerDefaults mockFsServerDefaults = mock(FsServerDefaults.class);
206+
when(mockFsServerDefaults.getTrashInterval()).thenReturn(1000L);
207+
return mockFsServerDefaults;
208+
}
209+
@Override
210+
protected void rename(final Path src, final Path dst,
211+
final Rename... options) throws IOException {
212+
mockFs.rename(src, dst);
213+
}
214+
}
215+
216+
private static class InstrumentedRM extends Delete.Rm {
217+
public static String NAME = "InstrumentedRM";
218+
private Exception error = null;
219+
@Override
220+
public void displayError(Exception e) {
221+
error = e;
222+
}
223+
}
224+
}

0 commit comments

Comments
 (0)