Repository: raphw/byte-buddy
Issue: JavaConstant.MethodHandle.getDescriptor() returns wrong value for PUT_FIELD and PUT_FIELD_STATIC cases

As always, thanks for this project.  I am nervous filing this bug because there are so few bugs in ByteBuddy that I feel like this can't actually be one!  But I think it is.

For background, see https://stackoverflow.com/questions/64296731/how-do-i-install-and-use-a-constant-methodhandle-in-bytebuddy.

In the case of `PUT_FIELD` and `PUT_FIELD_STATIC` `HandleType`s, I think that the wrong value is being returned from the default implementation of `getDescriptor()`.  Specifically, this will return `V` in "field setter" cases, because of this code:

https://github.com/raphw/byte-buddy/blob/291a512c609b647edccfdead540764ee402abd00/byte-buddy-dep/src/main/java/net/bytebuddy/utility/JavaConstant.java#L695-L696

For a "field setter" method handle, this will always return `V`, and that will cause a `ClassFormatError` that resembles the following:
```
java.lang.ClassFormatError: Field "fortyTwo" in class com/foo/bar/GeneratedSubclassOf$com$foo$bar$Baz$26753A95 has illegal signature "V"
```

This is because the return type of a "field setter" method handle as acquired via `JavaConstant.MethodHandle.ofSetter(FieldDescription.InDefinedShape)` will always be `void`, which is correct (see line 626 below):

https://github.com/raphw/byte-buddy/blob/291a512c609b647edccfdead540764ee402abd00/byte-buddy-dep/src/main/java/net/bytebuddy/utility/JavaConstant.java#L623-L627

But [section 5.4.3.5 of the JVM specification](https://docs.oracle.com/javase/specs/jvms/se15/html/jvms-5.html#jvms-5.4.3.5-240) indicates that if your method handle is of types 1 through 4 inclusive (which `PUT_FIELD` and `PUT_STATIC_FIELD` are), then resolution of the constant must follow the rules of field resolution.  [Those rules](https://docs.oracle.com/javase/specs/jvms/se15/html/jvms-5.html#jvms-5.4.3.2) indicate that the descriptor of the field reference needs to be used.  ByteBuddy is instead using the method handle's return type descriptor, which, in the case of field setter method handles, anyway, is `V`.

The workaround that seems to get around this problem is to create a subclass of `JavaConstant.MethodHandle` that overrides the `getDescriptor()` method to do the right thing:
```java
@Override
public final String getDescriptor() {
  // See https://stackoverflow.com/questions/64296731/how-do-i-install-and-use-a-constant-methodhandle-in-bytebuddy.
  switch (this.getHandleType()) {
  case PUT_FIELD:
  case PUT_STATIC_FIELD:
    // In the case of "field setter" method handles use the sole parameter type's descriptor
    // to follow the rules prescribed by
    // https://docs.oracle.com/javase/specs/jvms/se15/html/jvms-5.html#jvms-5.4.3.5-240
    return this.getParameterTypes().getOnly().getDescriptor();
  default:
    return super.getDescriptor();
  }
}
```

With this override, I can avoid the `ClassFormatError`.  (I don't know if it is best to solve the problem here or in `asConstantPoolValue()`.)

I'll attempt to put together a PR shortly.

Thanks again for a great project.

Changed files:
byte-buddy-dep/src/test/java/net/bytebuddy/utility/JavaConstantMethodHandleTest.java
byte-buddy-dep/src/main/java/net/bytebuddy/utility/JavaConstant.java

diff --git a/byte-buddy-dep/src/main/java/net/bytebuddy/utility/JavaConstant.java b/byte-buddy-dep/src/main/java/net/bytebuddy/utility/JavaConstant.java
index 51ad4443c8..5e0e4e2bb6 100644
--- a/byte-buddy-dep/src/main/java/net/bytebuddy/utility/JavaConstant.java
+++ b/byte-buddy-dep/src/main/java/net/bytebuddy/utility/JavaConstant.java
@@ -692,9 +692,11 @@ public interface JavaConstant {
          * @return The method descriptor of this method handle representation.
          */
         public String getDescriptor() {
-            if (handleType.isField()) {
-                return returnType.getDescriptor();
-            } else {
+            switch (handleType) {
+            case PUT_FIELD:
+            case PUT_STATIC_FIELD:
+                return parameterTypes.get(0).getDescriptor();
+            default:
                 StringBuilder stringBuilder = new StringBuilder().append('(');
                 for (TypeDescription parameterType : parameterTypes) {
                     stringBuilder.append(parameterType.getDescriptor());
diff --git a/byte-buddy-dep/src/test/java/net/bytebuddy/utility/JavaConstantMethodHandleTest.java b/byte-buddy-dep/src/test/java/net/bytebuddy/utility/JavaConstantMethodHandleTest.java
index b0b482f049..71b8ed1c47 100644
--- a/byte-buddy-dep/src/test/java/net/bytebuddy/utility/JavaConstantMethodHandleTest.java
+++ b/byte-buddy-dep/src/test/java/net/bytebuddy/utility/JavaConstantMethodHandleTest.java
@@ -20,7 +20,7 @@ import static org.mockito.Mockito.when;
 
 public class JavaConstantMethodHandleTest {
 
-    private static final String BAR = "bar", QUX = "qux";
+    private static final String BAR = "bar", FROB = "frob", QUX = "qux";
 
     @Rule
     public MethodRule javaVersionRule = new JavaVersionRule();
@@ -106,12 +106,13 @@ public class JavaConstantMethodHandleTest {
     @Test
     @SuppressWarnings("unchecked")
     public void testMethodHandleOfSetter() throws Exception {
-        JavaConstant.MethodHandle methodHandle = JavaConstant.MethodHandle.ofSetter(Foo.class.getDeclaredField(BAR));
+        JavaConstant.MethodHandle methodHandle = JavaConstant.MethodHandle.ofSetter(Foo.class.getDeclaredField(FROB));
         assertThat(methodHandle.getHandleType(), is(JavaConstant.MethodHandle.HandleType.PUT_FIELD));
-        assertThat(methodHandle.getName(), is(BAR));
+        assertThat(methodHandle.getName(), is(FROB));
         assertThat(methodHandle.getOwnerType(), is((TypeDescription) TypeDescription.ForLoadedType.of(Foo.class)));
         assertThat(methodHandle.getReturnType(), is(TypeDescription.VOID));
-        assertThat(methodHandle.getParameterTypes(), is((List<TypeDescription>) new TypeList.ForLoadedTypes(Void.class)));
+        assertThat(methodHandle.getParameterTypes(), is((List<TypeDescription>) new TypeList.ForLoadedTypes(Integer.class)));
+        assertThat(methodHandle.getDescriptor(), is(methodHandle.getParameterTypes().getOnly().getDescriptor()));
     }
 
     @Test
@@ -123,6 +124,7 @@ public class JavaConstantMethodHandleTest {
         assertThat(methodHandle.getOwnerType(), is((TypeDescription) TypeDescription.ForLoadedType.of(Foo.class)));
         assertThat(methodHandle.getReturnType(), is(TypeDescription.VOID));
         assertThat(methodHandle.getParameterTypes(), is((List<TypeDescription>) new TypeList.ForLoadedTypes(Void.class)));
+        assertThat(methodHandle.getDescriptor(), is(methodHandle.getParameterTypes().getOnly().getDescriptor()));
     }
 
     @Test
@@ -195,6 +197,8 @@ public class JavaConstantMethodHandleTest {
 
         public Void bar;
 
+        public Integer frob;
+      
         public Foo(Void value) {
             /* empty*/
         }