Skip to content

Commit

Permalink
Fix host header in endpoint discovery (#3108)
Browse files Browse the repository at this point in the history
* fix host header and config validation in endpoint discovery

* remove broken tests
  • Loading branch information
AllanZhengYP authored Feb 20, 2020
1 parent 70106ad commit dc5b223
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 28 deletions.
5 changes: 5 additions & 0 deletions .changes/next-release/bugfix-EndpointDiscovery-76fcc017.json
Original file line number Diff line number Diff line change
@@ -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"
}
20 changes: 15 additions & 5 deletions lib/discover_endpoint.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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);
Expand Down
3 changes: 3 additions & 0 deletions lib/http.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
});

Expand Down
48 changes: 25 additions & 23 deletions test/discover_endpoint.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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),
Expand Down

0 comments on commit dc5b223

Please sign in to comment.