Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ enum LinkType {
* is changed later it is then ignored (a dir with null entries)
*/
public static class INodeLink<T> extends INode<T> {
final URI[] targetDirLinkList;
final String[] targetDirLinkList;
private T targetFileSystem; // file system object created from the link.
// Function to initialize file system. Only applicable for simple links
private Function<URI, T> fileSystemInitMethod;
Expand All @@ -283,7 +283,7 @@ public static class INodeLink<T> extends INode<T> {
* Construct a mergeLink or nfly.
*/
INodeLink(final String pathToNode, final UserGroupInformation aUgi,
final T targetMergeFs, final URI[] aTargetDirLinkList) {
final T targetMergeFs, final String[] aTargetDirLinkList) {
super(pathToNode, aUgi);
targetFileSystem = targetMergeFs;
targetDirLinkList = aTargetDirLinkList;
Expand All @@ -294,11 +294,11 @@ public static class INodeLink<T> extends INode<T> {
*/
INodeLink(final String pathToNode, final UserGroupInformation aUgi,
Function<URI, T> createFileSystemMethod,
final URI aTargetDirLink) {
final String aTargetDirLink) throws URISyntaxException {
super(pathToNode, aUgi);
targetFileSystem = null;
targetDirLinkList = new URI[1];
targetDirLinkList[0] = aTargetDirLink;
targetDirLinkList = new String[1];
targetDirLinkList[0] = new URI(aTargetDirLink).toString();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the purpose of creating a URI and converting back to string?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the previous implementation, we were creating the URI object from the target file system path. If the path is not a valid URI, the filesystem initialization should have failed. As we are not keeping the URI object anymore, this ensures we are not dealing with an invalid URI and new URI object validates the target filesystem path.

this.fileSystemInitMethod = createFileSystemMethod;
}

Expand Down Expand Up @@ -336,7 +336,8 @@ public T getTargetFileSystem() throws IOException {
if (targetFileSystem != null) {
return targetFileSystem;
}
targetFileSystem = fileSystemInitMethod.apply(targetDirLinkList[0]);
targetFileSystem =
fileSystemInitMethod.apply(URI.create(targetDirLinkList[0]));
if (targetFileSystem == null) {
throw new IOException(
"Could not initialize target File System for URI : " +
Expand Down Expand Up @@ -404,7 +405,7 @@ private void createLink(final String src, final String target,
switch (linkType) {
case SINGLE:
newLink = new INodeLink<T>(fullPath, aUgi,
initAndGetTargetFs(), new URI(target));
initAndGetTargetFs(), target);
break;
case SINGLE_FALLBACK:
case MERGE_SLASH:
Expand All @@ -413,10 +414,10 @@ private void createLink(final String src, final String target,
throw new IllegalArgumentException("Unexpected linkType: " + linkType);
case MERGE:
case NFLY:
final URI[] targetUris = StringUtils.stringToURI(
StringUtils.getStrings(target));
final String[] targetUris = StringUtils.getStrings(target);
newLink = new INodeLink<T>(fullPath, aUgi,
getTargetFileSystem(settings, targetUris), targetUris);
getTargetFileSystem(settings, StringUtils.stringToURI(targetUris)),
targetUris);
break;
default:
throw new IllegalArgumentException(linkType + ": Infeasible linkType");
Expand Down Expand Up @@ -633,8 +634,7 @@ protected InodeTree(final Configuration config, final String viewName,
if (isMergeSlashConfigured) {
Preconditions.checkNotNull(mergeSlashTarget);
root = new INodeLink<T>(mountTableName, ugi,
initAndGetTargetFs(),
new URI(mergeSlashTarget));
initAndGetTargetFs(), mergeSlashTarget);
mountPoints.add(new MountPoint<T>("/", (INodeLink<T>) root));
rootFallbackLink = null;
} else {
Expand All @@ -652,7 +652,7 @@ protected InodeTree(final Configuration config, final String viewName,
+ "not allowed.");
}
fallbackLink = new INodeLink<T>(mountTableName, ugi,
initAndGetTargetFs(), new URI(le.getTarget()));
initAndGetTargetFs(), le.getTarget());
continue;
case REGEX:
addRegexMountEntry(le);
Expand All @@ -677,7 +677,7 @@ protected InodeTree(final Configuration config, final String viewName,
.info("Empty mount table detected for {} and considering itself "
+ "as a linkFallback.", theUri);
rootFallbackLink = new INodeLink<T>(mountTableName, ugi,
initAndGetTargetFs(), theUri);
initAndGetTargetFs(), theUri.toString());
getRootDir().addFallbackLink(rootFallbackLink);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -216,19 +216,27 @@ public static class MountPoint {
/**
* Array of target FileSystem URIs.
*/
private final URI[] targetFileSystemURIs;
private final String[] targetFileSystemPaths;

MountPoint(Path srcPath, URI[] targetFs) {
MountPoint(Path srcPath, String[] targetFs) {
mountedOnPath = srcPath;
targetFileSystemURIs = targetFs;
targetFileSystemPaths = targetFs;
}

public Path getMountedOnPath() {
return mountedOnPath;
}

public URI[] getTargetFileSystemURIs() {
return targetFileSystemURIs;
URI[] targetUris = new URI[targetFileSystemPaths.length];
for (int i = 0; i < targetFileSystemPaths.length; i++) {
targetUris[i] = URI.create(targetFileSystemPaths[i]);
}
return targetUris;
}

public String[] getTargetFileSystemPaths() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a public method, can we keep the old signature? We can transform from string to URI within the method itself.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added getTargetFileSystemURIs where we are returning URI[]. Also added method to return targetFileSystem Paths.

return targetFileSystemPaths;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,16 +185,18 @@ static AccessControlException readOnlyMountTable(final String operation,


static public class MountPoint {
private Path src; // the src of the mount
private URI[] targets; // target of the mount; Multiple targets imply mergeMount
MountPoint(Path srcPath, URI[] targetURIs) {
// the src of the mount
private Path src;
// Target of the mount; Multiple targets imply mergeMount
private String[] targets;
MountPoint(Path srcPath, String[] targetURIs) {
src = srcPath;
targets = targetURIs;
}
Path getSrc() {
return src;
}
URI[] getTargets() {
String[] getTargets() {
return targets;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1671,4 +1671,22 @@ public void testTargetFileSystemLazyInitializationForChecksumMethods()
// viewfs inner cache is disabled
assertEquals(cacheSize + 1, TestFileUtil.getCacheSize());
}

@Test
public void testInvalidMountPoints() throws Exception {
final String clusterName = "cluster" + new Random().nextInt();
Configuration config = new Configuration(conf);
config.set(ConfigUtil.getConfigViewFsPrefix(clusterName) + "." +
Constants.CONFIG_VIEWFS_LINK + "." + "/invalidPath",
"othermockfs:|mockauth/mockpath");

try {
FileSystem viewFs = FileSystem.get(
new URI("viewfs://" + clusterName + "/"), config);
fail("FileSystem should not initialize. Should fail with IOException");
} catch (IOException ex) {
assertTrue("Should get URISyntax Exception",
ex.getMessage().startsWith("URISyntax exception"));
}
}
}