Repository: aws/aws-sdk-js
Pull request #3108 (2020-02-20 09:12:58)

Fix host header in endpoint discovery
<!--
Thank you for your pull request. Please provide a description below.
-->
2 bugs are fixed for endpoint discovery:
1. When request's endpoint is updated with the one from endpoint discovery, the `Host` header should be also be updated. Otherwise, `ERR_TLS_CERT_ALTNAME_INVALID` error will be thrown
2. When given operation requires endpoint discovery, SDK will throw if endpoint discovery feature is not enabled

2 unit tests are removed from the suite because they are not needed anymore

##### Checklist
<!-- Remove items that do not apply. For completed items, change [ ] to [x]. -->

- [x] `npm run test` passes
- [x] changelog is added, `npm run add-change`


codecov-io: # [Codecov](https://codecov.io/gh/aws/aws-sdk-js/pull/3108?src=pr&el=h1) Report
> Merging [#3108](https://codecov.io/gh/aws/aws-sdk-js/pull/3108?src=pr&el=desc) into [master](https://codecov.io/gh/aws/aws-sdk-js/commit/70106adc70b148e61e3dbbbcd63086676b6d0a22?src=pr&el=desc) will **increase** coverage by `<.01%`.
> The diff coverage is `88.88%`.

[![Impacted file tree graph](https://codecov.io/gh/aws/aws-sdk-js/pull/3108/graphs/tree.svg?width=650&token=qXdPGBZWAr&height=150&src=pr)](https://codecov.io/gh/aws/aws-sdk-js/pull/3108?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master    #3108      +/-   ##
==========================================
+ Coverage   97.05%   97.05%   +<.01%     
==========================================
  Files         330      330              
  Lines        9534     9540       +6     
  Branches     1751     1753       +2     
==========================================
+ Hits         9253     9259       +6     
  Misses        281      281
```


| [Impacted Files](https://codecov.io/gh/aws/aws-sdk-js/pull/3108?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [lib/http.js](https://codecov.io/gh/aws/aws-sdk-js/pull/3108/diff?src=pr&el=tree#diff-bGliL2h0dHAuanM=) | `100% <100%> (ø)` | :arrow_up: |
| [lib/discover\_endpoint.js](https://codecov.io/gh/aws/aws-sdk-js/pull/3108/diff?src=pr&el=tree#diff-bGliL2Rpc2NvdmVyX2VuZHBvaW50Lmpz) | `97.68% <85.71%> (-0.54%)` | :arrow_down: |
| [lib/dynamodb/converter.js](https://codecov.io/gh/aws/aws-sdk-js/pull/3108/diff?src=pr&el=tree#diff-bGliL2R5bmFtb2RiL2NvbnZlcnRlci5qcw==) | `99.17% <0%> (+0.82%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/aws/aws-sdk-js/pull/3108?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/aws/aws-sdk-js/pull/3108?src=pr&el=footer). Last update [70106ad...a53ccc0](https://codecov.io/gh/aws/aws-sdk-js/pull/3108?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/.changes/next-release/bugfix-EndpointDiscovery-76fcc017.json b/.changes/next-release/bugfix-EndpointDiscovery-76fcc017.json
new file mode 100644
index 000000000..392066312
--- /dev/null
+++ b/.changes/next-release/bugfix-EndpointDiscovery-76fcc017.json
@@ -0,0 +1,5 @@
+{
+  "type": "bugfix",
+  "category": "EndpointDiscovery",
+  "description": "Update host header in requests when new endpoint is found; Throw error when endpoint discovery is required but not enabled"
+}
\ No newline at end of file
diff --git a/lib/discover_endpoint.js b/lib/discover_endpoint.js
index 728390a78..5fe5b48ec 100644
--- a/lib/discover_endpoint.js
+++ b/lib/discover_endpoint.js
@@ -273,7 +273,7 @@ function isFalsy(value) {
  * @param [object] request request object.
  * @api private
  */
-function isEndpointDiscoveryApplicable(request) {
+function isEndpointDiscoveryEnabled(request) {
   var service = request.service || {};
   if (service.config.endpointDiscoveryEnabled === true) return true;
 
@@ -325,13 +325,23 @@ function discoverEndpoint(request, done) {
   var service = request.service || {};
   if (hasCustomEndpoint(service) || request.isPresigned()) return done();
 
-  if (!isEndpointDiscoveryApplicable(request)) return done();
-
-  request.httpRequest.appendToUserAgent('endpoint-discovery');
-
   var operations = service.api.operations || {};
   var operationModel = operations[request.operation];
   var isEndpointDiscoveryRequired = operationModel ? operationModel.endpointDiscoveryRequired : 'NULL';
+  var isEnabled = isEndpointDiscoveryEnabled(request);
+
+  if (!isEnabled) {
+    // Unless endpoint discovery is required, SDK will fallback to normal regional endpoints.
+    if (isEndpointDiscoveryRequired === 'REQUIRED') {
+      throw util.error(new Error(), {
+        code: 'ConfigurationException',
+        message: 'Endpoint Discovery is not enabled but this operation requires it.'
+      });
+    }
+    return done();
+  }
+
+  request.httpRequest.appendToUserAgent('endpoint-discovery');
   switch (isEndpointDiscoveryRequired) {
     case 'OPTIONAL':
       optionalDiscoverEndpoint(request);
diff --git a/lib/http.js b/lib/http.js
index f49a53caa..4ec2c3731 100644
--- a/lib/http.js
+++ b/lib/http.js
@@ -161,6 +161,9 @@ AWS.HttpRequest = inherit({
     var newEndpoint = new AWS.Endpoint(endpointStr);
     this.endpoint = newEndpoint;
     this.path = newEndpoint.path || '/';
+    if (this.headers['Host']) {
+      this.headers['Host'] = newEndpoint.host;
+    }
   }
 });
 


Test patch:
diff --git a/test/discover_endpoint.spec.js b/test/discover_endpoint.spec.js
index 2dca03566..fb7a381a8 100644
--- a/test/discover_endpoint.spec.js
+++ b/test/discover_endpoint.spec.js
@@ -454,6 +454,19 @@ describe('endpoint discovery', function() {
       expect(spy.calls[0].arguments[0].httpRequest.headers['x-amz-api-version']).to.eql('2018-09-19');
     });
 
+    it('update "Host" header with newly discovered endpoint', function() {
+      var client = new AWS.Service({
+        endpointDiscoveryEnabled: true,
+        apiConfig: new AWS.Model.Api(api),
+      });
+      var https = require('https');
+      helpers.spyOn(https, 'request').andCallThrough();
+      var request = client.makeRequest('requiredEDOperation', {Query: 'query', Record: 'record'});
+      helpers.mockHttpResponse(200, {}, '{"Endpoints": [{"Address": "https://cell1.fakeservice.amazonaws.com/fakeregion", "CachePeriodInMinutes": 1}]}');
+      request.send();
+      expect(request.httpRequest.headers.Host).to.eql('cell1.fakeservice.amazonaws.com');
+    });
+
     if (AWS.util.isNode()) {
       describe('not make more endpoint operation requests if there is an in-flight request', function() {
         var port;
@@ -664,27 +677,6 @@ describe('endpoint discovery', function() {
       expect(getFromCacheSpy.calls.length).to.eql(0);
     });
 
-    it('if endpoint specified in global config, custom endpoint should be preferred', function() {
-      AWS.config.update({endpoint: 'custom-endpoint.amazonaws.com/fake-region'});
-      client = new AWS.Service({
-        apiConfig: new AWS.Model.Api(api),
-      });
-      var getFromCacheSpy = helpers.spyOn(AWS.endpointCache, 'get').andCallThrough();
-      client.makeRequest('requiredEDOperation', {Query: 'query', Record: 'record'}).send();
-      expect(getFromCacheSpy.calls.length).to.eql(0);
-      delete AWS.config.endpoint;
-    });
-
-    it('if endpoint specified in global config, custom endpoint should be preferred', function() {
-      AWS.config.update({mock: {endpoint: 'custom-endpoint.amazonaws.com/fake-region'}});
-      var MockService = helpers.MockServiceFromApi(api);
-      var client = new MockService({});
-      var getFromCacheSpy = helpers.spyOn(AWS.endpointCache, 'get').andCallThrough();
-      client.makeRequest('requiredEDOperation', {Query: 'query', Record: 'record'}).send();
-      expect(getFromCacheSpy.calls.length).to.eql(0);
-      delete AWS.config.mock;
-    });
-
     it('append "endpoint-discovery" to user-agent on all requests', function() {
       var client = new AWS.Service({
         endpointDiscoveryEnabled: true,
@@ -733,12 +725,22 @@ describe('endpoint discovery', function() {
       });
       helpers.mockHttpResponse(200, {}, '{"Endpoints": [{"Address": "https://cell1.fakeservice.amazonaws.com/fakeregion", "CachePeriodInMinutes": 1}]}');
       client.makeRequest('optionalEDOperation', {Query: 'query'}).send();
-      client.makeRequest('requiredEDOperation',  {Query: 'query', Record: 'record'}).send();
       expect(spy.calls.length).to.eql(0);
+      // Throw if operation requires endpoint discovery but the feature is disabled.
+      var request = client.makeRequest('requiredEDOperation',  {Query: 'query', Record: 'record'});
+      var error;
+      try {
+        request.send();
+      } catch (e) {
+        error = e;
+      }
+      expect(spy.calls.length).to.eql(0);
+      expect(error).not.to.eql(undefined);
+      expect(error.name).to.eql('ConfigurationException');
+      expect(error.message).to.eql('Endpoint Discovery is not enabled but this operation requires it.');
     });
 
     it('turn on endpoint discovery in client config', function() {
-
       client = new AWS.Service({
         endpointDiscoveryEnabled: true,
         apiConfig: new AWS.Model.Api(api),