RESTEasy Reactive - mapping MULTIPART_FORM_DATA to request body don't work

[michalvavrik]

Describe the bug

with #27526 in place, my multiplart form data request body is always null. My endpoint looks like:

    // resource method
    @POST
    @Path("/echo")
    @Consumes(MediaType.MULTIPART_FORM_DATA)
    @Produces(MediaType.TEXT_PLAIN)
    public String echo(String requestBody) {
        return requestBody;
    }

same situation is for wrapper

    // resource method
    @POST
    @Consumes(MediaType.MULTIPART_FORM_DATA)
    @Produces(MediaType.TEXT_PLAIN)
    @Path("/upload-multipart")
    @Blocking
    public String uploadMultipart(FileWrapper body) {
        deathRow.add(body.file);
        return utils.getSum(body.file.getAbsoluteFile().toPath());
    }
...
    @RegisterForReflection
    public class FileWrapper {
        @RestForm("file")
        @PartType(MediaType.APPLICATION_OCTET_STREAM)
        public File file;
    
        @RestForm("name")
        @PartType(MediaType.TEXT_PLAIN)
        public String name;
    }

Expected behavior

• Provide a way how I can map the request to the String (ideally OOTB).
• Make docs dummy proof (customize docs for me).

Actual behavior

requestBody is always null. body is not null but its fields are null.

How to Reproduce?

Reproducer:

Steps to reproduce the behavior:

1. git clone https://github.com/quarkus-qe/quarkus-test-suite.git
2. cases

• cd quarkus-test-suite/http/jaxrs-reactive && mvn clean verify -Dit.test=MultipartClientIT
• cd quarkus-test-suite/http/rest-client-reactive && mvn clean verify -Dit.test=LargeFileHandlingIT#uploadMultipart

Output of uname -a or ver

No response

Output of java -version

No response

GraalVM version (if different from Java)

No response

Quarkus version or git rev

No response

Build tool (ie. output of mvnw --version or gradlew --version)

No response

Additional information

No response

[quarkus-bot]

/cc @FroMage, @geoand, @stuartwdouglas

[michalvavrik]

@FroMage @geoand updated issue desc. as I found another issue and they are pretty similar

[geoand]

Thanks!

@FroMage can you please take a look at this? It would be great to have this fixed for the first CR if possible

[FroMage]

Yes, this must be related to the multipart change. I'll look.

[FroMage]

OK, so the first good news is that if I add these tests to the RESTEasy Reactive test suite, they pass.

[FroMage]

Perhaps it's a client issue then, because I see that's involved.

[FroMage]

I guess the @QuarkusScenario runs things via a different VM, making this hard to debug :(

[FroMage]

OK, I can reproduce it outside the QE tests. It appears to be a problem with the client indeed, not sending anything.

[FroMage]

Huh. So, that's embarassing… It appears BeanParamParser doesn't support the @Rest* annotations, which means on the client side only the JAX-RS annotations are supported for bean params. How we did not notice that before, I wonder. I'll fix this ASAP.

[michalvavrik]

Thank you

[FroMage]

This is the gift issue that keeps on giving.

So apparently, on the header we auto-hyphenate headers, so @RestHeader String fooBar is equivalent to @RestHeader("foo-Bar") String fooBar, which is almost useful, I guess, though the initial letter should be capitalised to be useful, as specified in the original issue: #13665. But this isn't done on the client, so it's not symmetrical.

And, since we designed @RestHeader (and others) for the server, we allowed it for fields and parameters, but not properties (methods). And the client allows and supports the original @HeaderParam on properties as well, which I can notice in @BeanParam tests. Pretty sure the server doesn't support properties. But the client does. What do you think of that @geoand ? Should I file an issue?

[geoand]

Pretty sure the server doesn't support properties

Not sure I am following what you mean here. Can you please elaborate?

[FroMage]

Well, given that @RestHeader (and the rest) are not allowed on methods, I don't think the server side supports them being defined on @BeanParam or endpoint properties. Now that I think about it, I'm not even sure we support @HeaderParam on those either, even though these are allowed on methods.

[geoand]

I must be missing something entirely as I don't understand what you mean by @RestHeader (and the rest) are not allowed on methods

[FroMage]

You must be as incredulous as I was:

@Retention(RetentionPolicy.RUNTIME)
@Target({ ElementType.FIELD, ElementType.PARAMETER })
public @interface RestHeader {
    String value() default "";
}

Notice the missing ElementType.METHOD ;)

[FroMage]

Amazingly, the spec doesn't really mention if those @*Param annotations should be on the getter or the setter, but it gives one example of them being on the setter. I suppose for the server, this makes sense. Less so for the client. Pffff…

[geoand]

Oh... Now I see...

Yeah, putting them on a method was not something we really envisioned

[FroMage]

But apparently people do that, they put them on setters for entity resources, and bean params. For the server. And on the client, they put them on getters I guess, of bean params. And on multipart classes, which are now treated like bean params.

[geoand]

Darn... I guess we need to formally support it then

[FroMage]

Well, I will start with filing an issue, then we can wait for people to need that. Seems pretty useless IMO.

[geoand]

I agree

[FroMage]

Properties: #28846
And I reopened #13665 for the headers

[geoand]

Great, thanks

[FroMage]

OK, done.

[michalvavrik]

Hello @FroMage , thank you for #28851, are there other steps required to fix this issue?

[michalvavrik]

Ah, still not fixed, I get it.