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

Add docs for maximum values in quarkus.http.limits #46428

Merged
merged 1 commit into from
Feb 24, 2025
Merged

Conversation

mzellho
Copy link
Contributor

@mzellho mzellho commented Feb 21, 2025

This PR adds some small enhancements to the docs in io.quarkus.vertx.http.runtime.ServerLimitsConfig, stating the respective maximum values for

quarkus.http.limits.max-header-size
quarkus.http.limits.max-body-size
quarkus.http.limits.max-chunk-size
quarkus.http.limits.max-form-attribute-size
quarkus.http.limits.max-form-buffered-bytes

io.quarkus.vertx.http.runtime.options.HttpServerOptionsUtils#applyCommonOptions does many things, amongst others it forwards these settings to Vert.x APIs that expect an int or a long, e.g. httpServerOptions.setMaxHeaderSize(httpConfig.limits().maxHeaderSize().asBigInteger().intValueExact());.

Sticking to that call, if quarkus.http.limits.max-header-size is set to 2g, that would lead to an java.lang.ArithmeticException: BigInteger out of int range (quite unlucky, but still: 2g or 2147483648 > Integer.MAX_VALUE or 2147483647).

BTW: I could imagine that this would cause an error elsewhere, but at least these Vert.x APIs would also accept -1 to allow unlimited length, which is not possible to specify with io.quarkus.runtime.configuration.MemorySize as per MEMORY_SIZE_PATTERN. Also, we cannot not-define them due to @WithDefault values in io.quarkus.vertx.http.runtime.ServerLimitsConfig (which I think are great in there).

Copy link

quarkus-bot bot commented Feb 21, 2025

Thanks for your pull request!

Your pull request does not follow our editorial rules. Could you have a look?

  • title should not end up with ellipsis (make sure the title is complete)
  • title should preferably start with an uppercase character (if it makes sense!)
  • title should not start with chore/docs/feat/fix/refactor but be a proper sentence

This message is automatically generated by a bot.

@mzellho mzellho changed the title chore: add docs for maximum values of maxHeaderSize, maxBodySize, max… chore: add docs for maximum values in quarkus.http.limits Feb 21, 2025
@mzellho mzellho changed the title chore: add docs for maximum values in quarkus.http.limits Add docs for maximum values in quarkus.http.limits Feb 21, 2025
@mzellho mzellho force-pushed the main branch 2 times, most recently from 94bb82c to ab091ba Compare February 22, 2025 08:23
…ChunkSize, maxFormAttributeSize, maxFormBufferedBytes
@gsmet
Copy link
Member

gsmet commented Feb 24, 2025

I agree it's a nice addition. I pushed it to CI to check everything is fine.

Copy link

quarkus-bot bot commented Feb 24, 2025

Status for workflow Quarkus Documentation CI

This is the status report for running Quarkus Documentation CI on commit 57401fe.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

Warning

There are other workflow runs running, you probably need to wait for their status before merging.

Copy link

🎊 PR Preview 2ca87c3 has been successfully built and deployed to https://quarkus-pr-main-46428-preview.surge.sh/version/main/guides/

  • Images of blog posts older than 3 months are not available.
  • Newsletters older than 3 months are not available.

This comment has been minimized.

Copy link

quarkus-bot bot commented Feb 24, 2025

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 57401fe.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

@gsmet gsmet merged commit 8b9acdc into quarkusio:main Feb 24, 2025
57 checks passed
@gsmet
Copy link
Member

gsmet commented Feb 24, 2025

And merged, thanks!

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.

2 participants