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

Check both Content-MD5 and x-ms-blob-content-md5 #2487

Merged
merged 1 commit into from
Jan 15, 2025
Merged

Conversation

gaul
Copy link
Contributor

@gaul gaul commented Nov 10, 2024

No description provided.

Copy link
Member

@blueww blueww left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a test case to cover it.
Besides that, please consider that API which support Content-MD5 and x-ms-blob-content-md5.

@gaul
Copy link
Contributor Author

gaul commented Nov 11, 2024

Could you expand on your suggestion? I believe that for block blobs Content-MD5 and x-ms-blob-content-md5 are synonyms: https://technet2.github.io/Wiki/blogs/windowsazurestorage/windows-azure-blob-md5-overview.html . It appears that the JavaScript API only sends the former but the Java API sends the latter. How can I inject a header that does not have a x-ms-meta- prefix?

@gaul
Copy link
Contributor Author

gaul commented Jan 13, 2025

@blueww Is there any way to move this PR forward? I don't believe that the test you suggested is possible with the SDK.

@blueww
Copy link
Member

blueww commented Jan 13, 2025

@gual

Please refer this test case for how to add header to request send from Storage JS SDK with customized policy.

serviceClientForOPTIONS.pipeline.addPolicy(customPolicy);

The customized policy class is defined in https://github.com/Azure/Azurite/blob/main/tests/blob/RequestPolicy/OPTIONSRequestPolicyFactory.ts

The sample test case changes several items in the request, if only for header, see a more simple guild in #2462 (comment)

@gaul
Copy link
Contributor Author

gaul commented Jan 14, 2025

@blueww I added the suggested test -- please take a look.

// Customize HTTP requests and responses by overriding sendRequest
// Parameter request is Azure.WebResource type
async sendRequest(request: WebResource) {
request.headers.set("x-ms-blob-content-md5", `${this.md5}`);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be better to write a more generic policy to add a header from input header name and value, then other coming test cases can also use it, instead of each header has seperate class to add header.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@blueww blueww merged commit ee4da29 into Azure:main Jan 15, 2025
34 checks passed
@gaul gaul deleted the content-md5 branch January 15, 2025 07:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants