ContentTypeDetector should recognize future versions of Java class files

[Godin]

Each time new version of bytecode comes out, we have following inconsistency between processing of classes by agent and processing of files:

offline instrumentation and report generation skip class file

$ cat <<END > Example.java
class Example {
  public static void main(String[] args) {
  }
END

$ javac --release 14 Example.java -d classes

$ ls classes
Example.class

$ java -jar jacoco-0.8.3/lib/jacococli.jar instrument classes --dest instrumented
[INFO] 0 classes instrumented to /tmp/j/instrumented.

$  java -jar jacoco-0.8.3/lib/jacococli.jar report --classfiles classes
[WARN] No execution data files provided.
[INFO] Analyzing 0 classes.

which is IMO misleading and error-prone,
whereas on-the-fly instrumentation fails

$ java -javaagent:jacoco-0.8.3/lib/jacocoagent.jar -cp classes Example
java.lang.instrument.IllegalClassFormatException: Error while instrumenting Example.
        at org.jacoco.agent.rt.internal_1f1cc91.CoverageTransformer.transform(CoverageTransformer.java:93)
        at java.instrument/java.lang.instrument.ClassFileTransformer.transform(ClassFileTransformer.java:246)
        at java.instrument/sun.instrument.TransformerManager.transform(TransformerManager.java:188)
        at java.instrument/sun.instrument.InstrumentationImpl.transform(InstrumentationImpl.java:563)
        at java.base/java.lang.ClassLoader.defineClass1(Native Method)
        at java.base/java.lang.ClassLoader.defineClass(ClassLoader.java:1016)
        at java.base/java.security.SecureClassLoader.defineClass(SecureClassLoader.java:151)
        at java.base/jdk.internal.loader.BuiltinClassLoader.defineClass(BuiltinClassLoader.java:823)
        at java.base/jdk.internal.loader.BuiltinClassLoader.findClassOnClassPathOrNull(BuiltinClassLoader.java:721)
        at java.base/jdk.internal.loader.BuiltinClassLoader.loadClassOrNull(BuiltinClassLoader.java:644)
        at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:602)
        at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:178)
        at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:521)
        at java.base/java.lang.Class.forName0(Native Method)
        at java.base/java.lang.Class.forName(Class.java:416)
        at java.base/sun.launcher.LauncherHelper.loadMainClass(LauncherHelper.java:760)
        at java.base/sun.launcher.LauncherHelper.checkAndLoadMain(LauncherHelper.java:655)
Caused by: java.io.IOException: Error while instrumenting Example.
        at org.jacoco.agent.rt.internal_1f1cc91.core.instr.Instrumenter.instrumentError(Instrumenter.java:170)
        at org.jacoco.agent.rt.internal_1f1cc91.core.instr.Instrumenter.instrument(Instrumenter.java:120)
        at org.jacoco.agent.rt.internal_1f1cc91.CoverageTransformer.transform(CoverageTransformer.java:91)
        ... 16 more
Caused by: java.lang.IllegalArgumentException: Unsupported class file major version 58
        at org.jacoco.agent.rt.internal_1f1cc91.asm.ClassReader.<init>(ClassReader.java:184)
        at org.jacoco.agent.rt.internal_1f1cc91.asm.ClassReader.<init>(ClassReader.java:166)
        at org.jacoco.agent.rt.internal_1f1cc91.asm.ClassReader.<init>(ClassReader.java:152)
        at org.jacoco.agent.rt.internal_1f1cc91.core.internal.instr.InstrSupport.classReaderFor(InstrSupport.java:247)
        at org.jacoco.agent.rt.internal_1f1cc91.core.instr.Instrumenter.instrument(Instrumenter.java:86)
        at org.jacoco.agent.rt.internal_1f1cc91.core.instr.Instrumenter.instrument(Instrumenter.java:118)
        ... 17 more

This happens because ContentTypeDetector has strict version check
and used by Instrumenter.instrumentAll and Analyzer.analyzeAll,
but not by Instrumenter.instrument and Analyzer.analyzeClass.

This check was introduced in 73ae850 to distinguish class files from Mach-O fat/universal binaries, because both have 0xCAFEBABE magic header.

To resolve above inconsistencies, simplify ContentTypeDetector and to avoid constant need to update it when new bytecode version comes out, I propose to relax check by just checking that unsigned 2 bytes number at position of major version is greater or equal to 45 (0x2D, Java 1.0 - 1.1).

For distinction from Mach-O this will be as good as current check: header of fat binary contains unsigned 4 bytes for number of architectures at a same place and in a same big-endian order where class file contains unsigned 2 bytes for minor and 2 bytes for major version, so both theoretically can be 46 and both will pass current check, however unlikely to see on practice single executable with such number of architectures.

[Godin]

@marchof we discussed this at JCrete 😉

I was thinking about reduction of cases in ContentTypeDetectorTest, and was questioning myself whether this should be classified as "Bug fix" or "API change"? Or maybe you'll have some other advices after review?

[marchof]

@Godin It's not a bug, it's a feature 😜

Unsupported class file versions are now consistently reported as exceptions also for report generation and offline instrumentation.

But your proposal is also good for me.