JDK-8068749 : Restrict javax.imageio.spi.ServiceRegistry to ImageIO types
  • Type: Bug
  • Component: client-libs
  • Sub-Component: javax.imageio
  • Affected Version: 9
  • Priority: P2
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2015-01-09
  • Updated: 2017-05-17
  • Resolved: 2015-08-13
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 9
9 b80Fixed
Sub Tasks
JDK-8175549 :  
Release Note: Restrict javax.imageio.spi.ServiceRegistry to ImageIO types - Closed
Description
The lookupProviders methods in javax.imageio.spi.ServiceRegistry are wrappers around ServiceLoader.load. This is problematic for modules because the ServiceLoader.load method look at their caller to check if the caller is in a module and if so, that the caller's module has the right "uses" in its module declaration. For now, then calling lookupProviders will cause ServiceLoader to check that ServiceRegistry's module (java.desktop) declares the "uses" and this will probably fail because the java.desktop module is unlikely to have it.

One approach to addressing this is to change these methods to be @CallerSensitive, pick-up the caller, and create a ServiceLoader in a manner that makes it look like the caller of ServiceRegistry.lookupProviders is the caller. 

The alternative is to do nothing and just deprecate these methods. This option may not be too bad if we can establish that these methods are rarely, if ever, used outside of the JDK.
Comments
Text for release notes: Java SE has been changed so that javax.imageio.spi.ServiceRegistry no longer allows service providers of other than Image I/O service types to be loaded. This is a behavioral change. Existing application binaries that attempt to use ServiceRegistry for non-Image-I/O services will cause IllegalArgumentException to be thrown at runtime. Such applications should be migrated to use java.util.ServiceLoader instead of javax.imageio.spi.ServiceRegistry.
03-08-2015
Review posted: http://mail.openjdk.java.net/pipermail/2d-dev/2015-July/005589.html Webrev: http://cr.openjdk.java.net/~smarks/reviews/8068749/webrev.0/
29-07-2015
I've removed "uses javax.imageio.spi.ImageReaderWriterSpi" from the module-info.java. I suspect my mistake when we initially added the uses for this API. Throwing IAE for non image I/O service types should be good.
30-06-2015
As noted in Alan's comment 2015-06-23, ServiceRegistry should explicitly check for specific ImageIO types and throw an exception if an attempt is made to use SR to load a service of another type. From Phil, the list of ImageIO types is as follows (all in javax.imageio.spi): - ImageInputStreamSpi - ImageOutputStreamSpi - ImageReaderSpi - ImageTranscoderSpi - ImageWriterSpi (Note, the java.desktop module-info.java has a "uses" declaration for ImageReaderWriterSpi. This might not be necessary and if not, it should be removed. It's an abstract superclass of ImageReaderSpi and ImageWriterSpi and there is no expectation of an actual service implementation of it that isn't one of its subtypes.) IllegalArgumentException should be thrown if code calls ServiceRegistry with a type other than one of the explicitly supported set. This is clearly a programming error, thus IAE is preferable to SCE, which indicates some kind of configuration error. The ServiceRegistry specification should be changed to indicate that it supports only these specific service types. I don't think "deprecate" is appropriate here, as this is an actual behavior change, not a warning of some future change. A recommendation to use ServiceLoader for general service loading should be added. One can imagine the addition of "escape-hatch" interfaces (such as system properties) that would allow ServiceRegistry to load service types outside the defined set. This mechanism would have to add the proper "uses" relationships at runtime, if possible. However, we believe the usage of ServiceRegistry for non-ImageIO service types to be infrequent enough that we don't see the need to provide this mechanism at this time.
30-06-2015
The explicit check that the service type is one of the image I/O service types is so that the API is not mis-used to attempt to load implementations of other service types that the java.desktop module uses. My comment about the reversing of the checks in the ServiceLoader code was just to explain the exception that Mandy pasted into a previous comment in the bug.
23-06-2015
I've reread the comments here and I still don't understand some things. Let's set aside for the moment the exact exception or error that ServiceRegistry throws if one of its load() or related methods is used on a non-ImageIO spi. Mandy, in the above comment, you said, SCE fails to access the service type it looks up. SCE meaning ServiceConfigurationError? Did you instead mean ServiceRegistry or ServiceLoader? Alan explained that -- I think -- there is an access check and a "uses" check in ServiceLoader. The ordering of these checks was reversed. Why is this significant if ServiceConfigurationError is still the result? Both of you have proposed that ServiceRegistry explicitly check for specific ImageIO interfaces instead of relying on the underlying call to ServiceLoader to throw SCE. This might be a fine thing to do, but what I don't understand is how the change in ordering of the access vs use checks has any bearing on this.
23-06-2015
The load methods will throw SCE if the service type is not accessible to the caller or it doesn't declare the "use". The implementation changed during refactoring so that the order of the checks is reversed compared to previously in jake. I think it's best for ServiceRegistry to have a list of the image I/O service types that it supports and only call ServiceLoader when invoked with one of those types. It can fail (IAE or whatever is the norm for this area) when the service type is something else (meaning a mis-use of the API).
06-06-2015
Switching to strict module, SCE fails to access the service type it looks up. This actually leads me to think if it's helpful to catch issue earlier if the service type is a non-imageio API and throw IAE rather than SCE indicating IAE. java.desktop uses other non image I/O SPI e.g. uses javax.print.PrintServiceLookup. What does ServiceRegistry.lookupProviders(javax.print.PrintServiceLookup.class) currently return? java.util.ServiceConfigurationError: javasoft.sqe.tests.api.javax.imageio.spi.ServiceRegistry.MyServiceProvider: not accessible to module java.desktop at java.util.ServiceLoader.fail(java.base@9.0/ServiceLoader.java:389) at java.util.ServiceLoader.checkModule(java.base@9.0/ServiceLoader.java:365) at java.util.ServiceLoader.<init>(java.base@9.0/ServiceLoader.java:318) at java.util.ServiceLoader.<init>(java.base@9.0/ServiceLoader.java:350) at java.util.ServiceLoader.load(java.base@9.0/ServiceLoader.java:1029) at javax.imageio.spi.ServiceRegistry.lookupProviders(java.desktop@9.0/ServiceRegistry.java:199) at javasoft.sqe.tests.api.javax.imageio.spi.ServiceRegistry.lookupProvidersTests.lookupProviders001(lookupProvidersTests.java:39) at sun.reflect.NativeMethodAccessorImpl.invoke0(java.base@9.0/Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(java.base@9.0/NativeMethodAccessorImpl.java:62) at sun.reflect.DelegatingMethodAccessorImpl.invoke(java.base@9.0/DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(java.base@9.0/Method.java:500) at javasoft.sqe.javatest.lib.MultiTest.invokeTestCase(MultiTest.java:405) at javasoft.sqe.javatest.lib.MultiTest.run(MultiTest.java:194) at javasoft.sqe.javatest.lib.MultiTest.run(MultiTest.java:126) at javasoft.sqe.tests.api.javax.imageio.spi.ServiceRegistry.lookupProvidersTests.main(lookupProvidersTests.java:28)
05-06-2015
I was working up a proposal to run past Phil when both he and I were hit by vulnerabilities. I'll check with him again. My comment of 2015-04-13 summarizes my current thinking.
15-05-2015
Hi Stuart, any update on this issue? do we have some plan to proceed?
15-05-2015
Sorry, I have been working on this and discussing this with Phil. However, he and I are now swamped with vulnerabilities so we're unlikely to be able to look at this for a while. I am working on a proposal, the gist of which is as follows: - Deprecate the "mis-uses" of ServiceRegistry to load anything other than the imageio-related SPIs. - I believe such usage is rare. I did an informal survey and discovered one such usage in some open source project; it had decided to migrate to ServiceLoader a couple years ago since they didn't want a dependency "on AWT." There are however many uses of ImageIO to load codecs for custom image formats. - Attempting to use ServiceRegistry to load non-imageio SPIs will be met with a ServiceConfigurationError because of the lack of "uses" declaration in java.desktop's module-info.java (as noted in Mandy's comment above 2015-02-19). I don't think it's necessary to add code to block such usage, since it's effectively blocked already anyway. However.... - As a compatibility aid it might be useful to add a command-line option something like the following -XX:AddModuleUses=java.desktop/com.example.MyServiceInterface by analogy to the existing -XX:AddModuleExports and -XX:AddModuleRequires options. This new option would effectively add the "uses" declaration at runtime in order to support cases where the customer isn't in a position to recompile the code to use ServiceLoader. It's likely that providers of such SPIs would be in the unnamed module (i.e. on the classpath) and so -XX:AddModuleProvides wouldn't seem necessary, but maybe that ought to be considered too.
13-04-2015
I'm not aware of any recent discussion on this issue but it should be straight-forward to limit ServiceRegistry to just the image I/O service types. The only compatibility impact should be mis-uses where someone is using ServiceRegistry instead of ServiceLoader to locate non image I/O service providers.
10-04-2015
I assume the original motive for these methods was to make it easy to locate implementations of the service types defined in javax.imageio.spi. So maybe the simplest thing to do is to just re-specify these methods to only work with these service types and to throw IAE (or maybe UOE) if the called with a service type that is not one of the allowed types. It would of course be interesting to see if there are applications or libraries using these methods to locate service types that aren't imageio.spi types.
20-02-2015
It would be useful to get some idea as to whether these methods are used. If just aren't used then changing their spec (to admit their brokeness) and deprecating them may be the simplest solution. On the other hand, if there is a lot of usage then it requires looking into the solution along the lines that I outlined. In any case, there it should be easy to migrate existing code to use ServiceLoader directly.
12-01-2015
Would deprecation help ? There would still be an API that fails to work and the JCK test would presumably fail too. I need to better understand the problem before deciding on a solution but I would hope that jigsaw does not preclude supporting existing standard SE public APIs.
12-01-2015