Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Java 9 java.specification.version failure #24

Closed
simonsteiner1984 opened this issue Apr 12, 2016 · 23 comments · Fixed by #48
Closed

Java 9 java.specification.version failure #24

simonsteiner1984 opened this issue Apr 12, 2016 · 23 comments · Fixed by #48

Comments

@simonsteiner1984
Copy link

Java 9 has java.specification.version == 9
ImageIO.getReaderMIMETypes();

Caused by: java.lang.NumberFormatException: For input string: ""
at java.lang.NumberFormatException.forInputString(java.base@9-ea/NumberFormatException.java:65)
at java.lang.Integer.parseInt(java.base@9-ea/Integer.java:705)
at java.lang.Integer.parseInt(java.base@9-ea/Integer.java:813)
at com.github.jaiimageio.impl.common.ImageUtil.processOnRegistration(ImageUtil.java:1401)

@tballison
Copy link

Just ran into this one, too, when testing Apache Tika with 9-ea.

@THausherr
Copy link

@jpsacha
Copy link

jpsacha commented Jun 14, 2016

The problem is with the code in ImageUtil line 1388:

String jvmVersionString = 
    System.getProperty("java.specification.version");
int verIndex = jvmVersionString.indexOf("1.");
// Skip the "1." part to get to the part of the version number that
// actually changes from version to version
// The assumption here is that "java.specification.version" is 
// always of the format "x.y" and not "x.y.z" since that is what has
// been practically observed in all JDKs to-date, an examination of
// the code returning this property bears this out. However this does
// not guarantee that the format won't change in the future, 
// though that seems unlikely.
jvmVersionString = jvmVersionString.substring(verIndex+2);

int jvmVersion = Integer.parseInt(jvmVersionString);

For Java 9 (current build) the "java.specification.version" has value just "9". The above code attempts to skip "1." at the beginning without checking that it is there.

A simple workaround would be to do a basic check:

String jvmVersionString = 
    System.getProperty("java.specification.version");
// Skip "1." in version name
if(jvmVersionString.contains("1.")) {
    // Now the old way, assuming that version always starts with "1.", true prior to version 9

    int verIndex = jvmVersionString.indexOf("1.");
    // Skip the "1." part to get to the part of the version number that
    // actually changes from version to version
    // The assumption here is that "java.specification.version" is
    // always of the format "x.y" and not "x.y.z" since that is what has
    // been practically observed in all JDKs to-date, an examination of
    // the code returning this property bears this out. However this does
    // not guarantee that the format won't change in the future,
    // though that seems unlikely.
    jvmVersionString = jvmVersionString.substring(verIndex + 2);
}

int jvmVersion = Integer.parseInt(jvmVersionString);

@THausherr
Copy link

Re version strings, you may want to read the discussion here:
https://issues.apache.org/jira/browse/PDFBOX-3155

Btw the JDK bug has been "solved", now it displays a stack trace and no longer fails.

@don-vip
Copy link

don-vip commented May 26, 2017

Can you please apply the patch proposed by @jpsacha ? Java 9 will be released in two months.

@sfeigl
Copy link

sfeigl commented Oct 12, 2017

Any chance we get a release that is JDK 9 fixed soon?

@nipafx
Copy link

nipafx commented Nov 30, 2017

I might open a PR soon. In preparation I asked on StackOverflow for a good way to determine the major version.

@simonsteiner1984
Copy link
Author

They have example code in https://issues.apache.org/jira/browse/PDFBOX-3155

@christopher-johnson
Copy link

The version finding method as the accepted answer in StackOverflow should replace the method in ImageUtil. @nicolaiparlog please submit a PR, so that we can resolve this very old (and annoying) issue. Thank you.

@nipafx
Copy link

nipafx commented Jan 11, 2018

@christopher-johnson I'd rather wait for a maintainer signaling readiness to accept such a PR.

@lfcnassif
Copy link

Any updates here?

@mipastgt
Copy link

Hi Nicolai,
could you please submit your PR. This bug is really a pain for everybody using Java 9 and beyond and 10 is just arround the corner. I doubt that you will get an explicit invitation here :-)

@pejobo
Copy link

pejobo commented Mar 5, 2018

@stain Hi Stian, this looks like a deadlock situation:

nicolaiparlog commented on 11 Jan

@christopher-johnson I'd rather wait for a maintainer signaling readiness to accept such a PR.

@stain
Copy link
Member

stain commented Mar 9, 2018

Hi, sorry about the hold up @pejobo - I've invited you as a maintainer and merged #48.

I can do the Maven release next week. Is there anything else that needs to go in?

@stain
Copy link
Member

stain commented Mar 9, 2018

Invited @pejobo and @nicolaiparlog as maintainers.

@nipafx
Copy link

nipafx commented Mar 9, 2018

Err, wow, that's quite the field promotion, thank you.

#48 solved this by string parsing - is there any interest in a solution like the one in the SO answer?

@pejobo
Copy link

pejobo commented Mar 9, 2018

Using reflection could induce other problems, I think the current solution is "good enough".

@nipafx
Copy link

nipafx commented Mar 10, 2018

Just in case you worry about the module system intervening and making reflection problematic, I want to point out that that's not a risk. All the involved classes and members are accessible and could be called just as well by non-reflection code. The only reason I don't do that is because it would make it necessary to compile the file with Java 9, which complicates things a lot.

That aside, I agree - the current solution is good enough. 👍

@stain
Copy link
Member

stain commented Mar 10, 2018

Reflection can also be blocked by security policies. A "clean" solution would be an intermediate class that delegates to a second class that calls Runtime.version normally, with a second fall-back to the current method if that intermediate class failed to load (because Runtime.version lacks before Java9). But yes, that would require that intermediate class to be compiled on Java 9.

@lbellonda
Copy link
Member

Hello, I think that solution simpler is better. I would like a solution that can be compiled on previous JDK without problems.
The current solution is good.

It is not possible check only first digits in the version string (just in case of a future 9.5) or trap the excution anyway in a catch block? The failure to detect the version at the moment is not a real problem given where it is used in the code (jvmVendor.equals("Sun Microsystems Inc.")).

At the moment the version is calculated outside of the block of code that use it. Moving the calculation inside the "if" would mitigate the problem.

Checking only digits allows the code to be binary compatible with some small changes in the JDK string format without forcing a new publication in a Maven repository.

@christopher-johnson
Copy link

A point of reference is org.gradle.api.JavaVersion

Where

println System.getProperty("java.specification.version")
println System.getProperty("java.version")
println Jvm.current().getJavaVersion()

produces:
9
9.0.4
1.9

Note that the JavaVersion is returned from getJavaVersion() in a backwards compatible format, despite the New JDK Versioning Scheme. The newly merged method is simple and will work, though a JavaVersion class that produces a stable string from java.version seems a bit more robust.

@lbellonda
Copy link
Member

Hello, the test from which the problem is born is for Java vendor "java.vendor" equals to "Sun Microsystems Inc.". Should the test be updated for Oracle too?

@pejobo
Copy link

pejobo commented Mar 16, 2018

Should the test be updated for Oracle too?

The "Oracle" vs. "Sun" thing is a bigger code change, where more places in the code are affected.
I would suggest to open a new bug and push a release with the latest code now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.