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

Fix reading bytecode version for java1 #1510

Merged
merged 1 commit into from
Aug 24, 2023
Merged

Conversation

SylvainJuge
Copy link
Contributor

The Java class file format (doc) stores the bytecode version in the bytes 4 to 8 just after the famous 0xCAFEBABE magic value.
In practice, the minor version hasn't been used since Java1, and both minor and major can be read as single bytes.

When reading the version of an existing class compiled for Java 1, the minor version (3) is read at the wrong offset and is thus ignored, however the major version (45) is properly read, which makes it as if we were attempting to read a class that was compiled for Java 1.0 (doc), which is not covered by ASM as there isn't any Opcodes.V1_0.

This PR fixes the bug in version reading and adds a test to cover it.

@raphw raphw merged commit b7f94db into raphw:master Aug 24, 2023
@raphw raphw self-assigned this Aug 24, 2023
@raphw raphw added the question label Aug 24, 2023
@raphw raphw added this to the 1.14.6 milestone Aug 24, 2023
@raphw
Copy link
Owner

raphw commented Aug 24, 2023

Good catch! I just moved the test to the parameterized version.

@SylvainJuge
Copy link
Contributor Author

Thanks for merging this !

This issue is linked to the 1.14.6 milestone, however this one has been released last week (and it's not included in the release notes), does it means it should actually be part of the 1.14.7 (or whatever comes next) release ?

@SylvainJuge SylvainJuge deleted the java1-version branch August 24, 2023 08:10
@raphw
Copy link
Owner

raphw commented Aug 24, 2023

I use this feature poorly where I mark the version where this was reported as I normally solve the issue within the next release. Pushing a release now.

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

Successfully merging this pull request may close these issues.

None yet

2 participants