Skip to content

Do not use HttpStream.Wrapper in SizeLimitHandler#11051

Merged
sbordet merged 4 commits intojetty-12.0.xfrom
jetty-12.0.x-sizeLimitHandlerFix
Dec 15, 2023
Merged

Do not use HttpStream.Wrapper in SizeLimitHandler#11051
sbordet merged 4 commits intojetty-12.0.xfrom
jetty-12.0.x-sizeLimitHandlerFix

Conversation

@lachlan-roberts
Copy link
Copy Markdown
Contributor

Using an HttpStream.wrapper will bypass the GzipHandler which wraps the request and response.
So we should also wrap the request and response in SizeLimitHandler.

This also fixes a bug where the read/write counts are not reset for each request.

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
@lachlan-roberts lachlan-roberts self-assigned this Dec 13, 2023
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
gregw
gregw previously approved these changes Dec 13, 2023
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
Copy link
Copy Markdown
Contributor

@sbordet sbordet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, but:

  • Please fix the class javadocs, as there are many typos.
  • Please add documentation about this Handler in server-http-handler-use.adoc. You can find sections that talk about other utility handlers Jetty provides (such as GzipHandler, etc.), and you can take one of those sections as example for the SizeLimitHandler. At least add in the docs what's in the class javadocs.

gregw
gregw previously approved these changes Dec 13, 2023
@lachlan-roberts lachlan-roberts added the Sponsored This issue affects a user with a commercial support agreement label Dec 14, 2023
Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
@sbordet
Copy link
Copy Markdown
Contributor

sbordet commented Dec 14, 2023

@lachlan-roberts I have updated documentation and javadocs.

@sbordet sbordet self-requested a review December 14, 2023 16:51
@sbordet sbordet merged commit 35af2d8 into jetty-12.0.x Dec 15, 2023
@sbordet sbordet deleted the jetty-12.0.x-sizeLimitHandlerFix branch December 15, 2023 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Sponsored This issue affects a user with a commercial support agreement

Projects

No open projects
Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

3 participants