Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The CorsRule type is of type CorsRule array, but when getProperties is called on a blob service client with no Cors rules it returns an empty string instead of an empty array. #11071

Closed
6 tasks
avenmia opened this issue Sep 8, 2020 · 2 comments · Fixed by #11344
Assignees
Labels
Azure.Core bug This issue requires a change to an existing behavior in the product in order to be resolved. Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. Storage Storage Service (Queues, Blobs, Files)

Comments

@avenmia
Copy link

avenmia commented Sep 8, 2020

  • Package Name: @azure/storage-blob
  • Package Version: "^12.2.0-preview.1"
  • Operating system: Windows
  • nodejs
    • version: v12.18.3
  • browser
    • name/version: chrome/85.0.4183.83
  • typescript
    • version: "typescript": "^3.9.7"
  • Is the bug related to documentation in

Describe the bug
A clear and concise description of what the bug is.

When calling getProperties() on a BlobServiceClient it returned a Cors property with the value: '' (empty string). When I modify the a different property (deleteRetentionPolicy) and try to call setProperties I get an error stating that cors needs to be of type array.

"{"message":"blobServiceProperties.Cors must be of type Array.","stack":"Error: blobServiceProperties.Cors must be of type Array.\n at serializeSequenceType "

To Reproduce
Steps to reproduce the behavior:

  1. Have a BlobServiceClient with no Cors settings
  2. Call BlobServiceClient.fromConnectionString to retrieve the blob service client
  3. Call getProperties() on that service client to retrieve the properties
  4. Modify a different property such as deleteRetentionPolicy
  5. call setProperties with the modified service properties
  6. Receive error on how cors needs to be of type array
const storageConnectionString = config.storageConnectionString;

    // Create the BlobServiceClient object which will be used to create a container client
    const blobServiceClient = BlobServiceClient.fromConnectionString(storageConnectionString);
    
    if (config.enableSoftDeletes)
    {
        const blobServiceProperties = (await blobServiceClient.getProperties())._response.parsedBody;
        blobServiceProperties.deleteRetentionPolicy =  { enabled: true, days: 10 };
        await blobServiceClient.setProperties(blobServiceProperties);    
    }

Expected behavior
A clear and concise description of what you expected to happen.

When calling getProperties on a blob service client with no Cors settings it should return an empty array in the Cors property. This will allow setProperties to be called without issues.

Screenshots
If applicable, add screenshots to help explain your problem.

After calling getProperties()
image

Additional context
Add any other context about the problem here.

Here is my current workaround:

 const blobServiceProperties: BlobServiceProperties = (await blobServiceClient.getProperties())._response.parsedBody;
 blobServiceProperties.cors = blobServiceProperties.cors || [];

Azure/azure-storage-node#655 (comment)

@jiacfan

@ghost ghost added needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. customer-reported Issues that are reported by GitHub users external to the Azure organization. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that labels Sep 8, 2020
@ramya-rao-a ramya-rao-a added Client This issue points to a problem in the data-plane of the library. Storage Storage Service (Queues, Blobs, Files) labels Sep 8, 2020
@ghost ghost removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Sep 8, 2020
@ghost ghost added the needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team label Sep 8, 2020
@ljian3377 ljian3377 removed their assignment Sep 9, 2020
@jiacfan jiacfan self-assigned this Sep 9, 2020
@jiacfan
Copy link
Member

jiacfan commented Sep 9, 2020

Thanks @avenmia, I'm following up this issue with colleagues working on auto generated deserialization.

@jiacfan jiacfan added the bug This issue requires a change to an existing behavior in the product in order to be resolved. label Sep 9, 2020
@ljian3377 ljian3377 added this to the [2020] October milestone Sep 9, 2020
@jeremymeng jeremymeng self-assigned this Sep 9, 2020
@jeremymeng
Copy link
Member

Looks like a deserialization issue in core-http. Will investigate further and work on a fix.

jeremymeng added a commit to jeremymeng/azure-sdk-for-js that referenced this issue Sep 18, 2020
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
@ramya-rao-a ramya-rao-a removed question The issue doesn't require a change to the product in order to be resolved. Most issues start as that needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team labels Sep 21, 2020
jeremymeng added a commit that referenced this issue Sep 24, 2020
)

* [core-http] de-serialize empty wrapped xml element list properly

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 #11071

* Address CR feedback

replace ternary with ?. and ??

* Port fix and test to core-client

* Move new tests from serviceClientTests to deserializationPolicyTests
deyaaeldeen pushed a commit to deyaaeldeen/azure-sdk-for-js that referenced this issue Sep 25, 2020
…re#11344)

* [core-http] de-serialize empty wrapped xml element list properly

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

* Address CR feedback

replace ternary with ?. and ??

* Port fix and test to core-client

* Move new tests from serviceClientTests to deserializationPolicyTests
@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Azure.Core bug This issue requires a change to an existing behavior in the product in order to be resolved. Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. Storage Storage Service (Queues, Blobs, Files)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants