Azure / azure-storage-python

Microsoft Azure Storage Library for Python
https://azure-storage.readthedocs.io
MIT License
338 stars 240 forks source link

Issue when generating a url using sas token. Method file_service.make_file_url() #609

Open GeisaFaustino opened 5 years ago

GeisaFaustino commented 5 years ago

Which service(blob, file, queue) does this issue concern?

File share

Which version of the SDK was used? Please provide the output of pip freeze.

azure-storage-file==2.0.1 azure-storage-common==2.0.0 sdk-version

What problem was encountered?

https://github.com/Azure/azure-storage-python/blob/b8c5c9a1bc73065fd015c0fec75617cef263c6ee/azure-storage-file/azure/storage/file/fileservice.py#L233

When genrating a url using the method make_file_url passing a sas token, the generated url has two query strings ('?') instead of one. generated_url

The SaS Token generated on Azure portal starts with a query string. SaSToken-Azure

Have you found a mitigation/solution?

Yes. Remove the query string ('?') at line 233.

Note: for table service, please post the issue here instead: https://github.com/Azure/azure-cosmosdb-python.

xiafu-msft commented 5 years ago

Hi @GeisaFaustino Thanks so much for reaching out and post the fix for this problem! While test_make_file_url_with_sas failed, please give us some time to look into this!

xiafu-msft commented 5 years ago

Hi @GeisaFaustino Could you please tell us how you generate sas_token in the piece of code you paste? (https://user-images.githubusercontent.com/32823639/59705929-c9ba4480-91d5-11e9-8d38-c9809189db2f.PNG) That could be helpful to us!

Thanks

GeisaFaustino commented 5 years ago

Hi @xiafu-msft
Sure! I just went to Azure portal, selected the storage account that I wanted, clicked on "Shared access signature" in the menu at left side and then clicked on "Generate SAS and connection string" button. I am pasting a screen shot of portal.

image image

xiafu-msft commented 5 years ago

Hi @GeisaFaustino Thanks for your detailed explanation! So you mean you found the SAS token from Azure portal and pasted that in your code, and when you called make_file_url, it took the SAS you copied from Azure portal? Probably you should use this api generate_file_shared_access_signature to generate the SAS token instead of copying from Azure portal. Hopefully this example could be helpful https://github.com/Azure/azure-storage-python/blob/8bb4204430adb7a51a1c041ea14e9d075a61fb69/tests/file/test_file.py#L1060

zezha-msft commented 5 years ago

@xiafu-msft we should make a fix, where we check whether the sas token starts with a '?'; if not, add a '?'.

xiafu-msft commented 5 years ago

Here is the pull request to fix the bug https://github.com/Azure/azure-storage-python/pull/611

GeisaFaustino commented 5 years ago

Yes. It was exactly what I did.

Also, I generated the SAS token as you suggested and then the url was created correctly. Thank you! image

However, I would like to suggest that the method make_file_url works with both SAS token, the one from Azure portal and the SAS token generated the API generate_account_shared_access_signature.

xiafu-msft commented 5 years ago

Hi @GeisaFaustino Thanks for catching this bug. You are totally right that ideally the method make_file_url should work with both SAS token and the SAS token generated the API. This pull request https://github.com/Azure/azure-storage-python/pull/611 is to fix that.

bobir01 commented 1 month ago

@xiafu-msft hello, i am also searching smth equal to make_file_url but for only blob service in api i could not find such method ?