NPE when listing an empty directory

[radai-rosenblatt]

using NfsServerV3 and my own VFS implementation, if VirtualFileSystem.list(Inode) returns an empty list (if the directory is empty) then i get an NPE.

the flow behind the NPE is as follows:

1. in NfsServerV3.NFSPROC3_READDIRPLUS_3() List dirList is assigned an empty list (a transform on the empty list that my VFS returned).
2. in NfsServerV3 line 761, the code creates a new dirlistplus3 instance: res.resok.reply = new dirlistplus3();
3. since dirList.size() is 0, the iteration loop starting at line 775 is never executed, which means the internal fields of the dirlistplus3 instance created in step 2 remain all null.
4. later, when attempting to serialize the result object, it encounters that dirlistplus3 object, and calls xdrEncode() on it.
5. in entryplus3 line 44 (inside xdrEncode) a NPE is generated since all fields are null ($this.fileid.xdrEncode(xdr) is the exact NPE cause)

i think in NfsServerV3 res.resok.reply.entries should only be initialized if there are results. setting the field to null in a breakpoint produced the expected result for me ("ls" call from shell completed)

[kofemann]

Well, technically, dirList.size() SHOULD never be zero, as any directory must contain '.' and '..'. if size is zero, then we have a bug in some other place.

[radai-rosenblatt]

even if dirList should always contain '.' and '..' (for the root as well?), what about a scenario where the cookie value is 2. that would cause those 2 initial entries to be skipped, and back to the same problem path (i think)

[kofemann]

Sure, NPE is not acceptable.

-kofemann /* caffeinated mutations of the core personality /

On Mon, Sep 8, 2014 at 11:02 AM, Radai Rosenblatt notifications@github.com wrote:

even if dirList should always contain '.' and '..' (for the root as well?), what about a scenario where the cookie value is 2. that would cause those 2 initial entries to be skipped, and back to the same problem path (i think)

— Reply to this email directly or view it on GitHub https://github.com/dCache/jpnfs/issues/5#issuecomment-54792350.

[radai-rosenblatt]

i've submitted a pull request for this - https://github.com/dCache/jpnfs/pull/8 the pull request covers only NfsServerV3.NFSPROC3_READDIR_3(), and covers another related NPE scenario (client provided count too small for any entries).

NFSPROC3_READDIRPLUS_3() probbaly has the same issue, which i'll fix in the same way if the current pull request is ok

[radai-rosenblatt]

AFAIK, the changes so far resolve the original issue. after having spent some time in the code I think there's one more edge case: count/limit checks are only done when writing actual directory entries to the result. in theory that means that it (wrongfully ?) returns NFS_OK even when the header size itself is larger than the specified count/limit.

[kofemann]

This depends on how much you want to cover. We take care about readdir reply size

int currcount = READDIR3RESOK_SIZE;

but not including RPC header. Which is good enough for most realistic workloads.

-kofemann /* caffeinated mutations of the core personality /

On Tue, Sep 9, 2014 at 5:19 PM, Radai Rosenblatt notifications@github.com wrote:

AFAIK, the changes so far resolve the original issue. after having spent some time in the code I think there's one more edge case: count/limit checks are only done when writing actual directory entries to the result. in theory that means that it (wrongfully ?) returns NFS_OK even when the header size itself is larger than the specified count/limit.

— Reply to this email directly or view it on GitHub https://github.com/dCache/jpnfs/issues/5#issuecomment-54985248.