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

[ELYWEB-180] Elytron web consumes the InputStream when form parameters are parsed #213

Open
wants to merge 1 commit into
base: 1.x
Choose a base branch
from

Conversation

rmartinc
Copy link

Issue: https://issues.redhat.com/browse/ELYWEB-180

Wrapping the HttpServletRequest to replay the request for the app. Main ideas:

  • Wrapping is the only way of doing this cos undertow avoids using InputStream and parameters for the same request on purpose (both getInputStream and parameters are setting readStarted to true and avoiding each other).
  • The wrapping is only done if the size of the request is below undertow MAX_BUFFERED_REQUEST_SIZE limit to avoid saving big chunks of data.
  • Only query parameters are managed in any other case.
  • Adding a new test and other classes for the change. Project httpmime was added to the pom (test scope) in order to test multi-part requests.
  • The elytron-web, wilfly-core and wildfly TS run OK with the change.

@fjuma Please review this when you have time. The PR is a bit complicated but I reached the conclusion that wrapping is the only option here. If you see other solution just let me know.

@rmartinc
Copy link
Author

Windows build is failing but in the previous sub-project undertow (all the changes in this PR affect to undertow-servlet). Besides I have tested in my Windows 2016 VM and everything works OK. So no idea why it is failing but it seems not related to the changes in the PR.

@rmartinc
Copy link
Author

@fjuma I have rebased the PR now that the windows issue is fixed. Run the tests again please.

@rmartinc
Copy link
Author

rmartinc commented Sep 6, 2022

@fjuma What do you think about this? It was here for a long time...

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.

1 participant