RFR: 8331467: ImageReaderFactory can cause a ClassNotFoundException if the default FileSystemProvider is not the system-default provider [v7]

[liyazzi]

On Sun, 15 Dec 2024 09:46:07 GMT, Alan Bateman <alanb at openjdk.org> wrote:

> The bigger question is whether specification of FileSystems.getDefault should be revised to specify how it should work when the default file system provider is deployed as a module (which would cover the case where there the provider has been linked into the a run-time image). The reasons the tests work is because the provider module exports the package with the provider class but it should be possible to fully encapsulate it. I'll try to do some think on what the right thing to do there is.

For the specification of FileSystems.getDefault,In my opinion, I think there would be 2 things need to be added.

**A**: add instructions for paying attention to circular dependencies when loading the default file system provider. The changes implemented by this PR to ImageFactory have solved the circular dependency problem in the case where the default file system provider is deployed as a module.

**B**: specify what we should do to keep full encapsulation when loading the default file system provider(maybe not one,but a list).The current performance of FileSystems.getDefault is that:

1. If the default file system provider is on classpath,then it will be in the Unnamed-Module which exports the package that contains the default file system provider.And it is okay.
2. If it is on module path,which means the default file system provider is in a Named-Module(this includes the case of linked into run-time image),unless we export the package with the provider class,it would throw IllegalAccessException: java.base -> customfs module.

Because in current specification of FileSystems.getDefault it is going this:

> Each class is loaded, **using the system class loader**, and instantiated by invoking a one argument constructor whose formal parameter type is FileSystemProvider.

It simply uses the `ClassLoader.getSystemClassLoader()` then leading to the performance mentioned above.

For the **B** point, my opinion is that: not just using the system class loader,but we could load provider class like java SPI loads the Service-Provider.Specifically, the code is as follows(change the FileSystems$DefaultFileSystemHolder.getDefaultProvider):


        // returns default provider
        private static FileSystemProvider getDefaultProvider() {
            // start with the platform's default file system provider
            FileSystemProvider provider = DefaultFileSystemProvider.instance();

            // if the property java.nio.file.spi.DefaultFileSystemProvider is
            // set then its value is the name of the default provider (or a list)
            String prop = "java.nio.file.spi.DefaultFileSystemProvider";
            String propValue = System.getProperty(prop);
            if (propValue != null) {
                boolean moduleSpecified;
                Class<?> c;
                Constructor<?> ctor;
                // the format of cn should be like this: [<module-name>/]<fully-qualified-class-name>
                for (String cn: propValue.split(",")) {
                    try {
                        moduleSpecified = cn.contains("/");
                        if(!moduleSpecified) {
                            c = Class
                                    .forName(cn, true, ClassLoader.getSystemClassLoader());
                            ctor = c
                                    .getDeclaredConstructor(FileSystemProvider.class);
                        }else {
                            // get the module name
                            int spiltIndex = cn.indexOf('/');
                            String moduleName = cn.substring(0, spiltIndex);
                            // get the class name
                            cn = cn.substring(spiltIndex +1);
                            Module module = ModuleLayer.boot().findModule(moduleName).orElseThrow(()->new Error("module not found: " + moduleName));
                            c = Class.forName(module,cn);
                            ctor = c.getDeclaredConstructor(FileSystemProvider.class);
                            // get the access to the provider class without exports package which contains default provider class
                            ctor.setAccessible(true);
                        }

                        provider = (FileSystemProvider)ctor.newInstance(provider);

                        // must be "file"
                        if (!provider.getScheme().equals("file"))
                            throw new Error("Default provider must use scheme 'file'");

                    } catch (Exception x) {
                        throw new Error(x);
                    }
                }
            }
            return provider;
        }


In the above code, it changes 2 things:

1. Change the format of the value of the `java.nio.file.spi.DefaultFileSystemProvider` to `[<module-name>/]<fully-qualified-class-name> ...` e.g. `-Djava.nio.file.spi.DefaultFileSystemProvider=module1/foo.cutomfsprovider1,bar.cutomfsprovider2`
2. If `<module-name>` is not specified, then load provider as usual; else load provider from its module without provider exporting its package.

I test with these cases:

test resource: two modular jar

1. **customfs** module: customfs.jar (contains the custom provider class: `foo.CustomFileSystemProvider`; export nothing)
2. **main** module: main.jar (contains main class: `bar.main` to run)

- class-path case: `java -cp customfs.jar:main.jar -Djava.nio.file.spi.DefaultFileSystemProvider=foo.CustomFileSystemProvider bar.Main` success

- module-path case: `java -p customfs.jar:main.jar -Djava.nio.file.spi.DefaultFileSystemProvider=customfs/foo.CustomFileSystemProvider --add-modules customfs -m main/bar.Main` diff: **foo.CustomFileSystemProvider -> customfs/er=foo.CustomFileSystemProvider** and success

- linked-into image case(contains **customfs**, **main**, **java.base** modules): `image/bin/java -Djava.nio.file.spi.DefaultFileSystemProvider=customfs/foo.CustomFileSystemProvider -m main/bar.Main` 
**failed: java.lang.Error: module not found: customfs**. I found that even in image, only if add `--add-modules customfs` option,would jvm add the **customfs** module to the `ModuleLayer.boot()`.
So: `image/bin/java -Djava.nio.file.spi.DefaultFileSystemProvider=customfs/foo.CustomFileSystemProvider --add-modules customfs -m main/bar.Main`  success

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

PR Comment: https://git.openjdk.org/jdk/pull/22628#issuecomment-2550669475