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

getMultiPartFormParameters() always returns EmptyMultivaluedMap after upgrade to Resteasy Reactive #24986

Closed
1 task done
xgp opened this issue Nov 23, 2023 · 8 comments · Fixed by #25093
Closed
1 task done
Labels

Comments

@xgp
Copy link
Contributor

xgp commented Nov 23, 2023

Before reporting an issue

  • I have read and understood the above terms for submitting issues, and I understand that my issue may be closed without action if I do not follow them.

Area

core

Describe the bug

In RealmResourceProvider extensions that use session.getContext().getHttpRequest().getMultiPartFormParameters() to retrieve parts of a multipart/form-data request, the method now always returns an EmptyMultivaluedMap instead of the parts of the request.

Version

23.0.0

Expected behavior

This should return a MultivaluedMap with the parts represented in the request when given a valid multipart/form-data request. This worked properly in <23.

Actual behavior

It always returns an EmptyMultivaluedMap.

How to Reproduce?

Create a RealmResourceProvider extension that returns the following Object for the getResource() method:

public class TestResource {
  @PUT
  @Consumes(MediaType.MULTIPART_FORM_DATA)
  public Response test() {
    HttpRequest req = session.getContext().getHttpRequest();
    log.infof("mediaType %s", req.getHttpHeaders().getMediaType());
    log.infof("contentType %s", req.getHttpHeaders().getHeaderString("content-type"));

    MediaType mediaType = req.getHttpHeaders().getMediaType();
    log.infof("isCompatible %b", MULTIPART_FORM_DATA_TYPE.isCompatible(mediaType));
    log.infof("hasBoundary %b", mediaType.getParameters().containsKey("boundary"));

    MultivaluedMap<String, FormPartValue> formDataMap = session.getContext().getHttpRequest().getMultiPartFormParameters();
    if (formDataMap != null) {
      log.infof("formDataMap %s", formDataMap);
      for (String k : formDataMap.keySet()) {
        log.infof("key %s", k);
      }
    }
    return Response.noContent().build();
  }
}

Make any multipart/form-data request to it (doesn't matter what's in it, as long as it's valid). The resource method outputs:

[INFO] mediaType multipart/form-data;boundary=hkfxPWUJazkCuZXCBiD7pU6vV0WRLpsBsHhVrqJ
[INFO] contentType multipart/form-data; boundary=hkfxPWUJazkCuZXCBiD7pU6vV0WRLpsBsHhVrqJ
[INFO] isCompatible true
[INFO] hasBoundary true
[INFO] formDataMap org.keycloak.quarkus.runtime.integration.jaxrs.EmptyMultivaluedMap@dfc8632

Anything else?

There is a more complete example you can run in an existing extension and see the result in the unit test.

Pull the xgp/23 branch https://github.com/p2-inc/keycloak-themes/tree/xgp/23

Run:
mvn clean install -Dtest=EmailsResourceTest#testGetUpdateTemplateMaster

@xgp xgp added kind/bug Categorizes a PR related to a bug status/triage labels Nov 23, 2023
@rmartinc
Copy link
Contributor

I think the problem is that the method is a PUT. Doing some tests a POST is working for me. And I think the reason for this behavior is in the KeycloakHandlerChainCustomizer. If it's a POST it's forcing the addition of the FormBodyHandler but not in a PUT or any other method.

@xgp I suppose you can force the parsing adding any @FormParam to the method (it can be any name, no matter it's not passed).

@pedroigor FYI. Maybe we need to also add the parser for other methods if the consumes expects the correct media type?

@xgp
Copy link
Contributor Author

xgp commented Nov 27, 2023

@rmartinc Thanks for pointing to where it actually happens, and the workaround. I can confirm that the workaround works.
@pedroigor I think the FormBodyHandler should be added in the case of POST, PUT and PATCH. Let me know if you agree, and I'll PR the change.

@pedroigor
Copy link
Contributor

@rmartinc You are correct. We should handle the body and support multi-part data when handling those missing HTTP methods.

@pedroigor
Copy link
Contributor

FTR, we only had POST because this is the only HTTP method we are using when processing multi-part requests.

@pedroigor
Copy link
Contributor

@xgp Thanks. Please, send the PR if you have time to work on it. Otherwise, let me know.

@rmartinc
Copy link
Contributor

rmartinc commented Nov 28, 2023

@xgp if you are interested you can send a PR, but maybe also check !resourceMethod.isFormParamRequired() in the if condition. We don't need to add the form parser if it is already required.

@xgp
Copy link
Contributor Author

xgp commented Nov 28, 2023

PR #25093

@pedroigor
Copy link
Contributor

@xgp Thanks, again!

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

Successfully merging a pull request may close this issue.

4 participants