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

[API] Storage APIs have a couple places where we do not allow the customer to pass a transactional md5 #4508

Closed
rickle-msft opened this issue Jul 22, 2019 · 9 comments
Assignees
Labels
blocking-release Blocks release Client This issue points to a problem in the data-plane of the library. Storage Storage Service (Queues, Blobs, Files)

Comments

@rickle-msft
Copy link
Contributor

rickle-msft commented Jul 22, 2019

Investigate Java's ability to support a custom polynomial for CRC. May fall back to MD5 if this is not possible. Needs arch board guidance for checksums

@rickle-msft rickle-msft added Client This issue points to a problem in the data-plane of the library. Storage Storage Service (Queues, Blobs, Files) labels Jul 22, 2019
@rickle-msft
Copy link
Contributor Author

service feature

@kurtzeborn
Copy link
Member

@rickle-msft could you provide the details here for the questions you have for the arch board and @JonathanGiles can chime with his opinion and clarify whether this even needs to be reviewed by the entire arch board.

@rickle-msft
Copy link
Contributor Author

@JonathanGiles
The question about CRC specifically is that the Storage implementation uses a custom polynomial that we are not sure we can support in Java. Either it's not doable without using JNI, which is really bad, or it gives no appreciable benefit over MD5, which defeats the purpose. I imagine Jonathan can resolve this independently.

The question that needs more formal guidance is on how we expose checksumming across the SDK. We completely omitted it in v11, but in v8, we had full support for it in a variety of scenarios. We exposed it as an optional parameter that could be attached to certain transactions and would be validated by the service upon receiving data (this is different from setting the Content MD5 property on the blob, which doesn't get validated at all). We also could calculate the MD5 on uploads and downloads and either set it with the final put call or check it against the received contentMD5 header after all the data had been streamed. These were fairly complicated operations that had considerations in the way of usability and performance. It's not clear to us whether we should pass options, accept a flag, provide a stand alone stream-wrapper that the customer should create themselves, etc.

@kurtzeborn kurtzeborn changed the title CRC ~CRC~ MD5 Sep 11, 2019
@kurtzeborn kurtzeborn changed the title ~CRC~ MD5 MD5 Sep 11, 2019
@kurtzeborn
Copy link
Member

Decision was made in xstore sync meeting previously to just use MD5 for Java here.

@jaschrep-msft
Copy link
Member

Closing. MD5 is a settable field.

@rickle-msft
Copy link
Contributor Author

I noticed there are a couple places where we do not allow the customer to pass a transactional md5

@rickle-msft rickle-msft reopened this Oct 8, 2019
@rickle-msft rickle-msft added the blocking-release Blocks release label Oct 10, 2019
@joshfree joshfree changed the title MD5 Storage APIs have a couple places where we do not allow the customer to pass a transactional md5 Oct 14, 2019
@joshfree
Copy link
Member

Updating title to reflect the work that's actually being tracked with this issue

@joshfree joshfree changed the title Storage APIs have a couple places where we do not allow the customer to pass a transactional md5 [API] Storage APIs have a couple places where we do not allow the customer to pass a transactional md5 Oct 22, 2019
@joshfree
Copy link
Member

Fixed by #5947

@joshfree
Copy link
Member

Closing now that #5947 has been merged.

@github-actions github-actions bot locked and limited conversation to collaborators Apr 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
blocking-release Blocks release Client This issue points to a problem in the data-plane of the library. Storage Storage Service (Queues, Blobs, Files)
Projects
None yet
Development

No branches or pull requests

6 participants