Skip to content
This repository has been archived by the owner on Jan 7, 2018. It is now read-only.

Commit

Permalink
Merge pull request #9 from NREL/long-hostnames
Browse files Browse the repository at this point in the history
Fix handling of long hostnames
  • Loading branch information
GUI committed Sep 22, 2015
2 parents c42c274 + c44ddf0 commit 22cbec7
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 0 deletions.
25 changes: 25 additions & 0 deletions lib/config_reloader/worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,8 @@ _.extend(Worker.prototype, {
hosts['*'].hostname = '_';
}

var longestHostname = 0;

// Now that we know each of the hosts and each of the URL prefixes those
// hosts are responsible for, we can assemble the regex for matching the
// prefixes on each host which we'll then pass to nginx.
Expand All @@ -271,10 +273,33 @@ _.extend(Worker.prototype, {
hostConfig.api_url_prefixes_matcher = '^(' + hostConfig.api_url_prefixes.join('|') + ')';
delete hostConfig.api_url_prefixes;
}

if(hostConfig.hostname.length > longestHostname) {
longestHostname = hostConfig.hostname.length;
}
});

// Choose the nginx host bucket size based on the longest hostname that's
// in the system. Try to pick among the nginx default values, since I think
// there may be benefits with sticking with these sizes (to align with CPU
// cache sizes): http://nginx.org/en/docs/hash.html Max out at 128. We can
// revisit this max if needs be.
//
// For reasons I don't currently understand, a setting of 128 only supports
// a hostname 110 characters long. So add in a buffer so that we increase
// this setting's length even for shorter hostnames.
var hostnameBucketSize = 32;
var hostnameBucketBuffer = 18;
if(longestHostname >= 32 - hostnameBucketBuffer && longestHostname < 64 - hostnameBucketBuffer) {
hostnameBucketSize = 64;
} else if(longestHostname >= 64 - hostnameBucketBuffer) {
hostnameBucketSize = 128;
}

var templateConfig = _.extend({}, config.getAll(), {
hosts: _.values(hosts),

_server_names_hash_bucket_size: hostnameBucketSize,
});

var newContent = this.nginxFrontendTemplate(templateConfig);
Expand Down
2 changes: 2 additions & 0 deletions templates/etc/nginx/frontend_hosts.conf.hbs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
server_names_hash_bucket_size {{_server_names_hash_bucket_size}};

{{#each hosts}}
server {
listen {{../http_port}}{{#if default}} default_server{{/if}};
Expand Down
9 changes: 9 additions & 0 deletions test/config/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,15 @@ apis:
url_matches:
- frontend_prefix: /wildcard-dot-info/
backend_prefix: /info/
- _id: long-frontend-host
frontend_host: abcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghij
backend_host: abcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghij
servers:
- host: 127.0.0.1
port: 9444
url_matches:
- frontend_prefix: /long-host-info/
backend_prefix: /info/
- _id: wildcard-frontend-host
frontend_host: "*"
backend_host: localhost
Expand Down
19 changes: 19 additions & 0 deletions test/integration/proxying.js
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,25 @@ describe('proxying', function() {
done();
});
});

// I'm not exactly sure why, but when we set server_names_hash_bucket_size
// to 128, it actually only allows hostnames 110 characters long. Perhaps
// something to investigate or better understand, but in the meantime
// documenting current behavior.
it('supports hostname lengths up to 110 characters', function(done) {
var options = _.merge({}, this.options, {
headers: {
'Host': 'abcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghij',
},
});

request.get('http://localhost:9080/long-host-info/', options, function(error, response, body) {
response.statusCode.should.eql(200);
var data = JSON.parse(body);
data.headers.host.length.should.eql(110);
done();
});
});
});

// Ensure basic HTTP requests of all HTTP methods work with the entire stack
Expand Down

0 comments on commit 22cbec7

Please sign in to comment.