Summary: | Possible EL Bug — Method Matching with Varargs | ||
---|---|---|---|
Product: | Tomcat 9 | Reporter: | Volodymyr Siedlecki <volosied> |
Component: | EL | Assignee: | Tomcat Developers Mailing List <dev> |
Status: | RESOLVED FIXED | ||
Severity: | normal | ||
Priority: | P2 | ||
Version: | 9.0.x | ||
Target Milestone: | ----- | ||
Hardware: | PC | ||
OS: | Mac OS X 10.1 |
Description
Volodymyr Siedlecki
2021-06-05 01:57:54 UTC
The EL spec is silent on how methods should be selected when there isn't an exact match between arguments and method parameters. Generally, the rule Tomcat tries to follow is to pick the same method as the Java compiler would. In this instance if you call: testBean.getMessage(testBean.getValue1()) then the Java compiler will call: getMessage(final EnumInterface enumInstance) which is what I would expect. I would also expect EL to behave the same way. It doesn't. I need to dig into what is going on with EL. On the surface it looks like a bug but it needs some further investigation to determine what is going on and why. Yes, this is definitely an EL bug. The root cause is the weight given in the method mapping to a potential method match where the potential matching method uses varArgs but the EL expression doesn't specify any varArgs. Currently this is treated as an assignable match for the varargs parameter which is too strong a match. I need to write some test cases to explore how the Java compiler behaves around this particular edge case. With that information in hand, I'll know how big a change is required to javax.el.Util.findWrapper(). I have a one line fix that addresses the specific issue described here (and doesn't cause any of the existing tests to fail) but I want to make sure there aren't any other related edge cases that also need to be addressed. Thanks for looking into it, Mark! Just realized I made two mistakes in the original post. I should have said - value1 is of type TestEnum. - TestEnum of type EnumInterface Glad I linked the correct example, at least. The one-line fix wasn't sufficient. A broader fix was required that in essence looked at the number of varargs used and preferred the method that matched using fewest varargs. Fixed in: - 10.1.x for 10.1.0-M1 onwards - 10.0.x for 10.0.7 onwards - 9.0.x for 9.0.47 onwards - 8.5.x for 8.5.67 onwards I was looking at this again (on 9.0.50). I think there's still a bug here? Using the code below: Bean bean = new Bean(); // JVM bean.bark(bean.getDog()); // -> bark(Animal animal) // EL ${ bean.bark(bean.dog) } // - > bark(Dog dog, String... text) _______ public interface Animal { } public class Dog implements Animal { } public class Bean { public String bark(Animal animal){ return "bark(Animal animal)"; } public String bark(Dog dog, String... text){ return "bark(Dog dog, String... text)"; } public Animal getDog(){ System.out.println("Returning Dog of type Animal"); return new Dog(); } } Yes, that looks like a bug to me. The JLS states (in rather more precise language) that a method match that doesn't use varargs beats one that does. I'll work on a fix. Fixed in: - 10.1.x for 10.1.0-M3 onwards - 10.0.x for 10.0.9 onwards - 9.0.x for 9.0.51 onwards - 8.5.x for 8.5.70 onwards Hey Mark, I hate to keep following up on this, but I think it's still not quite right. Please look at these two test cases I added: https://github.com/volosied/tomcat/commit/d6623d814ea30674fa27bda7dcbff3b1c268ba0b However, this patch I created seems to resolve both scenarios without another other failures. https://github.com/volosied/tomcat/commit/d876f1e57a265970e0653c2a906b66f18d91784a Thanks for the test cases. They demonstrate very well the ambiguity that exists in the current EL 4.0 spec. The question is, essentially, one of priority of matches. If we look at the Java language to start with, if a set of parameters could match multiple methods then the "best" match is chosen based on these checks in this order (the comparison ends as soon as a rule determines a method is better): - a non-varargs method is better than a var-args method - methods that match more exact parameter methods are better - methods that match more assignable parameters are better - methods that match fewer parameters using varargs are better What complicates things for EL is that it adds type coercion. The tests in the attached patch assume that type coercion is lower priority than "a non-varargs method beats a var-args method". This is not how the code was designed. The code was designed to implement: - a non-varargs method is better than a var-args method - methods that match more exact parameter methods are better - methods that match more assignable parameters are better - methods that match more coercible parameters are better - methods that match fewer parameters using varargs are better The next release of the EL spec will contain a clarification to this effect. Given this bug report, I'll probably add to that text to make clear that non-varargs still beats varargs even when coercion is used. Therefore, the matches observed in the tests are as expected because coercing an Integer to String is preferred to matching an Integer varargs. Thank for the detailed explanation, Mark! I'll leave this issue alone then. |