Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"Two vararg methods with same score" for resteasy ClientWebTarget.request() #125

Closed
petergeneric opened this issue May 1, 2021 · 13 comments · Fixed by #126
Closed

"Two vararg methods with same score" for resteasy ClientWebTarget.request() #125

petergeneric opened this issue May 1, 2021 · 13 comments · Fixed by #126

Comments

@petergeneric
Copy link
Contributor

petergeneric commented May 1, 2021

In OGNL 3.2.20 I'm experiencing the following log message. I think this error may be spurious here since there's a no-args method as well as 2 varargs methods.

Two vararg methods with same score(0): "public javax.ws.rs.client.Invocation$Builder org.jboss.resteasy.client.jaxrs.internal.ClientWebTarget.request(javax.ws.rs.core.MediaType[])" and "public javax.ws.rs.client.Invocation$Builder org.jboss.resteasy.client.jaxrs.internal.ClientWebTarget.request(java.lang.String[])" please report!

The OGNL statement being executed is:

someWebTarget.request().get()

Here's the method being called: https://github.com/resteasy/Resteasy/blob/main/resteasy-client/src/main/java/org/jboss/resteasy/client/jaxrs/internal/ClientWebTarget.java#L358

@lukaszlenart
Copy link
Collaborator

Are you sure you call request() with no params? OGNL has a test to confirm calling VarArgs method with null params, also I've added an additional test case and worked as expected.

@petergeneric
Copy link
Contributor Author

petergeneric commented May 2, 2021

I've just double-checked all other OGNL statements that are configured and all of them call request().

I built a unit test myself which appears to reproduce the issue against ognl:ognl:3.2.20 in central:

https://gist.github.com/petergeneric/d898dd50477d690cdedf9672ae945dc7

For me, the first assert passes (OGNL calls the right method), but the second assert that there's no stderr output fails

Hopefully it's not caused by me setting something up incorrectly - apologies in advance if that is the case!

@lukaszlenart
Copy link
Collaborator

Thanks a lot for your example, I was able reproduce this problem locally and what is more interesting it only happens in nested expressions like get().request()

@lukaszlenart
Copy link
Collaborator

lukaszlenart commented May 2, 2021

I found the source of the problem, by introducing #69 something has been broken, using
-Dognl.security.manager=forceDisableOnInit solves the problem.

@lukaszlenart
Copy link
Collaborator

Which version of Java do you use? Upgrading Java to version 8 solves the problem, so maybe it's time to abound Java 1.7 in OGNL and start using Java 8 🤔

@lukaszlenart
Copy link
Collaborator

@harawata what do you think about this ^^?

@petergeneric
Copy link
Contributor Author

I'm using a range of versions (it's an app suport framework), 1.8 LTS, 11 LTS and 16 (16 to be dropped whe 17 LTS comes out), this particular app was running on 16 (as was the unit test)

petergeneric added a commit to petergeneric/stdlib that referenced this issue May 2, 2021
…is a sign that this bug may have been fixed. The bug causes OGNL to incorrectly identify a case of 2 varargs method that also has a no-args option as problematic in a particular condition (see the github issue for more detail).

§Currently the only side-effect of this bug is to print an error message to application stderr in this case, e.g.
```
Two vararg methods with same score(0): "public javax.ws.rs.client.Invocation$Builder org.jboss.resteasy.client.jaxrs.internal.ClientWebTarget.request(javax.ws.rs.core.MediaType[])" and "public javax.ws.rs.client.Invocation$Builder org.jboss.resteasy.client.jaxrs.internal.ClientWebTarget.request(java.lang.String[])" please report!
```
petergeneric added a commit to petergeneric/stdlib that referenced this issue May 2, 2021
…is a sign that this bug may have been fixed. The bug causes OGNL to incorrectly identify a case of 2 varargs method that also has a no-args option as problematic in a particular condition (see the github issue for more detail).

§Currently the only side-effect of this bug is to print an error message to application stderr in this case, e.g.
```
Two vararg methods with same score(0): "public javax.ws.rs.client.Invocation$Builder org.jboss.resteasy.client.jaxrs.internal.ClientWebTarget.request(javax.ws.rs.core.MediaType[])" and "public javax.ws.rs.client.Invocation$Builder org.jboss.resteasy.client.jaxrs.internal.ClientWebTarget.request(java.lang.String[])" please report!
```
@harawata
Copy link
Contributor

harawata commented May 2, 2021

Hi @lukaszlenart @petergeneric !

I just took a quick look and the error does not depend on Java version, but it is not 100% reproducible.
Basically, the test result depends on the order of the methods returned from Class#getDeclaredMethods() which varies on every run.

There are several logical issues.

  1. The method OgnlRuntime#areArgsCompatible() currently returns the same score for the three request methods, but it should give the one with no arguments higher lower score when there is no arguments.
  2. The condition of this line looks incorrect (mm.score > score should be mm.score < score).
  3. To eliminate the order-dependency that I mentioned, reporting this particular error (i.e. "Two vararg methods with same score ...") may have to wait until all methods are evaluated.

[EDIT]
Sorry! Smaller score means better match. Edited 1 and deleted 2.

@lukaszlenart
Copy link
Collaborator

Thanks a lot @harawata for your insights! I will try to improve the code base on your ideas. Yet I would like to know if switching to Java 8 would be a problem for you?

@harawata
Copy link
Contributor

harawata commented May 5, 2021

Hi @lukaszlenart ,
MyBatis 3.5.x requires Java 8, so there is no problem on our side.
Thank you for checking!

@lukaszlenart
Copy link
Collaborator

lukaszlenart commented May 9, 2021

@petergeneric I have pushed OGNL 3.2.21-SNAPSHOT into https://s01.oss.sonatype.org/content/repositories/snapshots/ - could you test it?

@petergeneric
Copy link
Contributor Author

@lukaszlenart looks good to me, thanks (Maven couldn't find 3.2.21-SNAPSHOT on s01.oss or oss., but I found and used 3.2.21-20210509.182701-1 on oss.sonatype which looks like the right artifact from the timestamp).

I ran the test several times against JDK10 and JDK11 and didn't get any errors.

@lukaszlenart
Copy link
Collaborator

@petergeneric great, thanks a lot for your help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants