Skip to content

Commit

Permalink
Fix/stop stringing payload blobs (#1457)
Browse files Browse the repository at this point in the history
* Update rest-json and rest-xml to only cast payloads to strings when they are actually strings

* Add customizations to AWS.APIGateway, AWS.IotData, and AWS.Lambda to continue converting JSON payloads modeled as blobs into strings

* Move payload-to-string conversion function to AWS.util

* Add changelog entry
  • Loading branch information
jeskew committed Apr 12, 2017
1 parent 0c974a7 commit 0c1925f
Show file tree
Hide file tree
Showing 17 changed files with 163 additions and 32 deletions.
5 changes: 5 additions & 0 deletions .changes/next-release/bugfix-Parser-d7e3b2f2.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"type": "bugfix",
"category": "Parser",
"description": "Makes casting payload blobs to strings an exceptional behavior rather than the default"
}
1 change: 1 addition & 0 deletions clients/lambda.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ var apiLoader = require('../lib/api_loader');

apiLoader.services['lambda'] = {};
AWS.Lambda = Service.defineService('lambda', ['2014-11-11', '2015-03-31']);
require('../lib/services/lambda');
Object.defineProperty(apiLoader.services['lambda'], '2014-11-11', {
get: function get() {
var model = require('../apis/lambda-2014-11-11.min.json');
Expand Down
7 changes: 6 additions & 1 deletion lib/model/shape.js
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,12 @@ function StringShape() {
this.toType = function(value) {
value = this.api && nullLessProtocols.indexOf(this.api.protocol) > -1 ?
value || '' : value;
return this.isJsonValue ? JSON.parse(value) : value;
if (this.isJsonValue) {
return JSON.parse(value);
}

return value && typeof value.toString === 'function' ?
value.toString() : value;
};

this.toWireFormat = function(value) {
Expand Down
8 changes: 4 additions & 4 deletions lib/protocol/rest_json.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,13 @@ function extractData(resp) {
if (rules.payload) {
var payloadMember = rules.members[rules.payload];
var body = resp.httpResponse.body;
if (payloadMember.isStreaming) {
resp.data[rules.payload] = body;
} else if (payloadMember.type === 'structure' || payloadMember.type === 'list') {
if (payloadMember.type === 'structure' || payloadMember.type === 'list') {
var parser = new JsonParser();
resp.data[rules.payload] = parser.parse(body, payloadMember);
} else if (payloadMember.type === 'binary' || payloadMember.isStreaming) {
resp.data[rules.payload] = body;
} else {
resp.data[rules.payload] = body.toString();
resp.data[rules.payload] = payloadMember.toType(body);
}
} else {
var data = resp.data;
Expand Down
8 changes: 4 additions & 4 deletions lib/protocol/rest_xml.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,13 +74,13 @@ function extractData(resp) {
var payload = output.payload;
if (payload) {
var payloadMember = output.members[payload];
if (payloadMember.isStreaming) {
resp.data[payload] = body;
} else if (payloadMember.type === 'structure') {
if (payloadMember.type === 'structure') {
parser = new AWS.XML.Parser();
resp.data[payload] = parser.parse(body.toString(), payloadMember);
} else if (payloadMember.type === 'binary' || payloadMember.isStreaming) {
resp.data[payload] = body;
} else {
resp.data[payload] = body.toString();
resp.data[payload] = payloadMember.toType(body);
}
} else if (body.length > 0) {
parser = new AWS.XML.Parser();
Expand Down
17 changes: 5 additions & 12 deletions lib/services/apigateway.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,11 @@ AWS.util.update(AWS.APIGateway.prototype, {
*/
setupRequestListeners: function setupRequestListeners(request) {
request.addListener('build', this.setAcceptHeader);
if (request.operation === 'getSdk') {
request.addListener('extractData', this.useRawPayload);
}
},

useRawPayload: function useRawPayload(resp) {
var req = resp.request;
var operation = req.operation;
var rules = req.service.api.operations[operation].output || {};
if (rules.payload) {
var body = resp.httpResponse.body;
resp.data[rules.payload] = body;
if (request.operation === 'getExport') {
var params = request.params || {};
if (params.exportType === 'swagger') {
request.addListener('extractData', AWS.util.convertPayloadToString);
}
}
}
});
Expand Down
14 changes: 13 additions & 1 deletion lib/services/iotdata.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,14 @@
var AWS = require('../core');

/**
* @api private
*/
var blobPayloadOutputOps = [
'deleteThingShadow',
'getThingShadow',
'updateThingShadow'
];

/**
* Constructs a service interface object. Each API operation is exposed as a
* function on service.
Expand Down Expand Up @@ -71,7 +80,10 @@ AWS.util.update(AWS.IotData.prototype, {
* @api private
*/
setupRequestListeners: function setupRequestListeners(request) {
request.addListener('validateResponse', this.validateResponseBody)
request.addListener('validateResponse', this.validateResponseBody);
if (blobPayloadOutputOps.indexOf(request.operation) > -1) {
request.addListener('extractData', AWS.util.convertPayloadToString);
}
},

/**
Expand Down
13 changes: 13 additions & 0 deletions lib/services/lambda.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
var AWS = require('../core');

AWS.util.update(AWS.Lambda.prototype, {
/**
* @api private
*/
setupRequestListeners: function setupRequestListeners(request) {
if (request.operation === 'invoke') {
request.addListener('extractData', AWS.util.convertPayloadToString);
}
}
});

12 changes: 12 additions & 0 deletions lib/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -880,6 +880,18 @@ var util = {
v4: function uuidV4() {
return require('uuid').v4();
}
},

/**
* @api private
*/
convertPayloadToString: function convertPayloadToString(resp) {
var req = resp.request;
var operation = req.operation;
var rules = req.service.api.operations[operation].output || {};
if (rules.payload && resp.data[rules.payload]) {
resp.data[rules.payload] = resp.data[rules.payload].toString();
}
}

};
Expand Down
2 changes: 1 addition & 1 deletion test/fixtures/protocol/output/json.json
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@
}
},
"MapType": {
"type": "string",
"type": "map",
"key": { "shape": "StringType" },
"value": { "shape": "StringType" }
}
Expand Down
5 changes: 4 additions & 1 deletion test/json/parser.spec.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@ describe 'AWS.JSON.Parser', ->
describe 'parse', ->

it 'returns an empty document when there are no params', ->
expect(parse({}, '{}')).to.eql({})
rules =
type: 'structure'
members: {}
expect(parse(rules, '{}')).to.eql({})

describe 'structures', ->
rules =
Expand Down
9 changes: 7 additions & 2 deletions test/protocol/rest_json.spec.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,11 @@ describe 'AWS.Protocol.RestJson', ->
svc.extractData(response)

it 'JSON parses http response bodies', ->
defop output:
type: 'structure'
members:
a: type: 'integer'
b: type: 'string'
extractData '{"a":1, "b":"xyz"}'
expect(response.error).to.equal(null)
expect(response.data).to.eql({a:1, b:'xyz'})
Expand All @@ -245,7 +250,7 @@ describe 'AWS.Protocol.RestJson', ->
type: 'structure'
payload: 'Body'
members:
Body: location: 'body', type: 'binary'
Body: location: 'body', type: 'string'

extractData 'foobar'
expect(response.error).to.equal(null)
Expand Down Expand Up @@ -294,4 +299,4 @@ describe 'AWS.Protocol.RestJson', ->
extractData '{"i": 1, "bin": null}'
expect(response.error).to.equal(null)
expect(response.data.i).to.equal(1)
expect(response.data.bin).to.equal(null)
expect(response.data.bin).to.equal(null)
28 changes: 26 additions & 2 deletions test/services/apigateway.spec.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,35 @@ describe 'AWS.APIGateway', ->
expect(req.headers['Accept']).to.equal('application/json')
req = build('updateApiKey', apiKey: 'apiKey')
expect(req.headers['Accept']).to.equal('application/json')


describe 'response parsing', ->
describe 'getSdk', ->
it 'returns the raw payload as the body', (done) ->
body = new Buffer('∂ƒ©∆')
helpers.mockHttpResponse 200, {}, body
apigateway.getSdk (err, data) ->
expect(data.body).to.eql(body)
done()
done()

describe 'getExport', ->
it 'converts the body to a string when the exportType is "swagger"', (done) ->
swaggerDoc = JSON.stringify({swagger: "2.0", host: "foo.amazonaws.com"})
body = new Buffer(swaggerDoc)
helpers.mockHttpResponse 200, {}, body
apigateway.getExport {exportType: 'swagger'}, (err, data) ->
expect(Buffer.isBuffer(data.body)).to.be.false
expect(data.body).to.eql(swaggerDoc)
done()

it 'returns the raw payload when the exportType is not "swagger"', (done) ->
swaggerDoc = JSON.stringify({notSwagger: "2.0", host: "foo.amazonaws.com"})
body = new Buffer(swaggerDoc)
helpers.mockHttpResponse 200, {}, body
apigateway.getExport {exportType: 'notSwagger'}, (err, data) ->
expect(Buffer.isBuffer(data.body)).to.be.true
if typeof body.equals == 'function'
expect(body.equals(data.body)).to.be.true
else
expect(body.toString()).to.equal(data.body.toString())
expect(data.body.toString()).to.eql(swaggerDoc)
done()
29 changes: 29 additions & 0 deletions test/services/iotdata.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
var AWS = require('../../index');
var helpers = require('../helpers');
var Buffer = AWS.util.Buffer;

describe('AWS.IotData', function() {
var blobPayloadOutputOps = [
'deleteThingShadow',
'getThingShadow',
'updateThingShadow'
];

for (var i = 0; i < blobPayloadOutputOps.length; i++) {
describe(blobPayloadOutputOps[i], (function(operation) {
return function() {
it('converts the body to a string', function(done) {
var client = new AWS.IotData({endpoint: 'localhost'});
var shadow = JSON.stringify({foo: 'bar', fizz: ['buzz', 'pop']});
var body = new Buffer(shadow);
helpers.mockHttpResponse(200, {}, body);
client[operation]({thingName: 'thing'}, function(err, data) {
expect(Buffer.isBuffer(data.payload)).to.be.false;
expect(data.payload).to.eql(shadow);
done();
});
});
};
})(blobPayloadOutputOps[i]));
}
});
19 changes: 19 additions & 0 deletions test/services/lambda.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
var AWS = require('../../index');
var helpers = require('../helpers');
var Buffer = AWS.util.Buffer;

describe('AWS.IotData', function() {
describe('invoke', function() {
it('converts the body to a string', function(done) {
var client = new AWS.Lambda();
var payload = JSON.stringify({foo: 'bar', fizz: ['buzz', 'pop']});
var body = new Buffer(payload);
helpers.mockHttpResponse(200, {}, body);
client.invoke({FunctionName: 'function'}, function(err, data) {
expect(Buffer.isBuffer(data.Payload)).to.be.false;
expect(data.Payload).to.eql(payload);
done();
});
});
});
});
10 changes: 10 additions & 0 deletions test/services/s3.spec.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -1620,3 +1620,13 @@ describe 'AWS.S3', ->
expect(data.fields[key]).to.equal(conditions[key])
)
done()

describe 'getBucketPolicy', ->
it 'converts the body to a string', (done) ->
policy = JSON.stringify({key: 'value', foo: 'bar', fizz: 1})
body = new Buffer(policy)
helpers.mockHttpResponse 200, {}, body
s3.getBucketPolicy (err, data) ->
expect(Buffer.isBuffer(data.Policy)).to.be.false;
expect(data.Policy).to.eql(policy)
done()
8 changes: 4 additions & 4 deletions test/util.spec.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -566,10 +566,10 @@ describe 'AWS.util.base64', ->
catch e
err = e
expect(err.message).to.equal('Cannot base64 encode number 3.14')

it 'does not encode null', ->
expect(base64.encode(null)).to.eql(null)

it 'does not encode undefined', ->
expect(base64.encode(undefined)).to.eql(undefined)

Expand All @@ -592,7 +592,7 @@ describe 'AWS.util.base64', ->

it 'does not decode null', ->
expect(base64.decode(null)).to.eql(null)

it 'does not decode undefined', ->
expect(base64.decode(undefined)).to.eql(undefined)

Expand Down Expand Up @@ -639,7 +639,7 @@ describe 'AWS.util.hoistPayloadMember', ->
'payload': 'Stream'
'members': 'Stream': 'shape': 'BlobStream'
'BlobStream':
'type': 'blob'
'type': 'binary'
'streaming': true
httpResp =
'status_code': 200
Expand Down

0 comments on commit 0c1925f

Please sign in to comment.