search

Azure / azure-storage-cpp

Microsoft Azure Storage Client Library for C++
http://azure.github.io/azure-storage-cpp
Apache License 2.0
132 stars 148 forks source link

Removing the signed-resource requirement for account SAS tokens #396

Open tomswedlund opened 3 years ago

tomswedlund commented 3 years ago

Addressing the following bug: https://github.com/Azure/azure-storage-cpp/issues/383

Jinming-Hu commented 3 years ago

/azp run

azure-pipelines[bot] commented 3 years ago
Azure Pipelines could not run because the pipeline triggers exclude this branch/path.
Jinming-Hu commented 3 years ago

/azp run

azure-pipelines[bot] commented 3 years ago
Azure Pipelines successfully started running 1 pipeline(s).
azure-pipelines[bot] commented 3 years ago
Commenter does not have sufficient privileges for PR 396 in repo Azure/azure-storage-cpp
tomswedlund commented 3 years ago

Hi Jinming,

Sorry for the build break, I swear I built this locally. But this brings up a different issue. Should the conditional that I removed actually be removed, or should we be passing false to the function in case an account SAS is being used? Looks like "require_signed_resource" is only set to true in two places:

Should one of these (or both) be set to false instead?

Tom

Jinming-Hu commented 3 years ago

@tomswedlund Hi, thanks for your contribution. I believe the CI failure has nothing to do with the change in this PR. I'm gonna take some time to fix Azure pipelines.

tomswedlund commented 3 years ago

HI Jinming,

I see a build failure with the following error: microsoft.windowsazure.storage\src\shared_access_signature.cpp(85,0): Error C4100: 'require_signed_resource': unreferenced formal parameter

Looks like the 'require_signed_resource' function param is no longer being used because of my change, and hence the error. Should this parameter be removed altogether? Or should we passing 'false' in the two places I mentioned in my last comment?

Thanks, Tom