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

Ensure that file is written on disk for multipart when endpoint expects it #29716

Merged
merged 2 commits into from
Dec 8, 2022

Conversation

geoand
Copy link
Contributor

@geoand geoand commented Dec 7, 2022

With this change, if the JAX-RS endpoint expects a multipart request to
contain a file, then that file is always written to disk regardless
of whether the content-disposition contains the filename attribute
of not.

Fixes: #20938

P.S. The implementation is very clean, but I don't see a better way of passing the necessary information to the runtime code

@geoand
Copy link
Contributor Author

geoand commented Dec 7, 2022

Let's get #29729 in before this.

…ts it

With this change, if the JAX-RS endpoint expects a multipart request to
contain a file, then that file is always written to disk regardless
of whether the content-disposition contains the filename attribute
of not.

Fixes: quarkusio#20938
@geoand
Copy link
Contributor Author

geoand commented Dec 7, 2022

Merge conflict fixed

@geoand geoand added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Dec 7, 2022
@quarkus-bot
Copy link

quarkus-bot bot commented Dec 7, 2022

Failing Jobs - Building ec2a4a4

Status Name Step Failures Logs Raw logs
✔️ JVM Tests - JDK 11
✔️ JVM Tests - JDK 17
JVM Tests - JDK 17 MacOS M1 Set up runner ⚠️ Check → Logs Raw logs
✔️ JVM Tests - JDK 18

@geoand geoand merged commit 25b8a47 into quarkusio:main Dec 8, 2022
@quarkus-bot quarkus-bot bot added this to the 2.16 - main milestone Dec 8, 2022
@quarkus-bot quarkus-bot bot added kind/bugfix and removed triage/waiting-for-ci Ready to merge when CI successfully finishes labels Dec 8, 2022
@geoand geoand deleted the #20938 branch December 8, 2022 05:45
@gsmet gsmet modified the milestones: 2.16 - main, 2.13.6.Final Dec 14, 2022
@gsmet gsmet modified the milestones: 2.13.6.Final, 2.16 - main Dec 15, 2022
@gsmet
Copy link
Member

gsmet commented Dec 15, 2022

@geoand @rsvoboda I wasn't able to backport this one properly. It conflicts with another patch and even if the conflicts are fixed there are some test failures.
If we really want it in 2.13, it either requires a specific backport or we should also backport that other fix. But IIRC, the other "fix" is the significan changes to multipart handling that Stéphane made so I'm not really excited about the idea.

@geoand
Copy link
Contributor Author

geoand commented Dec 15, 2022

I am pretty sure we can make a specific backport PR to 2.13.

@geoand
Copy link
Contributor Author

geoand commented Dec 15, 2022

But to be honest, this one isn't really needed by Keycloak as they use a different approach

@gsmet gsmet modified the milestones: 2.16 - main, 2.15.1.Final Dec 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Resteasy Reactive Multipart fails if fileName is not set for a file part
3 participants