WARNING: Illegal reflective access by com.github.jknack.handlebars.context.MemberValueResolver

[free2bcreative]

I've been working on migrating some projects from java 8 to java 11. As I was compiling, I noticed a warning in the log:

WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by com.github.jknack.handlebars.context.MemberValueResolver (../.m2/repository/com/github/jknack/handlebars/4.1.2/handlebars-4.1.2.jar) to method java.util.Collections$EmptyMap.isEmpty()
WARNING: Please consider reporting this to the maintainers of com.github.jknack.handlebars.context.MemberValueResolver
WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
WARNING: All illegal access operations will be denied in a future release

So, I figured I'd create an issue and make sure you're aware. It works with java 11...not sure what will happen with java 12 which should be released March 2019.

[ThStock]

+1

[jugalkmahendra]

+1
I get similar warning (as below) for FieldValueResolver class.

WARNING: Illegal reflective access by com.github.jknack.handlebars.context.FieldValueResolver$FieldMember (file:/Users/jugal_mahendra/.m2/repository/com/github/jknack/handlebars/4.1.2/handlebars-4.1.2.jar) to field java.util.HashMap.table
WARNING: Please consider reporting this to the maintainers of com.github.jknack.handlebars.context.FieldValueResolver$FieldMember
WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
WARNING: All illegal access operations will be denied in a future release

[jknack]

would you like to submit a pull request?

[skrysmanski]

Just a note: I can reproduce this when running the tests in Issue588.

Java 11 basically complains about this line in MemberValueResolver.cache():

((AccessibleObject) m).setAccessible(true);

This StackOverflow answer backs this up. It seems, starting with Java 9, you just can't use setAccessible() anymore the way you used to. (So, I'm guessing the fix won't be that simple.)

[ghost]

Same error with OpenJDK 11.

[huehnerlady]

Same error with OpenJDK 12

[reachkvperumal]

same error with OpenJDK 14

[ubertobarbini]

I am willing to write a PR to fix this issue, but AFAIK this would mean drop compatibility for Java < 9.
Would this be acceptable?

[Pe-te]

Have closed the duplicate ticket for the same problem, here are my steps to reproduce:

macOS, homebrew, swagger-codegen version 3.0.18

openapi: 3.0.0
info:
  title: Thingies App
  version: "1"
  description: |+
    ...

paths:
  /thingies/load_all:
    post:
      responses:
        200:
          description: Load all thingies
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/Thingies"
servers:
  - url: https://thingies.test/api
components:
  schemas:
    Thingies:
      type: object
      properties:
        property_one:
          type: string

swagger-codegen generate -l swift5 -i "${YAML_FILE}" -o "${SWIFT_PATH}"

[agentgt]

@jknack The way to avoid this is error message for most folks is to make MapValueResolver smarter.

MapValueResolver should resolve name = "empty" and name = "size" but currently does not.

If we know the context object is an instanceof Map and name is size or empty it should resolve but currently does not:

  @SuppressWarnings({"rawtypes", "unchecked" })
  @Override
  public Object resolve(final Object context, final String name) {
    Object value = null;
    if (context instanceof Map) {
      value = ((Map) context).get(name);
      // fallback to EnumMap
      if (value == null && context instanceof EnumMap) {
        EnumMap emap = (EnumMap) context;
        if (emap.size() > 0) {
          Enum first = (Enum) emap.keySet().iterator().next();
          Enum key = Enum.valueOf(first.getClass(), name);
          value = emap.get(key);
        }
      }
    }
    return value == null ? UNRESOLVED : value;
  }

Similar to how you are doing an check if its an enummap you would do the same if its a map in general (and then cast and call empty or size).

There are probably more methods than that but that is what I see in general.

[agentgt]

I will put in a pull request later if there is interest.

[agentgt]

Whoops I guess you still need a custom JavaBeanValueResolver that respects public/private:

Overriding members with something like this fixes it:

@Override
protected Set<Method> members(
		Class<?> clazz) {
	if (Modifier.isPublic(clazz.getModifiers())) {
		return super.members(clazz);
	}
	return Collections.emptySet();
}

[JamesGuthrie]

This has become a runtime exception on jdk 16.

[yuzawa-san]

@jknack I found that using partials specifically causes exceptions in later JDK versions. There is some nuance here around the bean resolver identifying java.util.Collections$EmptyMap.isEmpty as a bean field for the name empty. That method is public which is fine, but EmptyMap is private, so it cannot be made accessible in newer JDK's.

I came up with #870
There is the larger issue of a user passing and Collections.emptyMap() in their context, but this PR does not solve for that specific case.

[agentgt]

@yuzawa-san and @jknack The solution is far simpler and less of a hack than using HashMap.

All that really needs to be done is to not call setAccessible. The isEmpty is completely fine to call as the method is actually public and you can call public methods on private classes through reflection. You just can't call setAccessible on it.

The real fix is to fix MemberValueResolver cache function:

  private Map<String, M> cache(final Class<?> clazz) {
    Map<String, M> mcache = this.cache.get(clazz);
    if (mcache == null) {
      mcache = new HashMap<>();
      Set<M> members = members(clazz);
      for (M m : members) {
        // Mark as accessible.
        if (m instanceof AccessibleObject) {
          ((AccessibleObject) m).setAccessible(true);
        }
        mcache.put(memberName(m), m);
      }
      this.cache.put(clazz, mcache);
    }
    return mcache;
  }

What we need to do is not call setAccessible blindly on JDK 9 or higher particularly JDK 16 and above.
We can't just not blindly call setAccesible because that would break others that rely on it.

So I propose we do what JMustache does https://github.com/samskivert/jmustache and use a flag. A system property or something wired in that allows setAccesible to be called.

The flag could be on by default unless its set to false or JDK 17 is detected and its not set.

Now this is still doesn't filter all the Member (field and method) so you will still get an IllegalAccessException but at least it is an exception and not the glaring WARNING.

@jknack I have had this fix for a long time in our own custom value resolvers. I can put in a PR if you are interested in releasing . Otherwise folks can simple just copy the MemberValueResolver code and make their own ValueResolvers.

Also my solution uses MethodHandles since that is the future of reflection.