diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/localizer/ContainerLocalizer.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/localizer/ContainerLocalizer.java index 32e3553..3abc085 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/localizer/ContainerLocalizer.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/localizer/ContainerLocalizer.java @@ -69,6 +69,7 @@ import org.apache.hadoop.yarn.util.ConverterUtils; import org.apache.hadoop.yarn.util.FSDownload; +import com.google.common.annotations.VisibleForTesting; import com.google.common.util.concurrent.ThreadFactoryBuilder; public class ContainerLocalizer { @@ -196,10 +197,14 @@ ExecutorService createDownloadThreadPool() { Callable download(Path path, LocalResource rsrc, UserGroupInformation ugi) throws IOException { - DiskChecker.checkDir(new File(path.toUri().getRawPath())); return new FSDownload(lfs, ugi, conf, path, rsrc); } + @VisibleForTesting + void checkDir(File path) throws IOException { + DiskChecker.checkDir(path); + } + static long getEstimatedSize(LocalResource rsrc) { if (rsrc.getSize() < 0) { return -1; @@ -238,9 +243,10 @@ protected void localizeFiles(LocalizationProtocol nodemanager, List newRsrcs = response.getResourceSpecs(); for (ResourceLocalizationSpec newRsrc : newRsrcs) { if (!pendingResources.containsKey(newRsrc.getResource())) { + Path path = new Path(newRsrc.getDestinationDirectory().getFile()); + this.checkDir(new File(path.toUri().getPath())); pendingResources.put(newRsrc.getResource(), cs.submit(download( - new Path(newRsrc.getDestinationDirectory().getFile()), - newRsrc.getResource(), ugi))); + path, newRsrc.getResource(), ugi))); } } break; diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/localizer/TestContainerLocalizer.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/localizer/TestContainerLocalizer.java index 22fad6f..04899d1 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/localizer/TestContainerLocalizer.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/localizer/TestContainerLocalizer.java @@ -36,6 +36,7 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import java.io.File; import java.io.IOException; import java.net.InetSocketAddress; import java.util.ArrayList; @@ -237,6 +238,53 @@ public void testContainerLocalizerClosesFilesystems() throws Exception { verify(localizer).closeFileSystems(any(UserGroupInformation.class)); } + @Test + public void testLocalizerDiskCheckDoesNotUriEncodePath() throws Exception { + FileContext fs = FileContext.getLocalFSFileContext(); + spylfs = spy(fs.getDefaultFileSystem()); + ContainerLocalizer localizer = setupContainerLocalizerForTest(); + + // Use a resource path containing a character that would require encoding in + // URI form. + doDiskCheckTest(localizer, "my\\Resource"); + } + + @Test + public void testLocalizerDiskCheckDoesNotUriDecodePath() throws Exception { + FileContext fs = FileContext.getLocalFSFileContext(); + spylfs = spy(fs.getDefaultFileSystem()); + ContainerLocalizer localizer = setupContainerLocalizerForTest(); + + // Use a resource path containing something that looks like it's URI-encoded + // and verify that the same path is checked (no decoding). + doDiskCheckTest(localizer, "my%5CResource"); + } + + private void doDiskCheckTest(ContainerLocalizer localizer, String testPath) + throws Exception { + Path base = new Path(new Path( + localDirs.get(0), ContainerLocalizer.USERCACHE), appUser); + Path privcache = new Path(base, ContainerLocalizer.FILECACHE); + String rsrcPath = new Path(privcache, testPath).toUri().getPath(); + ResourceLocalizationSpec rsrc = getMockRsrc(random, + LocalResourceVisibility.PRIVATE, new Path(rsrcPath)); + + when(nmProxy.heartbeat(isA(LocalizerStatus.class))) + .thenReturn(new MockLocalizerHeartbeatResponse(LocalizerAction.LIVE, + Collections.singletonList(rsrc))) + .thenReturn(new MockLocalizerHeartbeatResponse(LocalizerAction.DIE, + null)); + LocalResource tRsrc = rsrc.getResource(); + doReturn(new FakeDownload(rsrc.getResource().getResource().getFile(), true)) + .when(localizer).download(isA(Path.class), eq(tRsrc), + isA(UserGroupInformation.class)); + + // Localize and verify that the disk check was performed on the original + // provided path, not a URI-encoded version of the path. + assertEquals(0, localizer.runLocalization(nmAddr)); + verify(localizer).checkDir(new File(rsrcPath)); + } + @SuppressWarnings("unchecked") // mocked generics private ContainerLocalizer setupContainerLocalizerForTest() throws Exception {