From 1c762c01c4103714939c7c72f01c38dd7c3d6c54 Mon Sep 17 00:00:00 2001 From: Suresh Srinivas Date: Thu, 2 May 2013 21:40:36 +0000 Subject: [PATCH] HADOOP-9043. Disallow in winutils creating symlinks with forwards slashes. Contributed by Chris Nauroth. git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@1478577 13f79535-47bb-0310-9956-ffa450edef68 --- .../hadoop-common/CHANGES.txt | 3 + .../apache/hadoop/fs/local/RawLocalFs.java | 13 ++-- .../hadoop-common/src/main/winutils/symlink.c | 11 ++++ .../fs/TestLocalFSFileContextSymlink.java | 41 ++++++++++++- .../org/apache/hadoop/util/TestWinUtils.java | 60 +++++++++++-------- 5 files changed, 96 insertions(+), 32 deletions(-) diff --git a/hadoop-common-project/hadoop-common/CHANGES.txt b/hadoop-common-project/hadoop-common/CHANGES.txt index d30119c6b09..2e4ca20bc6b 100644 --- a/hadoop-common-project/hadoop-common/CHANGES.txt +++ b/hadoop-common-project/hadoop-common/CHANGES.txt @@ -539,6 +539,9 @@ Trunk (Unreleased) HADOOP-9532. HADOOP_CLIENT_OPTS is appended twice by Windows cmd scripts. (Chris Nauroth via suresh) + + HADOOP-9043. Disallow in winutils creating symlinks with forwards slashes. + (Chris Nauroth via suresh) Release 2.0.5-beta - UNRELEASED diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/local/RawLocalFs.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/local/RawLocalFs.java index f28d4f09ca0..f4ce75e1b29 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/local/RawLocalFs.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/local/RawLocalFs.java @@ -18,6 +18,7 @@ package org.apache.hadoop.fs.local; import java.io.IOException; +import java.io.File; import java.io.FileNotFoundException; import java.net.URI; import java.net.URISyntaxException; @@ -90,8 +91,8 @@ public class RawLocalFs extends DelegateToFileSystem { // NB: Use createSymbolicLink in java.nio.file.Path once available try { Shell.execCommand(Shell.getSymlinkCommand( - getPathWithoutSchemeAndAuthority(target), - getPathWithoutSchemeAndAuthority(link))); + getPathWithoutSchemeAndAuthority(target).getPath(), + getPathWithoutSchemeAndAuthority(link).getPath())); } catch (IOException x) { throw new IOException("Unable to create symlink: "+x.getMessage()); } @@ -175,12 +176,12 @@ public class RawLocalFs extends DelegateToFileSystem { throw new AssertionError(); } - private static String getPathWithoutSchemeAndAuthority(Path path) { - // This code depends on Path.toString() to remove the leading slash before - // the drive specification on Windows. + private static File getPathWithoutSchemeAndAuthority(Path path) { Path newPath = path.isUriPathAbsolute() ? new Path(null, null, path.toUri().getPath()) : path; - return newPath.toString(); + + // Path.toString() removes leading slash before drive spec on Windows. + return new File(newPath.toString()); } } diff --git a/hadoop-common-project/hadoop-common/src/main/winutils/symlink.c b/hadoop-common-project/hadoop-common/src/main/winutils/symlink.c index ff1779ce09f..ea372cc06dc 100644 --- a/hadoop-common-project/hadoop-common/src/main/winutils/symlink.c +++ b/hadoop-common-project/hadoop-common/src/main/winutils/symlink.c @@ -60,6 +60,17 @@ int Symlink(__in int argc, __in_ecount(argc) wchar_t *argv[]) goto SymlinkEnd; } + if (wcschr(longLinkName, L'/') != NULL || wcschr(longFileName, L'/') != NULL) + { + // Reject forward-slash separated paths as they result in unusable symlinks. + // + fwprintf(stderr, + L"Rejecting forward-slash separated path which would result in an " + L"unusable symlink: link = %s, target = %s\n", longLinkName, longFileName); + ret = FAILURE; + goto SymlinkEnd; + } + // Check if the the process's access token has the privilege to create // symbolic links. Without this step, the call to CreateSymbolicLink() from // users have the privilege to create symbolic links will still succeed. diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestLocalFSFileContextSymlink.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestLocalFSFileContextSymlink.java index 64d0525a188..714683b5cc7 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestLocalFSFileContextSymlink.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestLocalFSFileContextSymlink.java @@ -27,6 +27,7 @@ import org.apache.hadoop.fs.Path; import org.apache.hadoop.fs.FileUtil; import static org.apache.hadoop.fs.FileContextTestHelper.*; import static org.junit.Assert.*; +import static org.junit.Assume.assumeTrue; import org.junit.Test; import org.junit.Before; @@ -65,7 +66,44 @@ public class TestLocalFSFileContextSymlink extends FileContextSymlinkBaseTest { fc = FileContext.getLocalFSFileContext(); super.setUp(); } - + + @Override + public void testCreateDanglingLink() throws IOException { + // Dangling symlinks are not supported on Windows local file system. + assumeTrue(!Path.WINDOWS); + super.testCreateDanglingLink(); + } + + @Override + public void testCreateFileViaDanglingLinkParent() throws IOException { + assumeTrue(!Path.WINDOWS); + super.testCreateFileViaDanglingLinkParent(); + } + + @Override + public void testOpenResolvesLinks() throws IOException { + assumeTrue(!Path.WINDOWS); + super.testOpenResolvesLinks(); + } + + @Override + public void testRecursiveLinks() throws IOException { + assumeTrue(!Path.WINDOWS); + super.testRecursiveLinks(); + } + + @Override + public void testRenameDirToDanglingSymlink() throws IOException { + assumeTrue(!Path.WINDOWS); + super.testRenameDirToDanglingSymlink(); + } + + @Override + public void testStatDanglingLink() throws IOException { + assumeTrue(!Path.WINDOWS); + super.testStatDanglingLink(); + } + @Test /** lstat a non-existant file using a partially qualified path */ public void testDanglingLinkFilePartQual() throws IOException { @@ -87,6 +125,7 @@ public class TestLocalFSFileContextSymlink extends FileContextSymlinkBaseTest { @Test /** Stat and lstat a dangling link */ public void testDanglingLink() throws IOException { + assumeTrue(!Path.WINDOWS); Path fileAbs = new Path(testBaseDir1()+"/file"); Path fileQual = new Path(testURI().toString(), fileAbs); Path link = new Path(testBaseDir1()+"/linkToFile"); diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestWinUtils.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestWinUtils.java index f75fc35062a..3a0f8b00f2a 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestWinUtils.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestWinUtils.java @@ -19,6 +19,7 @@ package org.apache.hadoop.util; import static org.junit.Assert.*; +import static org.junit.Assume.assumeTrue; import java.io.File; import java.io.FileInputStream; @@ -44,6 +45,8 @@ public class TestWinUtils { @Before public void setUp() { + // Not supported on non-Windows platforms + assumeTrue(Shell.WINDOWS); TEST_DIR.mkdirs(); } @@ -70,11 +73,6 @@ public class TestWinUtils { @Test (timeout = 30000) public void testLs() throws IOException { - if (!Shell.WINDOWS) { - // Not supported on non-Windows platforms - return; - } - final String content = "6bytes"; final int contentSize = content.length(); File testFile = new File(TEST_DIR, "file1"); @@ -104,11 +102,6 @@ public class TestWinUtils { @Test (timeout = 30000) public void testGroups() throws IOException { - if (!Shell.WINDOWS) { - // Not supported on non-Windows platforms - return; - } - String currentUser = System.getProperty("user.name"); // Verify that groups command returns information about the current user @@ -229,11 +222,6 @@ public class TestWinUtils { @Test (timeout = 30000) public void testBasicChmod() throws IOException { - if (!Shell.WINDOWS) { - // Not supported on non-Windows platforms - return; - } - // - Create a file. // - Change mode to 377 so owner does not have read permission. // - Verify the owner truly does not have the permissions to read. @@ -285,11 +273,6 @@ public class TestWinUtils { @Test (timeout = 30000) public void testChmod() throws IOException { - if (!Shell.WINDOWS) { - // Not supported on non-Windows platforms - return; - } - testChmodInternal("7", "-------rwx"); testChmodInternal("70", "----rwx---"); testChmodInternal("u-x,g+r,o=g", "-rw-r--r--"); @@ -322,11 +305,6 @@ public class TestWinUtils { @Test (timeout = 30000) public void testChown() throws IOException { - if (!Shell.WINDOWS) { - // Not supported on non-Windows platforms - return; - } - File a = new File(TEST_DIR, "a"); assertTrue(a.createNewFile()); String username = System.getProperty("user.name"); @@ -349,4 +327,36 @@ public class TestWinUtils { assertTrue(a.delete()); assertFalse(a.exists()); } + + @Test (timeout = 30000) + public void testSymlinkRejectsForwardSlashesInLink() throws IOException { + File newFile = new File(TEST_DIR, "file"); + assertTrue(newFile.createNewFile()); + String target = newFile.getPath(); + String link = new File(TEST_DIR, "link").getPath().replaceAll("\\\\", "/"); + try { + Shell.execCommand(Shell.WINUTILS, "symlink", link, target); + fail(String.format("did not receive expected failure creating symlink " + + "with forward slashes in link: link = %s, target = %s", link, target)); + } catch (IOException e) { + LOG.info( + "Expected: Failed to create symlink with forward slashes in target"); + } + } + + @Test (timeout = 30000) + public void testSymlinkRejectsForwardSlashesInTarget() throws IOException { + File newFile = new File(TEST_DIR, "file"); + assertTrue(newFile.createNewFile()); + String target = newFile.getPath().replaceAll("\\\\", "/"); + String link = new File(TEST_DIR, "link").getPath(); + try { + Shell.execCommand(Shell.WINUTILS, "symlink", link, target); + fail(String.format("did not receive expected failure creating symlink " + + "with forward slashes in target: link = %s, target = %s", link, target)); + } catch (IOException e) { + LOG.info( + "Expected: Failed to create symlink with forward slashes in target"); + } + } }