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

RESTEasy Reactive: read ContainerRequestContext getEntityStream in filter resource method payload becomes null #17430

Closed
selvarajchennappan opened this issue May 24, 2021 · 22 comments · Fixed by #17687
Labels
area/resteasy-reactive kind/bug Something isn't working
Milestone

Comments

@selvarajchennappan
Copy link

Describe the bug

Resource method payload becomes null if we do
ContainerRequestContext#getEntityStream in ContainerRequestFilter

Payload : pojo [FileUpload ]

Expected behavior

(Describe the expected behavior clearly and concisely.)
payload shoudn't be null

Actual behavior

(Describe the actual behavior clearly and concisely.)
payload is null

if we remove filter then it works as expected .

quarkus-issue.zip

To Reproduce

Link to a small reproducer (preferably a Maven project if the issue is not Gradle-specific).

Or attach an archive containing the reproducer to the issue.

Steps to reproduce the behavior:

  1. add filter implementation and read getEntityStream
  2. upload file and file is null in resource class
  3. remove filter works
    please note that added @Blocking

Configuration

# Add your application.properties here, if applicable.

Java version : 1.8
Quarkus version : 1.13.4.Final
We have implemented our application using reactive, had forced to mark it as blocking we have use case to process the request entity at filter .However its not working

Screenshots

(If applicable, add screenshots to help explain your problem.)

Environment (please complete the following information):

Output of uname -a or ver

Output of java -version

GraalVM version (if different from Java)

Quarkus version or git rev

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

Additional context

(Add any other context about the problem here.)

@selvarajchennappan selvarajchennappan added the kind/bug Something isn't working label May 24, 2021
@quarkus-bot
Copy link

quarkus-bot bot commented May 24, 2021

/cc @FroMage, @geoand, @stuartwdouglas

@selvarajchennappan
Copy link
Author

could you please update on this issue . it is blocker now

@geoand
Copy link
Contributor

geoand commented May 25, 2021

You essentially need to do what is described in #17280

@geoand geoand closed this as completed May 25, 2021
@geoand geoand added the triage/invalid This doesn't seem right label May 25, 2021
@selvarajchennappan
Copy link
Author

thanks
I hope you had seen the attached code .
changed from implements ContainerRequestFilter to @ServerRequestFilter(priority = 0)

However it is not working .
@Blocking is also there
@ApplicationPath("/")
@Blocking
public class RestBlockingApplication extends Application {
}
Could you please check .

@geoand
Copy link
Contributor

geoand commented May 25, 2021

Please provide a complete working example and steps on how to reproduce the problem

@selvarajchennappan
Copy link
Author

It is not working when content type "multipart/form-data" fileUpload becomes null
we have to log every request inpustream to kafka.

quarkus-fileupload-log-issue.zip

Reproduce:-
upload file with name ["fileUpload"]from postman or any http client .
logs getting printed in filter but resource class its empty
application.properties
quarkus.http.port=8080
quarkus.http.body.delete-uploaded-files-on-end= false --tried with false as well actual is true .
quarkus.http.body.uploads-directory=/temp
quarkus.http.body.preallocate-body-buffer =true
quarkus.http.limits.max-body-size=200M

@selvarajchennappan
Copy link
Author

@geoand could you please update on this issue.

@geoand
Copy link
Contributor

geoand commented May 26, 2021

Please add a fully working example I can check without having to fill anything in myself and I'll give it another look

@selvarajchennappan
Copy link
Author

Added a simple project with working example .pls import it as maven project .
steps to Reproduce :
run DocumentRepositoryResourceTest [@QuarkusTest ]

Actual :
Added sysout in filter and resource class. input file is empty in resource class
successfully read content ctream in filter
File is empty

Expected :
successfully read content ctream in filter
File is not empty

Java version 1.8
Quarkus version : 1.13.4.Final
code-with-quarkus.zip

@geoand
Copy link
Contributor

geoand commented May 26, 2021

Thanks.

Basically you need to do as #17280 describe, i.e. to buffer the stream.
This is because in Java IO, when a stream is read, you can't re-read it.
So you buffer the data you read and then set the entity stream to the buffered stream thus allowing the framework to read the body as well

@selvarajchennappan
Copy link
Author

Thanks for immediate response .
@geoand set the entity stream to the buffered stream however its not working
InputStream is = ctx.getEntityStream();
String body = new BufferedReader(new InputStreamReader(is)).lines().collect(Collectors.joining("\n"));
ctx.setEntityStream(new ByteArrayInputStream(body.getBytes()));

As had mentioned before it works with content type json. I think issue is with Multipart fileupload .
Could you please check

@geoand
Copy link
Contributor

geoand commented May 26, 2021

I will check next week as I am on PTO

@geoand geoand reopened this May 26, 2021
@quarkus-bot quarkus-bot bot removed the triage/invalid This doesn't seem right label May 26, 2021
@geoand
Copy link
Contributor

geoand commented Jun 1, 2021

Unfortunately the way we handle things won't work for this use case.

The issue is that we reading the entire body in a filter is rathe dodgy and can mess up the subsequent processing by Vert.x (and therefore the Multipart handling)...
I think we should have a way to provide some kind of filter that logs the request body (or allows the user to handle it) that will play nice with the rest of the infrastructure, but I don't know what that would look like.

@FroMage @stuartwdouglas any ideas?

@FroMage
Copy link
Member

FroMage commented Jun 3, 2021

Is it because Vert.x handles multi-part by bypassing our entity stream?

@geoand
Copy link
Contributor

geoand commented Jun 3, 2021

Yes, exactly

@FroMage
Copy link
Member

FroMage commented Jun 3, 2021

Can we detect that someone changed it, and then load it differently frmo vert.x?

That won't work for any other custom vert.x-specific handler that bypasses the jax-rs API, but 🤷

@geoand
Copy link
Contributor

geoand commented Jun 3, 2021

I would much like to avoid having to do something like that. I would prefer if we could have some kind of special filter that could let the user handle the entire body without messing up the following handlers for Vert.x.
That said, I don't know what that would look like

@FroMage
Copy link
Member

FroMage commented Jun 3, 2021

Not sure we can do that if one of the handlers needs to stream the body into something: there's never any "fullly loaded" representation of the bytes.

The best we might be able to do is an API that lets users "see" the byte buffers as we pass them to whatever's consuming them. Not sure it'd help in this case.

Sort of like a streaming filter…

@stuartwdouglas
Copy link
Member

I think we need to write our own Multipart handler, and not rely on Vert.x.

stuartwdouglas added a commit to stuartwdouglas/quarkus that referenced this issue Jun 4, 2021
This gives us a lot more flexibility, and allows it to work
with multiple backends.

Fixes quarkusio#17430
@stuartwdouglas
Copy link
Member

With the linked PR the example works, if you correct the code to:

		InputStream is = ctx.getEntityStream();
		byte[] data = is.readAllBytes();
		String body = new String(data, StandardCharsets.UTF_8);
		System.out.println(" successfully read entity stream in filter" + body);
		// push to kafka
		ctx.setEntityStream(new ByteArrayInputStream(data));

@selvarajchennappan
Copy link
Author

Thanks a lot.
Does it work for content type "multipart/form-data" ?

stuartwdouglas added a commit to stuartwdouglas/quarkus that referenced this issue Jun 4, 2021
This gives us a lot more flexibility, and allows it to work
with multiple backends.

Fixes quarkusio#17430
@stuartwdouglas
Copy link
Member

Should work for both.

stuartwdouglas added a commit to stuartwdouglas/quarkus that referenced this issue Jun 8, 2021
This gives us a lot more flexibility, and allows it to work
with multiple backends.

Fixes quarkusio#17430
@quarkus-bot quarkus-bot bot added this to the 2.1 - main milestone Jun 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/resteasy-reactive kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants