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

Normalize Some Of The HTTP Server Parameters Between Backends #4037

Merged

Conversation

isomarcte
Copy link
Member

This commit attempts to normalize some of the common server configuration parameters between backends.

When switching between different backends, one would assume that the operational semantics would be as close as possible, assuming there was no direct configuration change in user code. Prior to this commit there was a substantial delta between these parameters in Blaze/Jetty/Ember.

It is likely that more parameters can be shared in the future, but some configuration options are not exposed for any give backend, and some things which are configurable on one backend may not even be expressible on another.

Thus, this commit attempts to be relatively conservative, synchronizing the more obvious parameters only.

When two backends currently differed on the configuration value for a given parameter, the more permissive value was chosen. Here is a summary of the configuration changes with respect to a given backend.

This commit attempts to normalize some of the common server configuration parameters between backends.

When switching between different backends, one would assume that the operational semantics would be as close as possible, assuming there was no direct configuration change in user code. Prior to this commit there was a substantial delta between these parameters in Blaze/Jetty/Ember.

It is likely that _more_ parameters can be shared in the future, but some configuration options are not exposed for any give backend, and some things which are configurable on one backend may not even be expressible on another.

Thus, this commit attempts to be relatively conservative, synchronizing the more obvious parameters only.

When two backends currently differed on the configuration value for a given parameter, the more _permissive_ value was chosen. Here is a summary of the configuration changes with respect to a given backend.

* Ember
    * Default host is now derived from `org.http4s.server.defaults.Host`, though it should be the same.
    * Default Max header size, 10240B -> 40960B, which was the value used by Blaze
* Jetty
    * Default Max header size, 8192B -> 40960B, which was the value used by Blaze
    * See: https://github.com/eclipse/jetty.project/blob/jetty-9.4.x/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConfiguration.java#L60
Copy link
Member

@rossabaker rossabaker left a comment

Choose a reason for hiding this comment

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

I like it. It's possible different defaults are appropriate for different backends, but I'd rather default to making them similar, and this is a step in the right direction.

@rossabaker rossabaker merged commit 86525d3 into http4s:series/0.21 Dec 21, 2020
@isomarcte isomarcte deleted the normalize-configuration-for-servers branch December 21, 2020 13:21
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.

None yet

3 participants