FieldValueResolver leads to IllegalAccessException in 4.3.0 #940
Comments
|
@mdesharnais I get this as well. The underlying problem seems to be that the access restrictions are stronger in Java 17. I resolved it by overriding @Override
public boolean matches(FieldWrapper field, String name) {
return !isStatic(field) && field.getName().equals(name) && isPublic(field);
}Presumably, this will be a problem for any templates that have assumed access to private fields. |
|
Guys, You need to stop accessing private fields in Java 17. Field must be public or you need a getter method. |
|
Hey @jknack: This happens when you pass a map into handlebars. We're not the ones using private fields, you are ;-) I tried the following, where Handlebars handlebars = new Handlebars(new ClassPathTemplateLoader());
Template tmpl = handlebars.compile("example.txt");
System.out.println(tmpl.apply(Context
.newBuilder(new HashMap<String, String>(Map.of()))
.resolver(FieldValueResolver.INSTANCE)
.build()));The result is the same as @mdesharnais 's stack trace above. |
|
@jhannes Look here. It checks what Java version are you running and setup the value resolvers properly. This work: Because it uses the default value resolvers. Now if you override that and just add FieldValueResolver, then it won't work. If yo still need to access to public field then add it to the end: |
|
@jknack Thanks for the clarification. This looks good. However, I can't get it to work properly in my context, which is OpenAPI Generator I've tried overriding OpenAPI's adaptor and remove the call to |
|
Odd. We can probably add the |
|
I totally agree. Private fields are going away. But at the moment, there is no migration path for e.g. openapi-generator. I'll examine the behavior a bit more so I can create a better reproduction |
|
@jknack I have made some more research into the OpenAPI with Java 17 issue and have found an example that mimics the issue. The following test fails to generate output: class FixedHandlebarsEngineAdapterTest {
public static class MyFieldValueResolver extends FieldValueResolver {
@Override
public boolean matches(FieldWrapper field, String name) {
return super.matches(field, name) && !isTransient(field);
}
protected boolean isTransient(FieldWrapper member) {
return Modifier.isTransient(member.getModifiers());
}
}
@Test
void java17failure() throws IOException {
HashMap<Object, Object> bundle = exampleModel();
Context context = Context
.newBuilder(bundle)
// Commenting in the following will make it work
/*
.resolver(
MapValueResolver.INSTANCE,
JavaBeanValueResolver.INSTANCE,
new MyFieldValueResolver())
*/
.build();
Handlebars handlebars = new Handlebars();
// Commenting out this will make it work again
/*
*/
handlebars.registerHelperMissing((obj, options) -> {
System.err.printf(Locale.ROOT, "Unregistered helper name '%s', processing template:%n%s%n", options.helperName, options.fn.text());
return "";
});
handlebars.getLoader().setPrefix("/typescript-fetch-api/");
handlebars.getLoader().setSuffix(".handlebars");
handlebars.registerHelper("json", Jackson2Helper.INSTANCE);
StringHelpers.register(handlebars);
handlebars.registerHelpers(ConditionalHelpers.class);
handlebars.registerHelpers(org.openapitools.codegen.templating.handlebars.StringHelpers.class);
Template tmpl = handlebars.compile("model");
String output = tmpl.apply(context);
System.out.println(output);
assert !output.isBlank();
}
private HashMap<Object, Object> exampleModel() {
HashMap<Object, Object> bundle = new HashMap<>();
ArrayList<Object> models = new ArrayList<>();
bundle.put("models", models);
Map<String, Object> petDto = new HashMap<>();
petDto.put("importPath", "PetDto");
CodegenModel petModel = new CodegenModel();
petModel.classname = "PetDto";
petModel.parent = "AnimalDto";
petModel.isEnum = false;
CodegenProperty property = new CodegenProperty();
property.name = "petName";
property.dataType = "string";
petModel.vars.add(property);
petDto.put("model", petModel);
models.add(petDto);
return bundle;
}
}Given the following templates: The issue seems to be composed of several problems and I don't know why it doesn't work. I'm wondering if Block.java is checking for As Handlebars use in OpenAPI Generator is an important use case for the project, I think this is could be an important issue |
|
Hi @jknack @jhannes, import java.util.Map;
import java.util.List;
import com.github.jknack.handlebars.Context;
import com.github.jknack.handlebars.Handlebars;
import com.github.jknack.handlebars.context.FieldValueResolver;
import com.github.jknack.handlebars.context.JavaBeanValueResolver;
import com.github.jknack.handlebars.context.MapValueResolver;
import com.github.jknack.handlebars.context.MethodValueResolver;
public final class Main {
public static void main(final String[] args) throws Throwable {
final var handlebars = new Handlebars();
final String str = handlebars
.compileInline("{{value}}{{#strings}} {{value}}.{{.}}{{/strings}}")
.apply(
Context.newBuilder(
Map.of(
"value", "aaa",
"strings", List.of("bbb", "ccc")))
.resolver(
MapValueResolver.INSTANCE,
JavaBeanValueResolver.INSTANCE,
MethodValueResolver.INSTANCE,
FieldValueResolver.INSTANCE)
.build());
System.out.println(str);
}
}I would expect the output to be "aaa aaa.bbb aaa.ccc", but get the same Considering that there is no way to know or control the fields unexposed by third party code, I argue that |
I second @mdesharnais 's proposal here |
|
Field must be public in modern JVM. You can see the root cause here: That is the JVM throwing the error, it isn't something the handlebars.java does. If the field doesn't exist, handlebars.java skip the resolver and execute next on the chain/pipeline. Are you asking to ignore existing non-public fields on modern JVM? |
|
Sorry for the delayed answer. Yes, I am asking for the |
|
@jknack we probably could do something like the same trick we did here: Basically ignore field access to any class that is in the "java." or "sun." package including possibly even public ones (because in a modular world even if they are public they may not be Otherwise we would have to use the modules API which would not work for JDK 8 compile. |
|
The other option is to include modern value resolvers that are module aware in a separate maven module (not java module but maven) and compile them with JDK 11/17 (E.g. Then in the JDK 8 code base you could use the Service Loader to load them up (to avoid using weird reflection hacks or check java version). This would require a new SPI. |
Just so we are clear... that will not always work. I field can be Anyway jknack chose the correct behavior on not including the FieldValueResolver as a default precisely because of these problems. With records now ideally one should not use field value resolver for struct objects anyway (albeit I get backwards compatibility). |
|
Hi @agentgt, thanks for looking into that.
You are correct, I should have written, that I am asking for the |
|
OK I see the issue. The cache still has members that are not accessible. 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 (isUseSetAccessible(m) && m instanceof AccessibleObject) {
((AccessibleObject) m).setAccessible(true);
}
////// WE are still adding the member
mcache.put(memberName(m), m);
}
this.cache.put(clazz, mcache);
}
return mcache;
}One of the reasons I was looking into this is I made some of the reflection changes and the above is sort of a bug. Anyway @jknack It looks we first need to check if its a 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) {
if (isUseSetAccessible(m)) {
((AccessibleObject) m).setAccessible(true);
mcache.put(memberName(m), m);
}
}
else {
mcache.put(memberName(m), m);
}
}
this.cache.put(clazz, mcache);
}
return mcache;
} |
|
Anyway a work around is this Field Value Resolver: var MY_FIELD_VALUE_RESOLVER = new FieldValueResolver() {
@Override
protected Set<FieldWrapper> members(
Class<?> clazz) {
var members = super.members(clazz);
return members.stream()
.filter(fw -> isValidField(fw))
.collect(Collectors.toSet());
}
boolean isValidField(
FieldWrapper fw) {
if (fw instanceof AccessibleObject) {
if (isUseSetAccessible(fw)) {
return true;
}
return false;
}
return true;
}
};You will need to java 8 if thats your target. |
|
As I mentioned here #948 (comment) We really cannot change And they will file bugs like #951 that are very hard to reproduce without their complete object graph. The real solution is too deprecate We then add another module to this project compiling FieldValueResolver with Java 9 or whatever using |
Thanks for your help. It's working now. |
|
Is this included in 4.3.1? I still get "com.github.jknack.handlebars.HandlebarsException: baseApi.handlebars:4:3: java.lang.IllegalStateException: Shouldn't be illegal to access field 'size'" |
|
It looks like the fix has not been implemented yet. I use plugin for Maven The plugin is based on Handlebars 4.3.1 - https://github.com/OpenAPITools/openapi-generator/blob/ff2e173de8c68c304fa1cc1162a09aef44a34899/pom.xml#L1226 The proposed earlier FieldValueResolver is implemented in the current HandlebarsEngineAdapter - https://github.com/OpenAPITools/openapi-generator/blob/ff2e173de8c68c304fa1cc1162a09aef44a34899/modules/openapi-generator/src/main/java/org/openapitools/codegen/templating/HandlebarsEngineAdapter.java#L77 I wonder why HandlebarsEngineAdapter initializes FieldValueResolver before the MethodValueResolver: // $ref: https://github.com/jknack/handlebars.java/issues/917
var MY_FIELD_VALUE_RESOLVER = new FieldValueResolver() {
@Override
protected Set<FieldWrapper> members(
Class<?> clazz) {
var members = super.members(clazz);
return members.stream()
.filter(fw -> isValidField(fw))
.collect(Collectors.toSet());
}
boolean isValidField(
FieldWrapper fw) {
if (fw instanceof AccessibleObject) {
if (isUseSetAccessible(fw)) {
return true;
}
return false;
}
return true;
}
};
Context context = Context
.newBuilder(bundle)
.resolver(
MapValueResolver.INSTANCE,
JavaBeanValueResolver.INSTANCE,
MY_FIELD_VALUE_RESOLVER.INSTANCE,
MethodValueResolver.INSTANCE)
.build();I'm gonna try to change the resolvers order, as @jknack said:
I would try to build a customized Handlebars adapter for OpenAPI Generator as described here https://openapi-generator.tech/docs/templating/#custom-engines |
|
@dobromyslov to avoid reasoning on wrong assumptions
The proposed FieldValueResolver was so far never actually used (I opened a pull request to rectify the issue) |
Hi,
the following code, wich uses the
FieldValueResolver, leads to anInaccessibleObjectException.Assuming that this code is in the file
Main.javaand that, sitting next to it, the filefoo.hbscontains the following.And that the file
bar.hbscontains the following.Then the following command produces the exception.
The text was updated successfully, but these errors were encountered: