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

CXF-7860: Ensure @FormParam parms are consistent with Form entities #453

Merged
merged 1 commit into from
Oct 9, 2018

Conversation

andymc12
Copy link
Contributor

@andymc12 andymc12 commented Oct 2, 2018

This addresses an issue surfaced by the Jakarta JAX-RS API community in Issue 659 where a parameter marked as @FormParam may be injected with a value that is inconsistent with an entity Form parameter. The latter will have the latest contents of the HTTP form body stream whereas the former will not - modifications made to the HTTP form body during filters or interceptors will be lost, thus it is possible that the following code would fail:

@POST
public Response processForm(@FormParam("value") String value, Form form) {
    assertEquals(value, form.asMap().getFirst("value"));
}

This change ensures that the @FormParam parameters will match the values in the Form parameter. There is probably an optimization that I am missing that would only perform this logic if the input stream were modified - suggestions welcome on how I could best do this. Thanks!

@andymc12 andymc12 self-assigned this Oct 2, 2018
@reta
Copy link
Member

reta commented Oct 6, 2018

@andymc12 Thanks a lot for addressing this very edgy case. I was looking how the current implementation works and basically I was trying to understand if we could defer the evaluation of the HTTP parameters (like @FormParam) to a later phase. In general, I think it would be feasible but would significantly impact the existing design.

However, I think I found a simple trick to make it work: reordering parameters initialization. Here is an interesting exercise:

  • Response processForm(@FormParam("value") String value, Form form) -> does not work
  • Response processForm(Form form, @FormParam("value") String value) -> works just fine

So the idea in the nutshell is to a modify JAXRSUtils::processParameters to initialize parameters in a different order. In this case, the Form should be initialized before any @FormParam, it triggers the reader interceptors and the right values are propagated in Form and @FromParam arguments. It should also work in case of constructors or class members.

What do you think?
Thanks!

@andymc12 andymc12 changed the title CXF-7860: Reprocess @FormParam parms so they contain the latest data CXF-7860: Ensure @FormParam parms are consistent with Form entities Oct 8, 2018
@andymc12
Copy link
Contributor Author

andymc12 commented Oct 8, 2018

@reta Thanks so much! That is a much better solution that what I had originally proposed. I had also considered that the "right approach" would be to refactor the phase when parameters were processed, but I think this solution is cleaner - and should not disrupt normal cases.

I've left the test cases unaltered, but I removed my changes from JAXRSInvoker and instead changed JAXRSUtils as you suggested. This passes all of the unit tests and the systests (including my new one). Can you take a look at the latest changes?

Thanks again!

@reta
Copy link
Member

reta commented Oct 8, 2018

Looks good, thanks @andymc12 !

@andymc12 andymc12 merged commit 0c88b66 into apache:3.2.x-fixes Oct 9, 2018
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 this pull request may close these issues.

2 participants