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

feat(frontend) Allow overriding akka-max-header-value-length #5094

Conversation

karoliskascenas
Copy link
Contributor

Allows overriding max-header-value-length via environment variables. This should enable users to solve the following issue:
akka.actor.ActorSystemImpl - Illegal request, responding with status '431 Request Header Fields Too Large': HTTP header value exceeds the configured limit of 8192 characters

@github-actions
Copy link

github-actions bot commented Jun 6, 2022

Unit Test Results (build & test)

334 tests  ±0   334 ✔️ ±0   2m 46s ⏱️ -32s
  78 suites ±0       0 💤 ±0 
  78 files   ±0       0 ±0 

Results for commit 0a3498e. ± Comparison against base commit 928db39.

♻️ This comment has been updated with latest results.

Copy link
Collaborator

@jjoyce0510 jjoyce0510 left a comment

Choose a reason for hiding this comment

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

Nice! Thank you!

cc @RyanHolstien to take a quick look as well.

@RyanHolstien
Copy link
Collaborator

This setting is already exposed as configurable through the play configuration. Instead of modifying the custom configurations, please just add a variable setting play.server.max-content-length

@RyanHolstien
Copy link
Collaborator

@jjoyce0510
Copy link
Collaborator

Let's try what @RyanHolstien has suggested - if its possible, we can make that change and should be good to merge! If there's any issue, let us know.

@karoliskascenas karoliskascenas force-pushed the feature/override-akka-max-header-value-length branch from 70c7563 to bf982a9 Compare June 6, 2022 19:45
@karoliskascenas
Copy link
Contributor Author

@RyanHolstien was looking at docs for 2.7.6 and it seems like the setting should be play.server.akka.max-header-value-length. Sorry I have close to no experience with neither play or akka. Let me know if this is fine.

@karoliskascenas karoliskascenas force-pushed the feature/override-akka-max-header-value-length branch from bf982a9 to 0a3498e Compare June 6, 2022 19:49
@RyanHolstien
Copy link
Collaborator

@karoliskascenas Yes this looks right! Sorry for putting the wrong variable name in my previous comment.

@jjoyce0510
Copy link
Collaborator

Will merge once this passes CI.

@jjoyce0510 jjoyce0510 merged commit c392142 into datahub-project:master Jun 6, 2022
@karoliskascenas karoliskascenas deleted the feature/override-akka-max-header-value-length branch June 7, 2022 05:13
@TobiasGoerke
Copy link

TobiasGoerke commented Jan 19, 2023

Setting the docker container's env variable DATAHUB_AKKA_MAX_HEADER_VALUE_LENGTH to e.g. 10k seems to have no effect and still results in HTTP header value exceeds the configured limit of 8192 characters.
Am I missing something? Thanks!

Edit: found the answer. Setting max-header-value-length works while max-header-size doesn't. Former is deprecated though and advises to use latter one.

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.

4 participants