Skip to content

Bump AWS_EVENT_STREAM_MAX_HEADERS_SIZE #123

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

Merged
merged 1 commit into from
Feb 13, 2025
Merged

Bump AWS_EVENT_STREAM_MAX_HEADERS_SIZE #123

merged 1 commit into from
Feb 13, 2025

Conversation

graebm
Copy link
Contributor

@graebm graebm commented Feb 13, 2025

Issue:

The previous PR increased MAX_MESSAGE_SIZE from 16MiB -> 256MiB because some services were bumping into this limit. This is lower that its actual technical max of INT32_MAX (2GiB+), but it seemed like there should still have some reasonable value.

MAX_HEADERS_SIZE is a similar constant, set lower than its actual technical limit of INT32_MAX (2GiB+), and was not adjusted in the previous PR

Description of changes:

  • Set MAX_HEADERS_SIZE equal to MAX_MESSAGE_SIZE
    • Yes, it's silly for a message to be 100% headers, but 🤷‍♀️
  • Document how many bytes are used to encode these things, so we know which constants can be bumped higher or not.
  • Add image and text to Readme (copied Transcribe docs) the explains the encoding (I always need to look this up myself)
  • Remove AWS_EVENT_STREAM_HEADER_STATIC_VALUE_LEN_MAX, a value that is only used once internally, and whose existence can only serve to confuse users of this library
  • Add AWS_EVENT_STREAM_HEADER_VALUE_LEN_MAX, because that is an actual technical limit a user might be interested in

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Copy link
Contributor

@waahm7 waahm7 left a comment

Choose a reason for hiding this comment

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

:shipit:

@graebm graebm merged commit 52fdfeb into main Feb 13, 2025
35 checks passed
@graebm graebm deleted the bump-headers-limit branch February 13, 2025 23:28
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