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

Allow long server names #651

Merged
merged 1 commit into from
May 19, 2023
Merged

Allow long server names #651

merged 1 commit into from
May 19, 2023

Conversation

ciarams87
Copy link
Member

Proposed changes

This commit increases the default server_names_hash_bucket_size to 256 and server_names_hash_max_size to 1024 to support hostnames longer than 47 characters.

Tested locally using the hostname abcdefghijklmn.too.long.server.name.example.org. Verified before the change, we get the error could not build server_names_hash, you should increase server_names_hash_bucket_size: 64, and after the change, there are no errors using this hostname.

Closes #632

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have updated necessary documentation
  • I have rebased my branch onto main
  • I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

@ciarams87 ciarams87 requested a review from a team as a code owner May 18, 2023 13:14
@github-actions github-actions bot added the bug Something isn't working label May 18, 2023
Copy link
Contributor

@kate-osborn kate-osborn left a comment

Choose a reason for hiding this comment

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

LGTM!

This echo command is getting long. We should prioritize the FIXME on line 98. cc @pleshakov

@sjberman
Copy link
Contributor

This echo command is getting long. We should prioritize the FIXME on line 98

I was actually looking at maybe cleaning this up for focus day.

@ciarams87 ciarams87 merged commit 82d32fa into main May 19, 2023
@ciarams87 ciarams87 deleted the bug/server-name-hash-size branch May 19, 2023 10:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Nginx fails to reload due to long server name
3 participants