On Wed, 6 Jan 2021 16:32:17 GMT, Maurizio Cimadamore <[email protected]> 
wrote:

>> This patch tweaks `MemorySegment::mapFile` so that it will throw 
>> `IllegalArgumentException` whenever the path to be mapped is associated with 
>> a custom file system provider.
>> 
>> The check in the implementation is heavily borrowed by what 
>> `UnixDomainSocketAddress::of(Path)` does (thanks Alan for the tip!). Not 
>> only we have to check if file system is the default one, but also if the 
>> default FS belongs to java.base (since that can be overridden).
>> 
>> The test simply check that paths coming from the (internal) JRT file system 
>> are not supported by the factory.
>
> Maurizio Cimadamore has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Simplify test not to depend on JDK internals

Looks fine to me.

Another option would have been to just check the class with instanceof. If the 
FileChannel returned by the path's filesystem has wrong type, it won't work. If 
we have some custom filesystem that just returns the original FileChannelImpl 
but does some additional checks before or after (e.g to hide some files from 
user), this would still work.

But I think it's better to be clear here and deny anything which is not default 
operating system filesystem, until we have a better solution.

In Lucene I added some hack that removes our test-only filesystem wrappers from 
the path. I will later go the route to mark those path instances with some 
interface Unwrappable that provides a method unwrap().

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

Marked as reviewed by uschindler (Author).

PR: https://git.openjdk.java.net/jdk16/pull/90

Reply via email to