Skip to content

Commit

Permalink
[core-http] de-serialize empty wrapped xml element list properly
Browse files Browse the repository at this point in the history
When de-serializing an empty list of xml elements we expect to get an empty array `[]` back. However, currently we get the empty string `''` instead.

The cause of the issue is that when de-serializing wrapped xml list, we were re-using the variable `unwrappedProperty`, and a short-circuit:

```typescript
unwrappedProperty = responseBody[xmlName!]; // line.1
unwrappedProperty = unwrappedProperty && unwrappedProperty[xmlElementName!]; // line.2
```

In a normal case of

```xml
<Cors>
  <CorsRule>...</CorsRule>
  <CorsRule>...</CorsRule>
</Cors>
```

The parsed object (`responseBody`) from xml parser is

```js
{ Cors: { CorsRule: [{...}, {...}] } }
```

After line.1, we have `unwrappedProperty` with value `{ CorsRule: [{...}, {...}] }`. Then after line.2, we have `unwrappedProperty` with the array as its value. So far so good.

However, in the failing case of

```xml
<Cors />
```

The parsed object from xml parser is

```js
{ Cors: '' }
```

After the first line, we have `unwrappedProperty` as the empty string `''`, the short-circuit on line two keeps `unwrappedProperty` as `''`, which fails the condition

```typescript
const isEmptyWrappedList = unwrappedProperty === undefined;
```

therefore, the expected empty array is not assigned.

This change removes the confusing mutable variable and the short-circuit. Now we just pick the empty array if the wrapped property is falsy.

Fixes Azure#11071
  • Loading branch information
jeremymeng committed Sep 18, 2020
1 parent db93f16 commit da579af
Show file tree
Hide file tree
Showing 3 changed files with 285 additions and 13 deletions.
33 changes: 20 additions & 13 deletions sdk/core/core-http/src/serializer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -697,21 +697,28 @@ function deserializeCompositeType(
);
} else {
const propertyName = xmlElementName || xmlName || serializedName;
let unwrappedProperty = responseBody[propertyName!];
if (propertyMapper.xmlIsWrapped) {
unwrappedProperty = responseBody[xmlName!];
unwrappedProperty = unwrappedProperty && unwrappedProperty[xmlElementName!];

const isEmptyWrappedList = unwrappedProperty === undefined;
if (isEmptyWrappedList) {
unwrappedProperty = [];
}
/* a list of <xmlElementName> wrapped by <xmlName>
For the xml example below
<Cors>
<CorsRule>...</CorsRule>
<CorsRule>...</CorsRule>
</Cors>
the responseBody has
{
Cors: {
CorsRule: [{...}, {...}]
}
}
xmlName is "Cors" and xmlElementName is"CorsRule".
*/
const wrapped = responseBody[xmlName!];
const elementList = wrapped ? wrapped[xmlElementName!] : [];
instance[key] = serializer.deserialize(propertyMapper, elementList, propertyObjectName);
} else {
const property = responseBody[propertyName!];
instance[key] = serializer.deserialize(propertyMapper, property, propertyObjectName);
}
instance[key] = serializer.deserialize(
propertyMapper,
unwrappedProperty,
propertyObjectName
);
}
} else {
// deserialize the property if it is present in the provided responseBody instance
Expand Down
40 changes: 40 additions & 0 deletions sdk/core/core-http/test/serializationTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1600,6 +1600,46 @@ describe("msrest", function() {

assert.deepEqual(result, {});
});

it("should be deserialized properly when item list wrapper is an empty string", function() {
const blobServiceProperties: msRest.CompositeMapper = {
xmlName: "StorageServiceProperties",
serializedName: "BlobServiceProperties",
type: {
name: "Composite",
className: "BlobServiceProperties",
modelProperties: {
cors: {
xmlIsWrapped: true,
xmlName: "Cors",
xmlElementName: "CorsRule",
serializedName: "Cors",
type: {
name: "Sequence",
element: {
type: {
name: "Composite",
className: "CorsRule"
}
}
}
}
}
}
};

const mappers = {
BlobServiceProperties: blobServiceProperties
};
const serializer = new msRest.Serializer(mappers, true);
const result: any = serializer.deserialize(
blobServiceProperties,
{ Cors: "" },
"mockedBlobServiceProperties"
);

assert.deepEqual(result, { cors: [] });
});
});

describe("polymorphic composite type array", () => {
Expand Down
225 changes: 225 additions & 0 deletions sdk/core/core-http/test/serviceClientTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,231 @@ describe("ServiceClient", function() {
assert.deepStrictEqual(res.slice(), [1, 2, 3]);
});

it("should deserialize xml response bodies with empty list wrapper", async function() {
let request: WebResource;
const httpClient: HttpClient = {
sendRequest: (req) => {
request = req;
return Promise.resolve({
request,
status: 200,
headers: new HttpHeaders({
"content-type": "application/xml"
}),
bodyAsText:
'<?xml version="1.0" encoding="utf-8"?><StorageServiceProperties><Cors /></StorageServiceProperties>'
});
}
};

const client1 = new ServiceClient(undefined, {
httpClient,
requestPolicyFactories: [deserializationPolicy()]
});
const blobServiceProperties: CompositeMapper = {
xmlName: "StorageServiceProperties",
serializedName: "BlobServiceProperties",
type: {
name: "Composite",
className: "BlobServiceProperties",
modelProperties: {
cors: {
xmlIsWrapped: true,
xmlName: "Cors",
xmlElementName: "CorsRule",
serializedName: "Cors",
type: {
name: "Sequence",
element: {
type: {
name: "Composite",
className: "CorsRule"
}
}
}
}
}
}
};
const res = await client1.sendOperationRequest(
{},
{
serializer: new Serializer(undefined, true),
httpMethod: "GET",
baseUrl: "httpbin.org",
responses: {
200: {
bodyMapper: blobServiceProperties
}
}
}
);

assert.strictEqual(res._response.status, 200);
assert.deepStrictEqual(res.cors, []);
});

it("should deserialize xml response bodies with wrapper around empty list", async function() {
let request: WebResource;
const httpClient: HttpClient = {
sendRequest: (req) => {
request = req;
return Promise.resolve({
request,
status: 200,
headers: new HttpHeaders({
"content-type": "application/xml"
}),
bodyAsText:
'<?xml version="1.0" encoding="utf-8"?><StorageServiceProperties><Cors></Cors></StorageServiceProperties>'
});
}
};

const client1 = new ServiceClient(undefined, {
httpClient,
requestPolicyFactories: [deserializationPolicy()]
});
const blobServiceProperties: CompositeMapper = {
xmlName: "StorageServiceProperties",
serializedName: "BlobServiceProperties",
type: {
name: "Composite",
className: "BlobServiceProperties",
modelProperties: {
cors: {
xmlIsWrapped: true,
xmlName: "Cors",
xmlElementName: "CorsRule",
serializedName: "Cors",
type: {
name: "Sequence",
element: {
type: {
name: "Composite",
className: "CorsRule"
}
}
}
}
}
}
};
const res = await client1.sendOperationRequest(
{},
{
serializer: new Serializer(undefined, true),
httpMethod: "GET",
baseUrl: "httpbin.org",
responses: {
200: {
bodyMapper: blobServiceProperties
}
}
}
);

assert.strictEqual(res._response.status, 200);
assert.deepStrictEqual(res.cors, []);
});

it("should deserialize xml response bodies with wrapper around non-empty list", async function() {
let request: WebResource;
const httpClient: HttpClient = {
sendRequest: (req) => {
request = req;
return Promise.resolve({
request,
status: 200,
headers: new HttpHeaders({
"content-type": "application/xml"
}),
bodyAsText: `<?xml version="1.0" encoding="utf-8"?>
<StorageServiceProperties>
<Cors>
<CorsRule>
<AllowedOrigins>example1.com</AllowedOrigins>
</CorsRule>
<CorsRule>
<AllowedOrigins>example2.com</AllowedOrigins>
</CorsRule>
</Cors>
</StorageServiceProperties>`
});
}
};

const client1 = new ServiceClient(undefined, {
httpClient,
requestPolicyFactories: [deserializationPolicy()]
});
const corsRule: CompositeMapper = {
serializedName: "CorsRule",
type: {
name: "Composite",
className: "CorsRule",
modelProperties: {
allowedOrigins: {
xmlName: "AllowedOrigins",
required: true,
serializedName: "AllowedOrigins",
type: {
name: "String"
}
}
}
}
};
const blobServiceProperties: CompositeMapper = {
xmlName: "StorageServiceProperties",
serializedName: "BlobServiceProperties",
type: {
name: "Composite",
className: "BlobServiceProperties",
modelProperties: {
cors: {
xmlIsWrapped: true,
xmlName: "Cors",
xmlElementName: "CorsRule",
serializedName: "Cors",
type: {
name: "Sequence",
element: {
type: {
name: "Composite",
className: "CorsRule"
}
}
}
}
}
}
};
const mappers = {
CorsRule: corsRule,
BlobServiceProperties: blobServiceProperties
};
const res = await client1.sendOperationRequest(
{},
{
serializer: new Serializer(mappers, true),
httpMethod: "GET",
baseUrl: "httpbin.org",
responses: {
200: {
bodyMapper: blobServiceProperties
}
}
}
);

assert.strictEqual(res._response.status, 200);
assert.deepStrictEqual(res.cors, [
{ allowedOrigins: "example1.com" },
{ allowedOrigins: "example2.com" }
]);
});

it("should use userAgent header name value from options", async function() {
const httpClient: HttpClient = {
sendRequest: (request: WebResource) => {
Expand Down

0 comments on commit da579af

Please sign in to comment.