[BUG] Blob Storage: GetBlockList should throw if conditions other than LeaseId is specified

[felipepessoto]

Describe the bug
If you specify IfMatch(ETag), this method silently ignores it.

azure-sdk-for-net/sdk/storage/Azure.Storage.Blobs/src/BlockBlobClient.cs

Line 2054 in 2c0cc4e

response = await BlockBlobRestClient.GetBlockListAsync(

Expected behavior
Throw exception, saying the ETag or any other condition doesn't work for this API.

Actual behavior (include Exception or Stack Trace)
Returns the list of blocks. Making me believe I don't have a concurrency problem.

BTW, any reason why Get Block List doesn't support ETag, forcing me to do it manually at client side?
https://docs.microsoft.com/en-us/rest/api/storageservices/get-block-list#request

[ghost]

Thanks for the feedback! We are routing this to the appropriate team for follow-up. cc @xgithubtriage.

Issue Details

---

Describe the bug
If you specify IfMatch(ETag), this method silently ignores it.

azure-sdk-for-net/sdk/storage/Azure.Storage.Blobs/src/BlockBlobClient.cs

Line 2054 in 2c0cc4e

response = await BlockBlobRestClient.GetBlockListAsync(

Expected behavior
Throw exception, saying the ETag or any other condition doesn't work for this API.

Actual behavior (include Exception or Stack Trace)
Returns the list of blocks. Making me believe I don't have a concurrency problem.

BTW, any reason why Get Block List doesn't support ETag, forcing me to do it manually at client side?
https://docs.microsoft.com/en-us/rest/api/storageservices/get-block-list#request

Author:	felipepessoto
Assignees:	-
Labels:	Client, Service Attention, Storage, needs-team-attention
Milestone:	-

[amnguye]

@seanmcc-msft @kasobol-msft Should we be throwing exceptions when the user is specifying headers in the BlobRequestConditions that don't apply to the certain API, or should we put that burden on the user to know.

Also should we look to making another Options bag specifically for APIs like this that don't take in certain headers like the rest do? That way the our APIs and options bags reflect what the service supports.

What are you trying to achieve with using this header with GetBlockList ?

[kasobol-msft]

there are three ways user can learn that they're doing something wrong:

1. compile time error (fastest feedback, easy to discover)
2. validation checks (feedback deferred to runtime, still easy to discover, but might be annoying)
3. documentation (hard to discover, feedback potentially after hours of debugging, very annoying).

I think 1. is not viable at this point as it will end up being explosion of models in the sdk, which leaves 2. But I guess it's not only that API.

[seanmcc-msft]

@amnguye, I would prefer not to do this, it would add additional complexity that we need to weigh the tradeoffs for. Lets sync offline and get back to this thread.

[felipepessoto]

@seanmcc-msft @kasobol-msft Should we be throwing exceptions when the user is specifying headers in the BlobRequestConditions that don't apply to the certain API, or should we put that burden on the user to know.

Also should we look to making another Options bag specifically for APIs like this that don't take in certain headers like the rest do? That way the our APIs and options bags reflect what the service supports.

What are you trying to achieve with using this header with GetBlockList ?

I don't have all the details, but my team has a framework built on top of Azure Storage, and after we have a Blob we use ETag for every operation to guarantee we don't have concurrency issues.

[felipepessoto]

there are three ways user can learn that they're doing something wrong:

1. compile time error (fastest feedback, easy to discover)
2. validation checks (feedback deferred to runtime, still easy to discover, but might be annoying)
3. documentation (hard to discover, feedback potentially after hours of debugging, very annoying).

I think 1. is not viable at this point as it will end up being explosion of models in the sdk, which leaves 2. But I guess it's not only that API.

I think 2 is good enough. Although the validation will happen at runtime, it will happen immediately at first call with the wrong parameters, so the developer will discover it very quickly, and most important before it goes to prod.

UPDATE: It depends what are the goals of the SDK, if it is only a wrapper over HTTP calls. Or a SDK that .NET Developer can rely without actively reading HTTP Rest API docs. If it is the latter, I would try to do as much validation as possible and put the developer in the right direction

3-) Developer may never find out what is wrong with their code. In a real world application, the GetBlockList call is only one of the places where you can have concurrency issues, and given it accepts a AccessCondition it would be one of the last places I would investigate.

[felipepessoto]

Do you know why https://docs.microsoft.com/en-us/rest/api/storageservices/get-block-list#request doesn't support access condition headers? I'd like to understand if I'm doing something wrong.

[amnguye]

I would take this question to https://azure.microsoft.com/en-us/support/community/

Apologies in advance for the redirection, but this is due to the face that this is the repo of the SDK, and not where you can ask for feature requests from the service. The SDK simply supports what the service can support and provides as much conveniences to the user, as much as the service supports.

[github-actions]

Hi @felipepessoto, we deeply appreciate your input into this project. Regrettably, this issue has remained inactive for over 2 years, leading us to the decision to close it. We've implemented this policy to maintain the relevance of our issue queue and facilitate easier navigation for new contributors. If you still believe this topic requires attention, please feel free to create a new issue, referencing this one. Thank you for your understanding and ongoing support.

[felipepessoto]

Any updates about making the change number 2?

validation checks (feedback deferred to runtime, still easy to discover, but might be annoying)

[amnguye]

Actually this feature request was completed via this PR.
#22097

Looks like we didn't close this issue cause it fell through the cracks. Apologies about that.

[amnguye]

This should be apart of Azure.Storage.Blobs version 12.11.0

But I recommend to at least upgrading to 12.13.0 to avoid a known vulnerability.
https://www.nuget.org/packages/Azure.Storage.Blobs/12.13.0