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

Update InstrumentedType.java to check instrumented classfile is in valid Unicode namespace instead #1613

Merged
merged 1 commit into from
Apr 4, 2024

Conversation

leerjae
Copy link
Contributor

@leerjae leerjae commented Apr 3, 2024

We own an Amazon open-source project which utilizes ByteBuddy, and this PR for ByteBuddy solves an obstacle we have encountered while supporting our customers Our project: https://github.com/awslabs/disco

In some cases we need to instrument shaded classes with FQN such as *.!internal.* that are valid JVM classfile names but are not accepted by the compiler. This is intentionally done to prevent external dependencies from using the shaded classes. These classes’ bytecode are valid and are processed and have its bytecode changed by ASM but failing InstrumentedType.validate() post-instrumentation validation because of isValidIdentifier() which only allows Java Identifier characters. Is it possible to add slack to the instrumented classfile name validation to allow characters from the Unicode namespace instead? According to the Oracle's JVM specification, classfile names can be drawn from all of the Unicode namespace (see link above).

@raphw raphw merged commit 95b083f into raphw:master Apr 4, 2024
@raphw raphw self-assigned this Apr 4, 2024
@raphw raphw added this to the 1.14.13 milestone Apr 4, 2024
@raphw
Copy link
Owner

raphw commented Apr 4, 2024

Makes sense to me as Byte Buddy follows the class file specification rather than the language specification. I was not aware of this. Generally, I do however recommend to disable validation unless testing.

@leerjae
Copy link
Contributor Author

leerjae commented Apr 9, 2024

Hi raphw, thank you for quickly accomodating the pull request. Also I've been trying to understand the additional type validation that BB does outside of what the JVM provides. I've been digging into the JVM specs and InstrumentType.validate() source code to try and understand how BB type validation is more expressive but having hard time wrapping my head around it. At a high level can you tell me how "Byte Buddy's checks might be more expressive in the context of using the library"?

@raphw
Copy link
Owner

raphw commented Apr 13, 2024

The validation will create a "better" error message than the VerificationError that the JVM emits. This is less true in newer JVM versions where the error message by the JVM is fairly understandable.

The main difference however is that a verification error in the JVM will break the class forever. If Byte Buddy fails verification during an agent's application for example, the instrumentation is aborted instead.

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

Successfully merging this pull request may close these issues.

2 participants