Skip to content

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

Closed
rmartinc wants to merge 1 commit intowildfly-security:1.xfrom
rmartinc:ELYWEB-180
Closed

[ELYWEB-180] Elytron web consumes the InputStream when form parameters are parsed#213
rmartinc wants to merge 1 commit intowildfly-security:1.xfrom
rmartinc:ELYWEB-180

Conversation

@rmartinc
Copy link
Copy Markdown

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
Copy Markdown
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
Copy Markdown
Author

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

@rmartinc
Copy link
Copy Markdown
Author

rmartinc commented Sep 6, 2022

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

@darranl
Copy link
Copy Markdown
Contributor

darranl commented Sep 19, 2025

Superceded by #297

@darranl darranl closed this Sep 19, 2025
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.

2 participants