raphw / byte-buddy

Runtime code generation for the Java virtual machine.
https://bytebuddy.net
Apache License 2.0
6.23k stars 804 forks source link

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

Closed leerjae closed 6 months ago

leerjae commented 6 months ago

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 commented 6 months ago

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 commented 5 months ago

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 commented 5 months ago

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.