diff --git a/hadoop-common-project/hadoop-common/CHANGES.txt b/hadoop-common-project/hadoop-common/CHANGES.txt
index 04e7744e941..7e238772247 100644
--- a/hadoop-common-project/hadoop-common/CHANGES.txt
+++ b/hadoop-common-project/hadoop-common/CHANGES.txt
@@ -1283,6 +1283,8 @@ Release 0.22.0 - Unreleased
HADOOP-7457. Remove out-of-date Chinese language documentation.
(Jakob Homan via eli)
+ HADOOP-7783. Add more symlink tests that cover intermediate links. (eli)
+
Release 0.21.1 - Unreleased
IMPROVEMENTS
diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/AbstractFileSystem.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/AbstractFileSystem.java
index f4632f30aed..79419507ba6 100644
--- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/AbstractFileSystem.java
+++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/AbstractFileSystem.java
@@ -385,7 +385,7 @@ public abstract class AbstractFileSystem {
checkPath(p);
String s = p.toUri().getPath();
if (!isValidName(s)) {
- throw new InvalidPathException("Path part " + s + " from URI" + p
+ throw new InvalidPathException("Path part " + s + " from URI " + p
+ " is not a valid filename.");
}
return s;
diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileContext.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileContext.java
index 8b20651f0d6..5a14f82a675 100644
--- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileContext.java
+++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileContext.java
@@ -1092,29 +1092,28 @@ public final class FileContext {
* Return a fully qualified version of the given symlink target if it
* has no scheme and authority. Partially and fully qualified paths
* are returned unmodified.
- * @param linkFS The AbstractFileSystem of link
- * @param link The path of the symlink
- * @param target The symlink's target
+ * @param pathFS The AbstractFileSystem of the path
+ * @param pathWithLink Path that contains the symlink
+ * @param target The symlink's absolute target
* @return Fully qualified version of the target.
*/
- private Path qualifySymlinkTarget(final AbstractFileSystem linkFS,
- Path link, Path target) {
- /* NB: makeQualified uses link's scheme/authority, if specified,
- * and the scheme/authority of linkFS, if not. If link does have
- * a scheme and authority they should match those of linkFS since
- * resolve updates the path and file system of a path that contains
- * links each time a link is encountered.
+ private Path qualifySymlinkTarget(final AbstractFileSystem pathFS,
+ Path pathWithLink, Path target) {
+ /* NB: makeQualified uses the target's scheme and authority, if
+ * specified, and the scheme and authority of pathFS, if not. If
+ * the path does have a scheme and authority we assert they match
+ * those of pathFS since resolve updates the file system of a path
+ * that contains links each time a link is encountered.
*/
- final String linkScheme = link.toUri().getScheme();
- final String linkAuth = link.toUri().getAuthority();
- if (linkScheme != null && linkAuth != null) {
- assert linkScheme.equals(linkFS.getUri().getScheme());
- assert linkAuth.equals(linkFS.getUri().getAuthority());
+ final String scheme = target.toUri().getScheme();
+ final String auth = target.toUri().getAuthority();
+ if (scheme != null && auth != null) {
+ assert scheme.equals(pathFS.getUri().getScheme());
+ assert auth.equals(pathFS.getUri().getAuthority());
}
- final boolean justPath = (target.toUri().getScheme() == null &&
- target.toUri().getAuthority() == null);
- return justPath ? target.makeQualified(linkFS.getUri(), link.getParent())
- : target;
+ return (scheme == null && auth == null)
+ ? target.makeQualified(pathFS.getUri(), pathWithLink.getParent())
+ : target;
}
/**
@@ -1148,16 +1147,19 @@ public final class FileContext {
}
/**
- * Returns the un-interpreted target of the given symbolic link.
- * Transparently resolves all links up to the final path component.
- * @param f
+ * Returns the target of the given symbolic link as it was specified
+ * when the link was created. Links in the path leading up to the
+ * final path component are resolved transparently.
+ *
+ * @param f the path to return the target of
* @return The un-interpreted target of the symbolic link.
*
* @throws AccessControlException If access is denied
* @throws FileNotFoundException If path f
does not exist
* @throws UnsupportedFileSystemException If file system for f
is
* not supported
- * @throws IOException If an I/O error occurred
+ * @throws IOException If the given path does not refer to a symlink
+ * or an I/O error occurred
*/
public Path getLinkTarget(final Path f) throws AccessControlException,
FileNotFoundException, UnsupportedFileSystemException, IOException {
@@ -1277,7 +1279,7 @@ public final class FileContext {
* getFsStatus, getFileStatus, exists, and listStatus.
*
* Symlink targets are stored as given to createSymlink, assuming the
- * underlying file system is capable of storign a fully qualified URI.
+ * underlying file system is capable of storing a fully qualified URI.
* Dangling symlinks are permitted. FileContext supports four types of
* symlink targets, and resolves them as follows
*
diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/Path.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/Path.java index 8ac3b6ae532..25c0904ed80 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/Path.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/Path.java @@ -66,13 +66,14 @@ public class Path implements Comparable { // Add a slash to parent's path so resolution is compatible with URI's URI parentUri = parent.uri; String parentPath = parentUri.getPath(); - if (!(parentPath.equals("/") || parentPath.equals(""))) + if (!(parentPath.equals("/") || parentPath.equals(""))) { try { parentUri = new URI(parentUri.getScheme(), parentUri.getAuthority(), parentUri.getPath()+"/", null, parentUri.getFragment()); } catch (URISyntaxException e) { throw new IllegalArgumentException(e); } + } URI resolved = parentUri.resolve(child.uri); initialize(resolved.getScheme(), resolved.getAuthority(), normalizePath(resolved.getPath()), resolved.getFragment()); @@ -211,7 +212,8 @@ public class Path implements Comparable { * There is some ambiguity here. An absolute path is a slash * relative name without a scheme or an authority. * So either this method was incorrectly named or its - * implementation is incorrect. + * implementation is incorrect. This method returns true + * even if there is a scheme and authority. */ public boolean isAbsolute() { return isUriPathAbsolute(); @@ -305,19 +307,16 @@ public class Path implements Comparable { return depth; } - /** * Returns a qualified path object. * * Deprecated - use {@link #makeQualified(URI, Path)} */ - @Deprecated public Path makeQualified(FileSystem fs) { return makeQualified(fs.getUri(), fs.getWorkingDirectory()); } - /** Returns a qualified path object. */ @InterfaceAudience.LimitedPrivate({"HDFS", "MapReduce"}) public Path makeQualified(URI defaultUri, Path workingDir ) { diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/FileContextSymlinkBaseTest.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/FileContextSymlinkBaseTest.java index fd3283d4687..da9b895b57e 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/FileContextSymlinkBaseTest.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/FileContextSymlinkBaseTest.java @@ -28,8 +28,9 @@ import org.apache.hadoop.fs.CreateFlag; import org.apache.hadoop.fs.Path; import org.apache.hadoop.fs.FSDataOutputStream; import static org.apache.hadoop.fs.FileContextTestHelper.*; -import static org.junit.Assert.*; +import static org.junit.Assert.*; +import static org.junit.Assume.assumeTrue; import org.junit.Test; import org.junit.Before; import org.junit.After; @@ -238,6 +239,31 @@ public abstract class FileContextSymlinkBaseTest { assertFalse(isDir(fc, linkToFile)); assertEquals(file.toUri().getPath(), fc.getLinkTarget(linkToFile).toString()); + // The local file system does not fully resolve the link + // when obtaining the file status + if (!"file".equals(getScheme())) { + assertEquals(fc.getFileStatus(file), fc.getFileStatus(linkToFile)); + assertEquals(fc.makeQualified(file), + fc.getFileStatus(linkToFile).getPath()); + assertEquals(fc.makeQualified(linkToFile), + fc.getFileLinkStatus(linkToFile).getPath()); + } + } + + @Test + /** Stat a relative link to a file */ + public void testStatRelLinkToFile() throws IOException { + assumeTrue(!"file".equals(getScheme())); + Path baseDir = new Path(testBaseDir1()); + Path file = new Path(testBaseDir1(), "file"); + Path linkToFile = new Path(testBaseDir1(), "linkToFile"); + createAndWriteFile(file); + fc.createSymlink(new Path("file"), linkToFile, false); + assertEquals(fc.getFileStatus(file), fc.getFileStatus(linkToFile)); + assertEquals(fc.makeQualified(file), + fc.getFileStatus(linkToFile).getPath()); + assertEquals(fc.makeQualified(linkToFile), + fc.getFileLinkStatus(linkToFile).getPath()); } @Test @@ -474,18 +500,15 @@ public abstract class FileContextSymlinkBaseTest { * creating using a partially qualified path is file system specific. */ public void testCreateLinkUsingPartQualPath1() throws IOException { + // Partially qualified paths are covered for local file systems + // in the previous test. + assumeTrue(!"file".equals(getScheme())); Path schemeAuth = new Path(testURI().toString()); Path fileWoHost = new Path(getScheme()+"://"+testBaseDir1()+"/file"); Path link = new Path(testBaseDir1()+"/linkToFile"); Path linkQual = new Path(schemeAuth, testBaseDir1()+"/linkToFile"); - - // Partially qualified paths are covered for local file systems - // in the previous test. - if ("file".equals(getScheme())) { - return; - } FileContext localFc = FileContext.getLocalFSFileContext(); - + fc.createSymlink(fileWoHost, link, false); // Partially qualified path is stored assertEquals(fileWoHost, fc.getLinkTarget(linkQual)); @@ -748,7 +771,7 @@ public abstract class FileContextSymlinkBaseTest { } @Test - /** Test create symlink to ../foo */ + /** Test create symlink to ../file */ public void testCreateLinkToDotDotPrefix() throws IOException { Path file = new Path(testBaseDir1(), "file"); Path dir = new Path(testBaseDir1(), "test"); @@ -1205,24 +1228,30 @@ public abstract class FileContextSymlinkBaseTest { } @Test - /** Operate on a file using a path with an intermediate symlink */ - public void testAccessFileViaSymlink() throws IOException { + /** + * Create, write, read, append, rename, get the block locations, + * checksums, and delete a file using a path with a symlink as an + * intermediate path component where the link target was specified + * using an absolute path. Rename is covered in more depth below. + */ + public void testAccessFileViaInterSymlinkAbsTarget() throws IOException { Path baseDir = new Path(testBaseDir1()); + Path file = new Path(testBaseDir1(), "file"); Path fileNew = new Path(baseDir, "fileNew"); Path linkToDir = new Path(testBaseDir2(), "linkToDir"); Path fileViaLink = new Path(linkToDir, "file"); Path fileNewViaLink = new Path(linkToDir, "fileNew"); fc.createSymlink(baseDir, linkToDir, false); - // Create, write, read, append, rename, get block locations and - // checksums, and delete a file using a path that contains a - // symlink as an intermediate path component. Rename is covered - // in more depth below. createAndWriteFile(fileViaLink); assertTrue(exists(fc, fileViaLink)); assertTrue(isFile(fc, fileViaLink)); assertFalse(isDir(fc, fileViaLink)); assertFalse(fc.getFileLinkStatus(fileViaLink).isSymlink()); assertFalse(isDir(fc, fileViaLink)); + assertEquals(fc.getFileStatus(file), + fc.getFileLinkStatus(file)); + assertEquals(fc.getFileStatus(fileViaLink), + fc.getFileLinkStatus(fileViaLink)); readFile(fileViaLink); appendToFile(fileViaLink); fc.rename(fileViaLink, fileNewViaLink); @@ -1237,6 +1266,58 @@ public abstract class FileContextSymlinkBaseTest { assertFalse(exists(fc, fileNewViaLink)); } + @Test + /** + * Operate on a file using a path with an intermediate symlink where + * the link target was specified as a fully qualified path. + */ + public void testAccessFileViaInterSymlinkQualTarget() throws IOException { + Path baseDir = new Path(testBaseDir1()); + Path file = new Path(testBaseDir1(), "file"); + Path fileNew = new Path(baseDir, "fileNew"); + Path linkToDir = new Path(testBaseDir2(), "linkToDir"); + Path fileViaLink = new Path(linkToDir, "file"); + Path fileNewViaLink = new Path(linkToDir, "fileNew"); + fc.createSymlink(fc.makeQualified(baseDir), linkToDir, false); + createAndWriteFile(fileViaLink); + assertEquals(fc.getFileStatus(file), + fc.getFileLinkStatus(file)); + assertEquals(fc.getFileStatus(fileViaLink), + fc.getFileLinkStatus(fileViaLink)); + readFile(fileViaLink); + } + + @Test + /** + * Operate on a file using a path with an intermediate symlink where + * the link target was specified as a relative path. + */ + public void testAccessFileViaInterSymlinkRelTarget() throws IOException { + assumeTrue(!"file".equals(getScheme())); + Path baseDir = new Path(testBaseDir1()); + Path dir = new Path(testBaseDir1(), "dir"); + Path file = new Path(dir, "file"); + Path linkToDir = new Path(testBaseDir1(), "linkToDir"); + Path fileViaLink = new Path(linkToDir, "file"); + + fc.mkdir(dir, FileContext.DEFAULT_PERM, false); + fc.createSymlink(new Path("dir"), linkToDir, false); + createAndWriteFile(fileViaLink); + // Note that getFileStatus returns fully qualified paths even + // when called on an absolute path. + assertEquals(fc.makeQualified(file), + fc.getFileStatus(file).getPath()); + // In each case getFileLinkStatus returns the same FileStatus + // as getFileStatus since we're not calling it on a link and + // FileStatus objects are compared by Path. + assertEquals(fc.getFileStatus(file), + fc.getFileLinkStatus(file)); + assertEquals(fc.getFileStatus(fileViaLink), + fc.getFileLinkStatus(fileViaLink)); + assertEquals(fc.getFileStatus(fileViaLink), + fc.getFileLinkStatus(file)); + } + @Test /** Test create, list, and delete a directory through a symlink */ public void testAccessDirViaSymlink() throws IOException { @@ -1272,4 +1353,4 @@ public abstract class FileContextSymlinkBaseTest { assertEquals(2, fc.getFileStatus(file).getModificationTime()); } } -} \ No newline at end of file +} diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestPath.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestPath.java index d36a39edb8f..b1d92686c26 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestPath.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestPath.java @@ -95,6 +95,7 @@ public class TestPath extends TestCase { assertEquals(new Path("/foo"), new Path("/foo/bar").getParent()); assertEquals(new Path("foo"), new Path("foo/bar").getParent()); assertEquals(new Path("/"), new Path("/foo").getParent()); + assertEquals(null, new Path("/").getParent()); if (Path.WINDOWS) { assertEquals(new Path("c:/"), new Path("c:/foo").getParent()); } @@ -159,6 +160,13 @@ public class TestPath extends TestCase { assertEquals(new Path("foo/bar/baz","../../..").toString(), ""); assertEquals(new Path("foo/bar/baz","../../../../..").toString(), "../.."); } + + /** Test Path objects created from other Path objects */ + public void testChildParentResolution() throws URISyntaxException, IOException { + Path parent = new Path("foo1://bar1/baz1"); + Path child = new Path("foo2://bar2/baz2"); + assertEquals(child, new Path(parent, child)); + } public void testScheme() throws java.io.IOException { assertEquals("foo:/bar", new Path("foo:/","/bar").toString());