RFR: JDK-8264110 (fs) possible UnsupportedOperationException in UnixUserDefinedFileAttributeView.read(...)

[Alan Bateman]

On Fri, 26 Mar 2021 19:03:57 GMT, Sebastian Stenzel <github.com+1204330+overheadhunter at openjdk.org> wrote:

>> src/java.base/unix/classes/sun/nio/fs/UnixUserDefinedFileAttributeView.java line 197:
>> 
>>> 195:                     unsafe.copyMemory(null, address, tmp, 0, n);
>>> 196:                     dst.put(tmp);
>>> 197:                 }
>> 
>> The changes look okay but I'm wondering what cases you have in mind so that we can add tests. We lack tests that use of these APIs with buffers that are created with JNI NewDirectByteBuffer.
>
> Tbh, I don't know any specific ByteBuffer implementation (other than DirectByteBuffer) that doesn't have an array, but in theory it is explicitly allowed. And since this `read(String name, ByteBuffer dst)` is public API, it should be prepared to deal with any allowed params.
> 
> But after all I didn't encounter any exception, I just copied the implementation from `write()` as I deemed it reasonable.
> 
> Alternatively, if ByteBuffers were sealed classes and only allowed HeapByteBuffers (known to have an array) or DirectByteBuffers (allowing direct access to the address, thus irrelevant in this part of the code) we could skip the distinction entirely. But I guess that isn't something we can decide in the scope of this PR.
> 
> Regarding test cases: Other than creating a ByteBuffer mock that returns false upon `hasArray()`, I don't know how to verify this branch. That said, so far I failed to understand what framework or pattern to use to write JDK tests. The UserDefinedAttributeView test seems to be a mere `main(String[] args)` class that must not complete exceptionally.

Buffers are closed abstraction and cannot be implemented outside of the java.nio package. When sealed classes are promoted from preview to be a permanent feature then maybe we should look at make them sealed. For now, I think the only scenario where the reported issue can arise is when someone injects their own class into java.nio, maybe a mock as you suggest. I'm not opposed to handling such cases but I suspect it would require work in several other areas of the JDK that currently have to distinguish between heap and direct buffers.

As regards tests, then yes, the basic unit test for UserDefinedAttributeView is an old style jtreg test. It pre-dates support for TestNG and other frameworks.

-------------

PR: https://git.openjdk.java.net/jdk/pull/3217