From c44ddf0cc5be43f164044c524bd3dd0685dc6361 Mon Sep 17 00:00:00 2001
From: Nick Muerdter <nick.muerdter@nrel.gov>
Date: Tue, 22 Sep 2015 21:28:17 +0300
Subject: [PATCH] Fix handling of long hostnames.

It was possible that nginx would bomb if an API backend was added with a
long hostname. The exact length that would trigger this error varied,
since nginx's default lengths actually varied depending on the CPU type.
But this should fix things so that we should properly support hostnames
up to 110 characters long regardless of CPU defaults.

Fixes https://github.com/NREL/api-umbrella/issues/168
---
 lib/config_reloader/worker.js               | 25 +++++++++++++++++++++
 templates/etc/nginx/frontend_hosts.conf.hbs |  2 ++
 test/config/test.yml                        |  9 ++++++++
 test/integration/proxying.js                | 19 ++++++++++++++++
 4 files changed, 55 insertions(+)

diff --git a/lib/config_reloader/worker.js b/lib/config_reloader/worker.js
index a52457a..ebc1b65 100644
--- a/lib/config_reloader/worker.js
+++ b/lib/config_reloader/worker.js
@@ -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.
@@ -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);
diff --git a/templates/etc/nginx/frontend_hosts.conf.hbs b/templates/etc/nginx/frontend_hosts.conf.hbs
index b27b188..da7dd11 100644
--- a/templates/etc/nginx/frontend_hosts.conf.hbs
+++ b/templates/etc/nginx/frontend_hosts.conf.hbs
@@ -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}};
diff --git a/test/config/test.yml b/test/config/test.yml
index 448f71e..1440efc 100644
--- a/test/config/test.yml
+++ b/test/config/test.yml
@@ -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
diff --git a/test/integration/proxying.js b/test/integration/proxying.js
index c5b5510..bbf4b2d 100644
--- a/test/integration/proxying.js
+++ b/test/integration/proxying.js
@@ -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