h5228_20130919_test.patch: a unit test for showing the bug. It will result the following exception.

Running org.apache.hadoop.hdfs.TestDistributedFileSystem
Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 2.991 sec <<< FAILURE! - in org.apache.hadoop.hdfs.TestDistributedFileSystem
testlistFiles(org.apache.hadoop.hdfs.TestDistributedFileSystem)  Time elapsed: 2.839 sec  <<< ERROR!
java.lang.NullPointerException: null
	at org.apache.hadoop.fs.Path.<init>(Path.java:105)
	at org.apache.hadoop.fs.Path.makeQualified(Path.java:433)
	at org.apache.hadoop.hdfs.protocol.HdfsLocatedFileStatus.makeQualifiedLocated(HdfsLocatedFileStatus.java:77)
	at org.apache.hadoop.hdfs.DistributedFileSystem$15.hasNext(DistributedFileSystem.java:739)
	at org.apache.hadoop.fs.FileSystem$5.hasNext(FileSystem.java:1729)
	at org.apache.hadoop.hdfs.TestDistributedFileSystem.testlistFiles(TestDistributedFileSystem.java:855)

h5228_20130919.patch: makeQualified if the path is a relative path.

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12603973/h5228_20130919.patch
against trunk revision .

+1 @author. The patch does not contain any @author tags.

+1 tests included. The patch appears to include 1 new or modified test files.

+1 javac. The applied patch does not increase the total number of javac compiler warnings.

+1 javadoc. The javadoc tool did not generate any warning messages.

+1 eclipse:eclipse. The patch built with eclipse:eclipse.

+1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

+1 release audit. The applied patch does not increase the total number of release audit warnings.

-1 core tests. The patch failed these unit tests in hadoop-hdfs-project/hadoop-hdfs:

org.apache.hadoop.hdfs.TestDFSShell

+1 contrib tests. The patch passed contrib unit tests.

Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/5001//testReport/
Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/5001//console

This message is automatically generated.

The test failure does not seem related. It also failed in some previous build (e.g. build #4999) and my local machine without the patch.

This bug was introduced by HADOOP-9418. Prior to that patch, this code path relied on DistributedFileSystem#getPathName to make the path absolute before returning it.

      { // initializer
        src = getPathName(p);

After that patch, getPathName is no longer guaranteed to return an absolute path. The code path here is slightly different now, but still does not guarantee an absolute path.

      { // initializer
        // Fully resolve symlinks in path first to avoid additional resolution
        // round-trips as we fetch more batches of listings
        src = getPathName(resolvePath(p));

Considering that getPathName is a widely used internal function, I'm wondering if there could be other bugs lurking at the other call sites if they also expected a guaranteed absolute path. Does HADOOP-9418 need to be reverted? I'll comment on that one so that all of the right watchers are notified.

I'm also wondering if this should be marked a blocker for the current 2.1.1-beta RC, considering the impact on existing client code. I suspect the bug is present in 2.1.0-beta, but it would not have been present in 2.0.x.

Hey guys, thanks for finding this and posting a patch. I agree it's very important to fix for 2.1.1, and almost certainly present in 2.1.0.

Couple patch notes:

• please remove the white-space only change in HdfsLocatedFileStatus
• please remove new @Deprecated annotations in DistributedFileSystem, let's keep this fix small
• test name should be camel-cased as testListFiles
• should use #fixRelativePart() rather than in-lining the qualification.

Basically, besides the new test, I think all we need is (matching the style of the rest of DFS):

Path absF = fixRelativePart(p);

and then renaming p to absF below.

I don't think this by itself is cause to revert HADOOP-9418. Basically all DFS methods are supposed to pass Paths through fixRelativePart and then getPathName, it just looks like we missed this one. It might be worth auditing DFS to make sure we didn't miss anything else.

Thanks, Andrew. Your explanation makes sense, and I confirmed that everything else in DistributedFileSystem uses a combination of fixRelativePart and getPathName. This is the only call site that has the problem. I agree that HADOOP-9418 doesn't need to be reverted.

> I don't think this by itself is cause to revert HADOOP-9418. Basically all DFS methods are supposed to pass Paths through fixRelativePart and then getPathName, it just looks like we missed this one. It might be worth auditing DFS to make sure we didn't miss anything else.

Hi Andrew,

I am sorry to hear that your HADOOP-9418 patch causes a serious bug in DistributFileSystem, which is a very important client API. How exactly to audit DFS do you suggest so that it could avoid further bugs? It does seem that HADOOP-9418 has not been well tested. Do you agree? I do think reverting HADOOP-9418 may make more sense.

We have extensive symlink tests checked in as part of HADOOP-8040, so it's unfair to say that it's not well tested. We've also been testing and fixing things upstream for the last few months; I think some number of bugs like this are unavoidable as symlinks was a pretty big change that required touching basically all the client API code.

As to fixing this particular issue, it'd be great to get it done in a timely fashion for the 2.1.1 release. I'd be happy to rework Nicholas' v1 patch myself if that would help. Chris, thanks for going through DFS to check the other call sites, much appreciated.

I'm attaching a new patch consisting of Nicholas's test and Andrew's suggested fix.

+1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12604377/HDFS-5228.2.patch
against trunk revision .

+1 @author. The patch does not contain any @author tags.

+1 tests included. The patch appears to include 1 new or modified test files.

+1 javac. The applied patch does not increase the total number of javac compiler warnings.

+1 javadoc. The javadoc tool did not generate any warning messages.

+1 eclipse:eclipse. The patch built with eclipse:eclipse.

+1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

+1 release audit. The applied patch does not increase the total number of release audit warnings.

+1 core tests. The patch passed unit tests in hadoop-hdfs-project/hadoop-hdfs.

+1 contrib tests. The patch passed contrib unit tests.

Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/5017//testReport/
Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/5017//console

This message is automatically generated.

> We have extensive symlink tests checked in as part of HADOOP-8040, ...

Which part of HADOOP-8040? Where are the new tests?

Chris, thanks for posting a patch. I do think that the best solution is to revert HADOOP-9418, which seemed not well tested. HADOOP-9418 first broke HADOOP-9761 and then this. As Andrew said, "... I think some number of bugs like this are unavoidable as symlinks was a pretty big change that required touching basically all the client API code. ..." We should not let HADOOP-9418 destabilize 2.1.1.

Andrew, you have not answered my following question. I hope that you are not avoiding it.

How exactly to audit DFS do you suggest so that it could avoid further bugs?

Chris, I'm +1 for your v2 patch, thanks for the help.

Hey Nicholas:

Which part of HADOOP-8040? Where are the new tests?

HADOOP-9370 and HADOOP-9355 extended the FileContext symlink tests to FileSystem. These are both linked on HADOOP-8040. You can also see this if you look for tests in the HADOOP-9418 patch.

Andrew, you have not answered my following question. I hope that you are not avoiding it.

I'm going to quote myself to answer the question:

Basically all DFS methods are supposed to pass Paths through fixRelativePart and then getPathName, it just looks like we missed this one. It might be worth auditing DFS to make sure we didn't miss anything else.

Chris already did this audit, perhaps you can ask him if you need additional in-person clarification.

Okay, let's commit the patch for the moment. We could revert the recent Symlink work later if we found more problems. For the record, I did hear some performance issues but not yet sure about the cause of them.

SUCCESS: Integrated in Hadoop-trunk-Commit #4461 (See https://builds.apache.org/job/Hadoop-trunk-Commit/4461/)
HDFS-5228. The RemoteIterator returned by DistributedFileSystem.listFiles may throw NullPointerException. Contributed by szetszwo and cnauroth (szetszwo: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1525828)

• /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
• /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DistributedFileSystem.java
• /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDistributedFileSystem.java

I have committed this. Thanks, Chris!

SUCCESS: Integrated in Hadoop-Yarn-trunk #342 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/342/)
HDFS-5228. The RemoteIterator returned by DistributedFileSystem.listFiles may throw NullPointerException. Contributed by szetszwo and cnauroth (szetszwo: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1525828)

• /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
• /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DistributedFileSystem.java
• /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDistributedFileSystem.java

FAILURE: Integrated in Hadoop-Mapreduce-trunk #1558 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1558/)
HDFS-5228. The RemoteIterator returned by DistributedFileSystem.listFiles may throw NullPointerException. Contributed by szetszwo and cnauroth (szetszwo: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1525828)

• /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
• /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DistributedFileSystem.java
• /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDistributedFileSystem.java

SUCCESS: Integrated in Hadoop-Hdfs-trunk #1532 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1532/)
HDFS-5228. The RemoteIterator returned by DistributedFileSystem.listFiles may throw NullPointerException. Contributed by szetszwo and cnauroth (szetszwo: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1525828)

• /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
• /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DistributedFileSystem.java
• /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDistributedFileSystem.java

Closing tickets that are already part of a release.